ros2 / launch

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

[let] action does not match spec #595

Closed oceanusxiv closed 2 years ago

oceanusxiv commented 2 years ago

Bug report

Required Info:

Steps to reproduce issue

Follow the documentation in https://docs.ros.org/en/galactic/How-To-Guides/Launch-files-migration-guide.html#new-tags-in-ros-2 and produce such a launch file

<launch version="0.1.1">
    <let var="foo" value="asd"/>
</launch>

and run it with ros2 launch example.launch.xml

Expected behavior

No error

Actual behavior

Task exception was never retrieved
future: <Task finished name='Task-2' coro=<LaunchService._process_one_event() done, defined at /opt/ros/galactic/lib/python3.8/site-packages/launch/launch_service.py:226> exception=InvalidLaunchFileError('xml')>
Traceback (most recent call last):
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/launch_description_sources/any_launch_file_utilities.py", line 53, in get_launch_description_from_any_launch_file
    return loader(launch_file_path)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/launch_description_sources/frontend_launch_file_utilities.py", line 35, in get_launch_description_from_frontend_launch_file
    return parser.parse_description(root_entity)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/frontend/parser.py", line 111, in parse_description
    actions = [self.parse_action(child) for child in entity.children]
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/frontend/parser.py", line 111, in <listcomp>
    actions = [self.parse_action(child) for child in entity.children]
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/frontend/parser.py", line 88, in parse_action
    return instantiate_action(entity, self)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/frontend/expose.py", line 39, in instantiate_action
    action_type, kwargs = action_parse_methods[entity.type_name](entity, parser)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/frontend/expose.py", line 105, in wrapper
    ret = found_parse_method(entity, parser)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/actions/set_launch_configuration.py", line 54, in parse
    name = parser.parse_substitution(entity.get_attr('name'))
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch_xml/entity.py", line 119, in get_attr
    raise attr_error
AttributeError: Attribute name of type <class 'str'> not found in Entity let

Additional information

The above conforms to the spec described in https://design.ros2.org/articles/roslaunch_xml.html#substitution-syntax.

The actual implementation does not match the spec, where instead of var, what is actually implemented uses name. Which can be seen in https://github.com/ros2/launch/blob/13d0c28a86b1975411f2b5fb54bc51a0ff66113c/launch/launch/actions/set_launch_configuration.py#L54

In fact the test also explicitly violates the spec in: https://github.com/ros2/launch/blob/13d0c28a86b1975411f2b5fb54bc51a0ff66113c/launch_xml/test/launch_xml/test_let_var.py#L29

clalancette commented 2 years ago

We discussed it, and it looks like the implementation is what we are going for. Therefore, I'll leave https://github.com/ros2/ros2_documentation/issues/2322 open to update the documentation and the spec.