Support <gazebo_ros> in package.xml exports
#492
Conversation
This copies the implementation from `gazebo_ros_paths.py` to provide a
way for packages to set resource paths from `package.xml`.
```
e.g. <export>
<gazebo_ros gazebo_model_path="${prefix}/models"/>
<gazebo_ros gazebo_media_path="${prefix}/models"/>
</export>
```
The value of `gazebo_model_path` and `gazebo_media_path` is appended to `GZ_SIM_RESOURCE_PATH`
The value of `plugin_path` appended to `GZ_SIM_SYSTEM_PLUGIN_PATH`
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Good point!
Yes, I'll cherry-pick this to Iron and Rolling. I was going to start on Rolling first, but I wanted to make sure the |
There was a problem hiding this comment.
Just commenting here what we spoke in the weekly meeting: let's try to create a tutorial that documents all the options of injecting the paths available and point from this code to the user to specify that this is not the easiest way or the proffered way with a pointer to the tutorial.
| if 'gazebo_model_path' in export.attributes: | ||
| xml_path = export.attributes['gazebo_model_path'] | ||
| xml_path = xml_path.replace('${prefix}', package_share_path) | ||
| gazebo_model_path.append(xml_path) |
There was a problem hiding this comment.
I think that one of the most frustrating issue for newcomers and people dealing with Gazebo + ROS integration is to badly configure the asset directories assuming random paths that are not the ones really processed here. I would like to use the PR to improve this situation.
How about adding a bit of sanity checks before the .append( calls in this file that can emit warning/errors for the user to check/fix, some ideas:
- The
xml_pathin a vast majority of the cases needs to be an existing non empty directory. - For the
gazebo_model_path, we can probably scan for the expected subdirectory structure with at least one .sdf and one .config. - For
gazebo_plugin_pathwe can probably expect that the user hosts at least one .so or one binary file.
There was a problem hiding this comment.
I looked at doing this but the way this works, it's actually looking at all the packages in the system, not just the individual package the user might be interested in. And would have to do the checks for all packages every time this file is launched. Aside from the performance implications, a user launching this file would see errors/warnings coming from other packages they have no control over.
An alternative is to create a tool that can be used to search for a particular model/resource by it's URI. For example, if I wanted to check that the URI model://my_awesome_pkg/models/cool_robot is valid, I would run this tool and it would search all exported model paths and give me some diagnostic messages if the model couldn't be found.
There was a problem hiding this comment.
@j-rivero any thoughts? I'd like to unblock ros/urdf_sim_tutorial#16 soon if possible.
There was a problem hiding this comment.
I looked at doing this but the way this works, it's actually looking at all the packages in the system, not just the individual package the user might be interested in.
I was thinking on limiting the directories to inspect to the ones added via:
<export>
<gazebo_ros gazebo_model_path="${prefix}/models"/>
<gazebo_ros gazebo_media_path="${prefix}/media"/>
<gazebo_ros plugin_path="${prefix}/plugins"/>
</export>Why do we need to check all the packages in the system?
And would have to do the checks for all packages every time this file is launched. Aside from the performance implications,
I agree that there is a performance implication that can be bad.
An alternative is to create a tool that can be used to search for a particular model/resource by it's URI. For example, if I wanted to check that the URI model://my_awesome_pkg/models/cool_robot is valid, I would run this tool and it would search all exported model paths and give me some diagnostic messages if the model couldn't be found.
That would be really useful indeed. Another approach would be to enable a launch argument or environment flag that enable this debug behavior.
Given the situation, I'm not blocking the PR, thought it was easier to add the checks.
There was a problem hiding this comment.
The code for package_name in get_package_names(): iterates through all the packages available. Then we'd need to check if any package that exports gazebo_ros meets the sanity requirements you described.
|
Are we waiting on the Tutorial to get this merged? I have a URDF/Gazebo tutorial waiting to be merged and there is a section ("Side note: Configuring meshes") that covers this area. The tutorial is written for for Gazebo Classique, but as linked above, I'm trying to get the Gazebo New Coke version to work too. |
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
|
|
|
@azeey do you have any plan to forward port this to iron and rolling? |
馃帀 New feature
Summary
This copies the implementation from
gazebo_ros_paths.pyto provide a way for packages to set resource paths frompackage.xml.The value of
gazebo_model_pathandgazebo_media_pathis appended toGZ_SIM_RESOURCE_PATHThe value ofplugin_pathappended toGZ_SIM_SYSTEM_PLUGIN_PATH.This will make transition from Gazebo Classic easier.
Test it
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.