ros2 / launch

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

Difficult to specify YAML argument choices #698

Open danzimmerman opened 1 year ago

danzimmerman commented 1 year ago

Expected behavior

With no clear documentation I could find, I expected to be able to write a YAML launch file argument declaration in a way that mirrors Python launch files.

For example, I want to translate the following, taken from ur_robot_driver/launch/ur_control.launch.py:

DeclareLaunchArgument(
    "ur_type",
    description="Type/series of used UR robot.",
    choices=["ur3", "ur3e", "ur5", "ur5e", "ur10", "ur10e", "ur16e"],
    default_value="ur5e",
)

YAML allows list syntax in a couple of different ways, so I expected I might be able to write:

- arg:
    name: ur_type
    default: ur5e
    choices: [ur3, ur3e, ur5, ur5e, ur10, ur10e, ur16e]
    description: Robot to use in the examples.

There's a required change of default_value to default, but that is clear from the official docs' examples.

Actual behavior

I couldn't find any documentation that YAML launch supported choices. However, I dug around in the source code and in issues/PRs, and figured out that it was possible to do it in XML (for example, #529)

I had to iterate through the following errors:

[ERROR] [launch]: Caught exception in launch (see debug for traceback): 
  Caught exception when trying to load file of format [yaml]: 
    Unexpected key(s) found in `arg`: {'choices'}
...
[ERROR] [launch]: Caught exception in launch (see debug for traceback): 
  Caught exception when trying to load file of format [yaml]: 
    Attribute choice of Entity arg expected to be a list of dictionaries.
...
[ERROR] [launch]: Caught exception in launch (see debug for traceback): 
  Caught exception when trying to load file of format [yaml]: 
    Can not find attribute value in Entity choice

In the end, this appears to be the a valid way to supply argument choices in a YAML launch file:

- arg:
    name: ur_type
    default: ur5e
    choice: [value: ur3, value: ur3e, value: ur5, value: ur5e, value: ur10, value: ur10e, value: ur16e]
    description: Robot to use in the examples.

With this argument declaration, I get the desired validation behavior when passing an incorrect command-line value of ur_type:=ur5x, and no errors if I pass one of the suggested/declared choices:

ros2 launch ur_examples_gazebo_classic ur_sim_examples.launch.yaml ur_type:=ur5x
[INFO] [launch]: All log files can be found below /home/dan/.ros/log/2023-03-27-12-06-31-631160-schelkunoff-530229
[INFO] [launch]: Default logging verbosity is set to INFO
[ERROR] [launch.actions.declare_launch_argument]: 
  Argument "ur_type" provided value "ur5x" is not valid. 
    Valid options are: ['ur3', 'ur3e', 'ur5', 'ur5e', 'ur10', 'ur10e', 'ur16e']

However, this is pretty non-obvious.

It seems like the frontend is parsing YAML the same way as XML. In XML, each choice is expected to be its own XML tag with a value, like this test added in #529:

<launch>
    <arg name="my_arg" default="asd" description="something">
        <choice value="asd"/>
        <choice value="bsd"/>
    </arg>
</launch>

Reasonable and intuitive for XML, but it seems unexpected for YAML.

I don't understand the launch system well enough to propose a PR for the YAML frontend that would allow choices specified as a YAML list/sequence.

I plan to propose a documentation PR for ros2_documentation. This is a nice feature for launch args, and I couldn't find clear documentation of argument choices for Python and XML, I just had a Python example in hand from the UR packages.

danzimmerman commented 1 year ago

I guess this is very similar to param and remap for nodes in the tutorials, where repeated tags in XML map instead to a list of dicts named after that repeated tag:

    remap:
    -
        from: "/input/pose"
        to: "/turtlesim1/turtle1/pose"
    -
        from: "/output/cmd_vel"
        to: "/turtlesim2/turtle1/cmd_vel"

So this is already familiar in YAML files, just feels less ergonomic when the list of dicts only requires value and nothing more.

clalancette commented 1 year ago

This could probably be improved with some custom handling in launch. If you have time to look at that, it would be much appreciated.