Add config relative path environment variable

[mabelzhang]

🎉 New feature

Closes #337

Summary

Before trying to load the --config path, check environment variable GZ_GUI_RESOURCE_PATH.
This allows relative config paths with respect to those environment variables to be specified after --config.

Implementation similar to ign-gazebo https://github.com/ignitionrobotics/ign-gazebo/blob/b825c27841f38c1df06cd053f70f9836786b590d/src/cmd/cmdgazebo.rb.in#L393-L409 but in C++ instead of Ruby.

Questions:

- Do we want both variables? Are those the final names we want? The ign-gazebo env var name is IGN_GAZEBO_RESOURCE_PATH. So it might be more consistent if we call it IGN_GUI_RESOURCE_PATH.
From the description of the original issue, maybe we only want IGN_GUI_CONFIG_PATH and not the *RESOURCE* variable?

- Is there a way to test at the ign gui level that automatically close the GUI?
Currently, the added unit test brings up the GUI via popen, but doesn't close it. That's not going to work as an automated test, as it requires manual closing of the window. The way ign-gazebo does it is by running headless (-s), which ign-gui doesn't have a flag for. Other ign-gui tests that automatically close the window initialize Application or MainWindow in the code. Here, we are testing the Ruby code at the ign command a level above, so we don't have those objects.
Should we just not test the valid cases? Only the valid cases bring up the GUI. The invalid cases just print a message and exit.

Test it

Environment variable should show up in help message:

$ ign gui --help
...
Environment variables:                                                  
  GZ_GUI_RESOURCE_PATH    Colon separated paths used to locate GUI     
 resources such as configuration files.

Unit test:

./build/ignition-gui3/bin/UNIT_Application_TEST 

Manual test:

Test with config files in examples/config/.
For all following commands, replace environment variable with path on your machine.

Valid environment variable and file, should show color themed GUI:

GZ_GUI_RESOURCE_PATH=/path/to/ign/src/ign-gui/examples/config ign gui --config style.config

Invalid file:

# No env var
ign gui --config pubsub.config
[GUI] [Err] [Application.cc:263] Failed to load file [style.config]: XMLError

# Valid env var, invalid file
$ GZ_GUI_RESOURCE_PATH=/path/to/ign/src/ign-gui/examples/config ign gui --config pubsub.config2
[GUI] [Err] [Application.cc:263] Failed to load file [style.config]: XMLError

Multiple paths, some valid, some invalid. Should find file and load up correct GUI:

GZ_GUI_RESOURCE_PATH=/invalid:/path/to/ign/src/ign-gui/examples/config ign gui --config style.config

Test at ign-gazebo level:

Should show color themed GUI (add Gz Scene 3D plugin to see the world axis):

GZ_GUI_RESOURCE_PATH=/path/to/ign/src/ign-gui/examples/config ign gazebo --gui-config style.config default.sdf

Do after merging

New env var should be added to the ign gazebo help message after this is merged.

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[chapulina]

Thanks for the PR! A few preliminary notes:

• cmdgui.rb affects the command line tool, but not when ign-gui is used as a library, which is an important use case for simulation. So I think the lookup should probably be done around here:

  https://github.com/ignitionrobotics/ign-gui/blob/eac2fc29f6ee6fae0dac30901deec28073025539/src/Application.cc#L217-L227

Do we want both variables? Are those the final names we want?

I think only one variable is enough; "resource" sounds good to me.

We shouldn't add any new variables with "ign", because we'd need to tick-tock them later. That applies even for Citadel and Fortress. So let's go with GZ_GUI_RESOURCE_PATH here.

Is there a way to test at the ign gui level that automatically close the GUI?

Maybe that won't be necessary once the implementation is moved to Application?

[codecov]

Codecov Report

Merging #386 (6d25226) into ign-gui3 (aafacbf) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           ign-gui3     #386   +/-   ##
=========================================
  Coverage     31.12%   31.12%           
=========================================
  Files            29       29           
  Lines          1343     1343           
=========================================
  Hits            418      418           
  Misses          925      925           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aafacbf...6d25226. Read the comment docs.

[mabelzhang]

Ah okay. I contemplated putting it in Application but went to cmdgui after looking at ign-gazebo. Hadn't thought of the library use. Will move.

[mabelzhang]

All above resolved - moved to Application, renamed env var to GZ_*, unit test OK.
Should add env var description to ign gazebo help message after this PR is in.
Ready for review.

[chapulina]

Thanks! Works for me!

I think it's worth it adding a note about the env var to the config tutorial:

• https://github.com/ignitionrobotics/ign-gui/blob/ign-gui6/tutorials/07_config.md#loading

[chapulina]

Thanks for iterating, LGTM!