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

Add XML and YAML launch scripts for dummy_robot_bringup #440

Merged
merged 3 commits into from
Apr 27, 2020
Merged

Add XML and YAML launch scripts for dummy_robot_bringup #440

merged 3 commits into from
Apr 27, 2020

Conversation

p-vega
Copy link
Contributor

@p-vega p-vega commented Apr 19, 2020

Add XML and YAML launch scripts for dummy_robot_bringup
Fixes #434
Signed-off-by: Pablo Vega epvega@gmail.com

Signed-off-by: Pablo Vega <epvega@gmail.com>
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

Nice addition, but for one comment.

pkg: robot_state_publisher
exec: robot_state_publisher
output: screen
args: $(find-pkg-share dummy_robot_bringup)/launch/single_rrbot.urdf
Copy link
Contributor

Choose a reason for hiding this comment

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

@p-vega this form of robot_state_publisher configuration is deprecated -- use parameters instead of command line arguments, like in the XML.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hidmic Thanks for reviewing, I couldn't find any example of the proper parameter syntax for the yaml launch script. Can you point me in the right direction?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, check this test.

Use parameters instead of command line arguments for
robot_state_publisher configuration

Signed-off-by: Pablo Vega <epvega@gmail.com>
@p-vega p-vega requested a review from hidmic April 25, 2020 09:03
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM

@jacobperron shall we merge this now or defer it till after Foxy?

<launch>
<node pkg="dummy_map_server" exec="dummy_map_server" output="screen" />
<node pkg="robot_state_publisher" exec="robot_state_publisher" output="screen" >
<param name='robot_description' value="$(command 'cat $(find-pkg-share dummy_robot_bringup)/launch/single_rrbot.urdf')"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

@p-vega why single quotes for the name attribute value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hidmic you're right, they should be double quotes. I'm sorry for the typo, I'll change it and I'll add a new commit.

@jacobperron
Copy link
Member

shall we merge this now or defer it till after Foxy?

@hidmic I don't think we need to defer since this is a leaf package.

… name

Signed-off-by: Pablo Vega <epvega@gmail.com>
@hidmic
Copy link
Contributor

hidmic commented Apr 27, 2020

Alright, there're no tests covering this functionality so I don't think running CI's worth it. Merging.

Thank you for your contribution @p-vega !

@hidmic hidmic merged commit 359ffb4 into ros2:master Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use launch front-end for dummy_robot_bringup
3 participants