Launch file glob syntax compatibility with multiple ROS 2 versions

jtbandes commented 3 years ago

In ROS 1 versions of this package, launch files use:

https://github.com/RobotWebTools/rosbridge_suite/blob/a09a964fb5956321aca3b296da367e21d3d2e044/rosbridge_server/launch/rosbridge_websocket.launch#L24

In the original ROS 2 version, this arg value was wrapped in double single-quotes, ''[*]''.

In #574 I changed it to single single-quotes for Galactic:

- <arg name="topics_glob" default="''[*]''" />
+ <arg name="topics_glob" default="'[*]'" />

'[*]' works in Galactic, but it apparently broke Foxy.

The ''[*]'' version apparently works pre-Galactic, but in Galactic I get the following error:

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=ValueError('Failed to convert \'\'\'[*]\'\'\' using yaml rules: yaml.safe_load() failed\nwhile scanning an alias\n  in "<unicode string>", line 1, column 4:\n    \'\'[*]\'\'\n       ^\nexpected alphabetic or numeric character, but found \']\'\n  in "<unicode string>", line 1, column 5:\n    \'\'[*]\'\'\n        ^')>
Traceback (most recent call last):
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/utilities/type_utils.py", line 237, in convert_as_yaml
    output = yaml.safe_load(value)
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 162, in safe_load
    return load(stream, SafeLoader)
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 114, in load
    return loader.get_single_data()
  File "/usr/lib/python3/dist-packages/yaml/constructor.py", line 49, in get_single_data
    node = self.get_single_node()
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 35, in get_single_node
    if not self.check_event(StreamEndEvent):
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 98, in check_event
    self.current_event = self.state()
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 142, in parse_implicit_document_start
    if not self.check_token(DirectiveToken, DocumentStartToken,
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 116, in check_token
    self.fetch_more_tokens()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 227, in fetch_more_tokens
    return self.fetch_alias()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 610, in fetch_alias
    self.tokens.append(self.scan_anchor(AliasToken))
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 922, in scan_anchor
    raise ScannerError("while scanning an %s" % name, start_mark,
yaml.scanner.ScannerError: while scanning an alias
  in "<unicode string>", line 1, column 4:
    ''[*]''
       ^
expected alphabetic or numeric character, but found ']'
  in "<unicode string>", line 1, column 5:
    ''[*]''
        ^

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/launch_service.py", line 228, in _process_one_event
    await self.__process_event(next_event)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/launch_service.py", line 248, in __process_event
    visit_all_entities_and_collect_futures(entity, self.__context))
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  [Previous line repeated 2 more times]
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 38, in visit_all_entities_and_collect_futures
    sub_entities = entity.visit(context)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/action.py", line 108, in visit
    return self.execute(context)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch_ros/actions/node.py", line 465, in execute
    self._perform_substitutions(context)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch_ros/actions/node.py", line 420, in _perform_substitutions
    evaluated_parameters = evaluate_parameters(context, self.__parameters)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch_ros/utilities/evaluate_parameters.py", line 151, in evaluate_parameters
    output_params.append(evaluate_parameter_dict(context, param))
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch_ros/utilities/evaluate_parameters.py", line 120, in evaluate_parameter_dict
    evaluated_value = value.evaluate(context)
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch_ros/parameter_descriptions.py", line 91, in evaluate
    self.__evaluated_parameter_value = perform_typed_substitution(
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/utilities/type_utils.py", line 545, in perform_typed_substitution
    return coerce_to_type(
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/utilities/type_utils.py", line 252, in coerce_to_type
    return convert_as_yaml(value, f"Failed to convert '{value}' using yaml rules")
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/utilities/type_utils.py", line 241, in convert_as_yaml
    raise ValueError(f'{error_msg}: yaml.safe_load() failed\n{err}')
ValueError: Failed to convert '''[*]''' using yaml rules: yaml.safe_load() failed
while scanning an alias
  in "<unicode string>", line 1, column 4:
    ''[*]''
       ^
expected alphabetic or numeric character, but found ']'
  in "<unicode string>", line 1, column 5:
    ''[*]''
        ^

Ideally we can find a syntax that's compatible with all ROS 2 versions, since this repo currently only has one ros2 branch.

@XDGFX you reported this issue in https://github.com/ros2/rviz/issues/720#issuecomment-897520027 — do you happen to know why the 2x-single-quote syntax works in Foxy? I'm trying to understand the original reason for the change and what we can do to make this work across versions.

jtbandes commented 3 years ago

Just thinking out loud here...

I don't see anything relevant in https://docs.ros.org/en/galactic/Guides/Launch-files-migration-guide.html.
According to the YAML spec, '' is how you escape single quotes inside an already single-quoted string. But this isn't in a single-quoted string...so why is it valid pre-Galactic?
In addition to XML, ROS 2 launch files can also be written in Python and YAML — so maybe we should just convert the launch file to one of these?

jtbandes commented 3 years ago

I was able to reproduce the error in Foxy, which happens with both [*] and '[*]':

future: <Task finished name='Task-2' coro=<LaunchService._process_one_event() done, defined at /opt/ros/foxy/lib/python3.8/site-packages/launch/launch_service.py:271> exception=ScannerError('while scanning an alias', <yaml.error.Mark object at 0xffffb2d13910>, "expected alphabetic or numeric character, but found ']'", <yaml.error.Mark object at 0xffffb2d139a0>)>
Traceback (most recent call last):
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/launch_service.py", line 273, in _process_one_event
    await self.__process_event(next_event)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/launch_service.py", line 293, in __process_event
    visit_all_entities_and_collect_futures(entity, self.__context))
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 45, in visit_all_entities_and_collect_futures
    futures_to_return += visit_all_entities_and_collect_futures(sub_entity, context)
  [Previous line repeated 1 more time]
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 38, in visit_all_entities_and_collect_futures
    sub_entities = entity.visit(context)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/action.py", line 108, in visit
    return self.execute(context)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch_ros/actions/node.py", line 417, in execute
    self._perform_substitutions(context)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch_ros/actions/node.py", line 377, in _perform_substitutions
    evaluated_parameters = evaluate_parameters(context, self.__parameters)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch_ros/utilities/evaluate_parameters.py", line 145, in evaluate_parameters
    output_params.append(evaluate_parameter_dict(context, param))
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch_ros/utilities/evaluate_parameters.py", line 70, in evaluate_parameter_dict
    yaml_evaluated_value = yaml.safe_load(evaluated_value)
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 162, in safe_load
    return load(stream, SafeLoader)
  File "/usr/lib/python3/dist-packages/yaml/__init__.py", line 114, in load
    return loader.get_single_data()
  File "/usr/lib/python3/dist-packages/yaml/constructor.py", line 49, in get_single_data
    node = self.get_single_node()
  File "/usr/lib/python3/dist-packages/yaml/composer.py", line 35, in get_single_node
    if not self.check_event(StreamEndEvent):
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 98, in check_event
    self.current_event = self.state()
  File "/usr/lib/python3/dist-packages/yaml/parser.py", line 142, in parse_implicit_document_start
    if not self.check_token(DirectiveToken, DocumentStartToken,
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 116, in check_token
    self.fetch_more_tokens()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 227, in fetch_more_tokens
    return self.fetch_alias()
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 610, in fetch_alias
    self.tokens.append(self.scan_anchor(AliasToken))
  File "/usr/lib/python3/dist-packages/yaml/scanner.py", line 922, in scan_anchor
    raise ScannerError("while scanning an %s" % name, start_mark,
yaml.scanner.ScannerError: while scanning an alias
  in "<unicode string>", line 1, column 2:
    [*]
     ^
expected alphabetic or numeric character, but found ']'
  in "<unicode string>", line 1, column 3:
    [*]
      ^

jtbandes commented 3 years ago

I think simply removing the [*] as the default value is a viable solution. It looks like the code treats an empty string / glob list as matching all topics:

https://github.com/RobotWebTools/rosbridge_suite/blob/8ffd4a4b1b17b66e572445b05a6d8edfd58c2d24/rosapi/src/rosapi/glob_helper.py#L32-L35

RobotWebTools / rosbridge_suite

Launch file glob syntax compatibility with multiple ROS 2 versions #588