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

Ignore ros-args in parameter bridge #65

Merged
merged 3 commits into from
Mar 31, 2020
Merged

Conversation

luca-della-vedova
Copy link
Member

Starting from eloquent, the --ros-args flag becomes mandatory and, according to the design document, even an empty --ros-flags is valid.

When embedding the parameter_bridge in a launch file in eloquent, i.e. in the following way (forwarding ignition clock to ROS2):

    <node pkg="ros_ign_bridge" exec="parameter_bridge"
        args="/clock@rosgraph_msgs/msg/Clock[ignition.msgs.Clock"
    />

ros_ign_bridge crashes because it doesn't expect the --ros-args flag:

[ERROR] [parameter_bridge-1]: process has died [pid 1398, exit code 255, cmd '/home/luca/demos_ws/install/ros_ign_bridge/lib/ros_ign_bridge/parameter_bridge /clock@rosgraph_msgs/msg/Clock[ignition.msgs.Clock --ros-args'].

This PR ignores every command line argument after the --ros-args (since it's not used anyway in the bridge), fixing the crash.

Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
@chapulina
Copy link
Contributor

Technically, user-defined arguments could come after a -- after --ros-args. Should we handle that case? i.e. ignore all args between --ros-args and --?

@chapulina chapulina added the ROS 2 ROS 2 label Mar 27, 2020
Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
@luca-della-vedova
Copy link
Member Author

luca-della-vedova commented Mar 30, 2020

True, the specific use case was for use in launch files but it makes sense to fix all the cases.
Since it was becoming quite ugly if we kept the same structure (i.e. we would have needed to "change" the iterator in two places or edit the vector while it is being iterated on) I created a new function that to preprocess the arguments and convert them to a vector of strings for easier iteration.

Tested with the following commands:
Original use case:
ros2 run ros_ign_bridge parameter_bridge /clock@rosgraph_msgs/msg/Clock@ignition.msgs.Clock --ros-args

And recommended use case:
ros2 run ros_ign_bridge parameter_bridge --ros-args test1 -test2 __test3 -- clock@rosgraph_msgs/msg/Clock@ignition.msgs.Clock

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM! @luca-della-vedova , could you just fix the lint_cmake and uncrustify test failures before merging? Thanks!

We gotta check why Travis didn't mark the build as failed - CC 👨‍🌾 @j-rivero

Signed-off-by: Luca Della Vedova <luca@openrobotics.org>
@luca-della-vedova luca-della-vedova merged commit 5903853 into dashing Mar 31, 2020
@luca-della-vedova luca-della-vedova deleted the ignore_ros_args branch March 31, 2020 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ROS 2 ROS 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants