Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standalone Ignition Gazebo executables #694

Closed
wants to merge 6 commits into from

Conversation

mjcarroll
Copy link
Contributor

🎉 Standalone Executables

Summary

Adds ign-gazebo-server and ign-gazebo-gui to be used as standalone executables in valgrind, gdb, heaptrack, etc.

Test it

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

This introduces the framework for two standalone executables: "server" and "gui"

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll added the beta Targeting beta release of upcoming collection label Mar 18, 2021
@mjcarroll mjcarroll self-assigned this Mar 18, 2021
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Mar 18, 2021
@chapulina chapulina requested review from chapulina and scpeters March 19, 2021 02:15
@chapulina chapulina removed the beta Targeting beta release of upcoming collection label Mar 25, 2021
@chapulina
Copy link
Contributor

This can come in a minor, removed from beta

@scpeters
Copy link
Member

We just merged the proof-of-concept for this in gazebosim/gz-transport#216, but it was a bit different since ign-transport uses separate commands for each executable and put most of the command-line parsing into the c++ executables:

  • ign topic -> ign-transport-topic
  • ign service -> ign-transport-service

while ign gazebo uses arguments to the gazebo command to determine whether to open the server or gui or both:

  • ign gazebo -s -> ign-gazebo-server
  • ign gazebo -g -> ign-gazebo-gui
  • ign gazebo -> both ign-gazebo-server and ign-gazebo-gui

Furthermore, the cmdgazebo.rb ruby command has complex logic for finding the world file to load (specified as a positional argument):

  • check if file is a path to an existing file
  • if not, check for file as a relative path in all the paths listed in IGN_GAZEBO_RESOURCE_PATH
  • if still not, check for file as a relative path to the worldInstallDir function
  • if still not, call findFuelResource to try to download the file
  • then after all that, parse the file through embedded ruby (ERB)

it would be helpful to have a plan for what will be done in C++ and what will be done in ruby in order to proceed

@mjcarroll
Copy link
Contributor Author

ign gazebo -s -> ign-gazebo-server
ign gazebo -g -> ign-gazebo-gui
ign gazebo -> both ign-gazebo-server and ign-gazebo-gui

This seems reasonable, but I think that @ahcorde 's current work may allow us to not run them as separate processes. Part of this was an artifact of OGRE.

command has complex logic for finding the world file to load

I think it would also be helpful to have this logic implemented in C++, so it could be accessible via API.

@chapulina
Copy link
Contributor

I think it would also be helpful to have this logic implemented in C++, so it could be accessible via API.

All of that should be available from C++, except for the ERB parsing. That's why we had to duplicate a lot of the file locating logic from C++ into the Ruby script. It would be great to have that consistent regardless of whether we use the C++ API or the CLI.

@ahcorde
Copy link
Contributor

ahcorde commented Aug 13, 2021

This seems reasonable, but I think that @ahcorde 's current work may allow us to not run them as separate processes. Part of this was an artifact of OGRE.

PR #793

Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
@peci1
Copy link
Contributor

peci1 commented Jan 5, 2022

@mjcarroll why is this PR closed? Is there any ongoing effort in this direction for ign-gazebo?

@mjcarroll mjcarroll deleted the mjcarroll/standalone_cli branch August 1, 2022 15:46
@mjcarroll mjcarroll restored the mjcarroll/standalone_cli branch September 16, 2022 18:08
@mjcarroll mjcarroll reopened this Sep 16, 2022
@mjcarroll mjcarroll changed the base branch from ign-gazebo5 to gz-sim7 September 16, 2022 18:08
@mjcarroll
Copy link
Contributor Author

This is out-of-date enough that I'm going to close this PR and re-open with a new implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants