ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
126 stars 141 forks source link

Improve launch arguments introspection #556

Closed ivanpauno closed 2 years ago

ivanpauno commented 2 years ago

Currently you can set an argument of a nested launch description from the top level, without the need of redeclaring the argument. This updates the get_launch_arguments() method to return a more complete result (currently it doesn't return arguments from included launch descriptions). It also updates the include_launch_description launch arguments validation by tracking all the include launch description actions in the middle (this basically avoids reintroducing https://github.com/ros2/launch/issues/248).

Related discussion https://github.com/ros2/launch/issues/313.

ivanpauno commented 2 years ago

@jacobperron I'm still thinking if this is the right approach, but feedback would be appreciated.

ivanpauno commented 2 years ago

The only other thing I would mention is that when displaying them on the CLI it would be nice to differentiate between arguments that are conditional and ones that are certain.

That's already possible, see:

https://github.com/ros2/launch/blob/8bb79dd1fa64e84b734475f27c16714d788206d7/launch/launch/launch_description.py#L131-L132

and:

https://github.com/ros2/launch_ros/blob/b84b72748e22d8fd648151d96478d19305c96c82/ros2launch/ros2launch/api/api.py#L104-L106

Stuffing the attribute in the original action is a bit hacky though, we could maybe be returning a:

List[Tuple[DeclareLaunchArgument, List[IncludeLaunchDescription], bool]]

where the last boolean indicates if the DeclareLaunchArgument action is being conditionally included or not.

ivanpauno commented 2 years ago