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.

Sign up for GitHub

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

Jump to bottom

Support <gazebo_ros> in package.xml exports #492

Merged
merged 3 commits into from
Mar 11, 2024
Merged

Support <gazebo_ros> in package.xml exports #492

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7eb4945
Support `<gazebo_ros>` in `package.xml` exports
azeey Feb 2, 2024
5859b2a
Add a section to README on the `<gazebo_ros>` tag
azeey Feb 28, 2024
cb902d7
Simplified code per reviewer feedback
azeey Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 88 additions & 6 deletions ros_gz_sim/launch/gz_sim.launch.py.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,102 @@

"""Launch Gazebo Sim with command line arguments."""

import os
from os import environ

from ament_index_python.packages import get_package_share_directory
from catkin_pkg.package import InvalidPackage, PACKAGE_MANIFEST_FILENAME, parse_package
from ros2pkg.api import get_package_names
from launch import LaunchDescription
from launch.actions import DeclareLaunchArgument, OpaqueFunction
from launch.actions import ExecuteProcess, Shutdown
from launch.substitutions import LaunchConfiguration

# Copied from https://github.com/ros-simulation/gazebo_ros_pkgs/blob/79fd94c6da76781a91499bc0f54b70560b90a9d2/gazebo_ros/scripts/gazebo_ros_paths.py
"""
Search for model, plugin and media paths exported by packages.

e.g. <export>
<gazebo_ros gazebo_model_path="${prefix}/models"/>
<gazebo_ros gazebo_media_path="${prefix}/models"/>
</export>
${prefix} is replaced by package's share directory in install.

Thus the required directory needs to be installed from CMakeLists.txt
e.g. install(DIRECTORY models
DESTINATION share/${PROJECT_NAME})
"""

class GazeboRosPaths:

@staticmethod
def get_paths():
gazebo_model_path = []
gazebo_plugin_path = []
gazebo_media_path = []

for package_name in get_package_names():
package_share_path = get_package_share_directory(package_name)
package_file_path = os.path.join(package_share_path, PACKAGE_MANIFEST_FILENAME)
if os.path.isfile(package_file_path):
try:
package = parse_package(package_file_path)
except InvalidPackage:
continue
for export in package.exports:
if export.tagname == 'gazebo_ros':
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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:

  1. The xml_path in a vast majority of the cases needs to be an existing non empty directory.
  2. For the gazebo_model_path, we can probably scan for the expected subdirectory structure with at least one .sdf and one .config.
  3. For gazebo_plugin_path we can probably expect that the user hosts at least one .so or one binary file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@j-rivero any thoughts? I'd like to unblock ros/urdf_sim_tutorial#16 soon if possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

if 'plugin_path' in export.attributes:
xml_path = export.attributes['plugin_path']
xml_path = xml_path.replace('${prefix}', package_share_path)
gazebo_plugin_path.append(xml_path)
if 'gazebo_media_path' in export.attributes:
xml_path = export.attributes['gazebo_media_path']
xml_path = xml_path.replace('${prefix}', package_share_path)
gazebo_media_path.append(xml_path)

gazebo_model_path = os.pathsep.join(gazebo_model_path)
gazebo_plugin_path = os.pathsep.join(gazebo_plugin_path)
gazebo_media_path = os.pathsep.join(gazebo_media_path)

return gazebo_model_path, gazebo_plugin_path, gazebo_media_path


def launch_gz(context, *args, **kwargs):
env = {'GZ_SIM_SYSTEM_PLUGIN_PATH':
':'.join([environ.get('GZ_SIM_SYSTEM_PLUGIN_PATH', default=''),
environ.get('LD_LIBRARY_PATH', default='')]),
'IGN_GAZEBO_SYSTEM_PLUGIN_PATH': # TODO(CH3): To support pre-garden. Deprecated.
':'.join([environ.get('IGN_GAZEBO_SYSTEM_PLUGIN_PATH', default=''),
environ.get('LD_LIBRARY_PATH', default='')])}
model_path, plugin_path, media_path = GazeboRosPaths.get_paths()
env = {
"GZ_SIM_SYSTEM_PLUGIN_PATH": os.pathsep.join(
[
environ.get("GZ_SIM_SYSTEM_PLUGIN_PATH", default=""),
environ.get("LD_LIBRARY_PATH", default=""),
plugin_path,
]
),
"IGN_GAZEBO_SYSTEM_PLUGIN_PATH": os.pathsep.join( # TODO(CH3): To support pre-garden. Deprecated.
[
environ.get("IGN_GAZEBO_SYSTEM_PLUGIN_PATH", default=""),
environ.get("LD_LIBRARY_PATH", default=""),
plugin_path,
]
),
"GZ_SIM_RESOURCE_PATH": os.pathsep.join(
[
environ.get("GZ_SIM_RESOURCE_PATH", default=""),
model_path,
j-rivero marked this conversation as resolved.
Show resolved Hide resolved
media_path
]
),
"IGN_GAZEBO_RESOURCE_PATH": os.pathsep.join( # TODO(azeey): To support pre-garden. Deprecated.
[
environ.get("IGN_GAZEBO_RESOURCE_PATH", default=""),
model_path,
media_path,
]
),
}

gz_args = LaunchConfiguration('gz_args').perform(context)
gz_version = LaunchConfiguration('gz_version').perform(context)
Expand Down
Loading