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 ROS2 Launch Files to all relevant examples #289

Merged
merged 1 commit into from
Aug 11, 2023

Conversation

adeeb10abbas
Copy link
Member

@adeeb10abbas adeeb10abbas commented Aug 7, 2023

This PR adds launch files to all the examples that involve a visual (rviz) component. All the launch files were tested.

@EricCousineau-TRI for review, thanks!


This change is Reviewable

Copy link
Collaborator

@EricCousineau-TRI EricCousineau-TRI left a comment

Choose a reason for hiding this comment

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

:lgtm: with a few small nits, thanks!

Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @adeeb10abbas)


drake_ros_examples/examples/hydroelastic/launch/hydroelastic_cc_launch.py line 6 at r1 (raw file):

from ament_index_python.packages import get_package_prefix
from ament_index_python.packages import get_package_share_directory
import os

nit (here and elsewhere) For imports, can you sort between (builtin, third-party), and then alpha sort between those?

import os

from ament_index_python.packages import get_package_prefix
from ament_index_python.packages import get_package_share_directory
from launch import LaunchDescription
from launch.actions import ExecuteProcess
from launch_ros.actions import Node

drake_ros_examples/examples/iiwa_manipulator/README.md line 40 at r1 (raw file):

Or
```sh
# Using ROS2 Launch

nit For consistency with others, can you put C++ first, Python second, and use the same comments?
e.g.

# Using ROS2 Launch (C++)
...
# or (Python)
...

drake_ros_examples/examples/iiwa_manipulator/README.md line 40 at r1 (raw file):

Or
```sh
# Using ROS2 Launch

nit (here and elsewhere) Should use ROS 2 (with space), rather than ROS2 (no space)


drake_ros_examples/examples/iiwa_manipulator/launch/iiwa_manipulator_py_launch.py line 18 at r1 (raw file):

    )

    # Node for multirobot in C++

nit copy-pasta, C++ -> Python.

Alternatively, consider removing these comments?

Copy link
Member Author

@adeeb10abbas adeeb10abbas left a comment

Choose a reason for hiding this comment

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

Reviewable status: 4 of 12 files reviewed, 1 unresolved discussion (waiting on @EricCousineau-TRI)


drake_ros_examples/examples/iiwa_manipulator/launch/iiwa_manipulator_py_launch.py line 18 at r1 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

nit copy-pasta, C++ -> Python.

Alternatively, consider removing these comments?

yeah will just get rid of 'em! Done!

Launch files were added to the following examples.
	- `multirobot`
	- `hydroelastic`
	- `iiwa_manipulator`
@EricCousineau-TRI
Copy link
Collaborator

Flake appears similar to #290 (most likely a timeout), merging for now (\cc @calderpg-tri @IanTheEngineer )

@EricCousineau-TRI EricCousineau-TRI merged commit 656bbc7 into RobotLocomotion:main Aug 11, 2023
6 of 7 checks passed
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.

2 participants