-
Notifications
You must be signed in to change notification settings - Fork 776
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
[ros2] gazebo_ros_init plugin #776
Conversation
[ros2] gazebo_ros_pkgs 'meta'package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! A couple of optional ideas for simplicity
void GazeboRosInitTest::TearDown() | ||
{ | ||
// If fork failed, don't need to do anything | ||
if (pid_ < 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonder if this test could be refactored to avoid the duplicate code for forking / subscribing / verifying messages received that is in the other tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah you're right I'll do that. I had started writing a completely different test but then they ended up very similar so it should be possible to move this to test_plugins
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done on 10be195
gazebo_ros/test/CMakeLists.txt
Outdated
|
||
foreach(world ${worlds}) | ||
configure_file(worlds/${world}.world ${world}.world COPYONLY) | ||
endforeach() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be simpler to just install the worlds directory unless we plan on having non-world files in there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean just configuring the whole directory (i.e. copying to the build folder) or actually installing? I agree with configuring, but I don't think it's a good idea to install them since they're specific to tests; otherwise we'd need to install the plugins too for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just copying the whole worlds directory to build: 10be195
Redo of #775
Added the
gazebo_ros_init
system plugin, whose single responsibility is to forward command line arguments fromgzserver
torclcpp::init
.Also added a simple launch file
empty_world.launch.py
, which launchesgzserver
with thegazebo_ros_init
plugin. It's not really useful right now, because as I understand it, launch files currently don't support arguments (see [launch] add new concept for LaunchArgument ros2/launch#107). So you might as well callgzserver
directly.You can try it like this:
ros2 launch gazebo_ros empty_world.launch.py
Added a test world plugin
ros_world_plugin
and test worldros_world_plugin.world
to test that command line arguments passed together withgazebo_ros_init
affect all nodes launched afterwards.For example, something like this should remap the world plugin's topic:
GAZEBO_PLUGIN_PATH=${GAZEBO_PLUGIN_PATH}:build/gazebo_ros/test/; gzserver --verbose -s libgazebo_ros_init.so src/gazebo_ros_pkgs/gazebo_ros/test/worlds/ros_world_plugin.world ros_world_plugin:/test:=/new_test
And this should change the name of the node in the world plugin:
GAZEBO_PLUGIN_PATH=${GAZEBO_PLUGIN_PATH}:build/gazebo_ros/test/; gzserver --verbose -s libgazebo_ros_init.so src/gazebo_ros_pkgs/gazebo_ros/test/worlds/ros_world_plugin.world ros_world_plugin:__node:=new_ros_world_plugin