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

Change robot types to include a namespace #272

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

luca-della-vedova
Copy link
Member

@luca-della-vedova luca-della-vedova commented Dec 27, 2024

New feature implementation

Depends on open-rmf/rmf_traffic_editor#521.

Implemented feature

This PR changes the robot_spawn_type in demo maps to include an Open-RMF namespace, similar to how normal models have an OpenRobotics namespace. The relevant models (all except the RobotPlaceholder that is not used anymore as far as I can tell) have been uploaded to a new Open-RMF organization on the gazebosim portal.
This is to help in future-proofing the maps for the site editor.

Test it!

When doing a clean build with this branch and the matching rmf_traffic_editor branch, all demos should still work.

Signed-off-by: Luca Della Vedova <[email protected]>
Signed-off-by: Luca Della Vedova <[email protected]>
@luca-della-vedova luca-della-vedova changed the title Remove rmf_demos_assets and use robots in Fuel Change robot types to include a namespace Dec 31, 2024
Comment on lines +730 to +731
- {model_name: Open-RMF/TeleportDispenser, name: mopcart_dispenser, static: true, x: 284.29, y: 443.7, yaw: 1.5708, z: 0}
- {model_name: Open-RMF/TeleportIngestor, name: mopcart_collector, static: true, x: 317.372, y: 589.088, yaw: 1.5708, z: 0}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this change, together with uploading the TeleportIngestor and TeleportDispenser plugins to fuel, will make pit_crew download the models and there will be silently two of the same model in .gazebo/models as well as the rmf_demos installation folder.
The possible actions are either:

  1. Remove the models from Fuel (Site editor won't be able to download it) or:
  2. Remove the model from rmf_demos_assets and it will be downloaded by pit_crew (it's just a box with a plugin).
  3. Keep as is and there might be ambigious lookup in Gazebo.

Option 3) is definitely not a good idea, 1) and 2) have different tradeoffs

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is gazebo only given the plain model name without the organization name to distinguish between them? And is gazebo not able to account for an organization name when searching for a model?

If yes to both then I imagine this is a broader problem that will need to be resolved within Gazebo or else it's going to cause trouble for large projects in general.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is gazebo only given the plain model name without the organization name to distinguish between them?

Yes. As part of the generation the organization name is removed here so OpenRobotics/Chair is translated into a bare model://Chair.

And is gazebo not able to account for an organization name when searching for a model?

It should definitely be possible if we specify models the "proper gazebo way", which is through Fuel URLs other than our own schema, like this.

I haven't looked into what happens if we remove the "organization stripping" logic from our world generation, but there are a few caveats:

  • If it works as intended, we will need to change our model download pipeline to have subfolders for organizations (it currently doesn't).
  • We will also need to change how we install models from rmf_demos_assets to be installed under an Open-RMF organization subfolder.
  • If users have their own asset package (i.e. forked rmf_demos_assets) it will stop working unless they also update their folder structure.

Finally, the root of this specific issue is that the asset exists both on Fuel and rmf_demos_assets and that would persist. pit_crew has no knowledge of what models have been installed and sourced in the workspace so it can't detect that an Open-RMF/TeleportIngestor has been installed. It will only detect that the map has an Open-RMF/TeleportIngestor model and thus it should try downloading it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

2 participants