From 606eaee60db693c22c1afff4f5dd6c794084e81e Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Mon, 5 Aug 2019 15:02:45 -0300 Subject: [PATCH] Use node namespace if no other was specified (#51) Signed-off-by: ivanpauno --- launch_ros/launch_ros/actions/node.py | 48 +++++++++++-------- .../launch_ros/actions/push_ros_namespace.py | 25 ++++++++-- .../actions/test_push_ros_namespace.py | 19 +++++--- 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/launch_ros/launch_ros/actions/node.py b/launch_ros/launch_ros/actions/node.py index 92e6c016..1846f6ff 100644 --- a/launch_ros/launch_ros/actions/node.py +++ b/launch_ros/launch_ros/actions/node.py @@ -17,11 +17,14 @@ import os import pathlib from tempfile import NamedTemporaryFile +from typing import cast +from typing import Dict from typing import Iterable from typing import List from typing import Optional from typing import Text # noqa: F401 from typing import Tuple # noqa: F401 +from typing import Union from launch.action import Action from launch.actions import ExecuteProcess @@ -126,35 +129,28 @@ def __init__( cmd += [] if arguments is None else arguments # Reserve space for ros specific arguments. # The substitutions will get expanded when the action is executed. - ros_args_index = 0 if node_name is not None: cmd += [LocalSubstitution( - 'ros_specific_arguments[{}]'.format(ros_args_index), description='node name')] - ros_args_index += 1 - cmd += [LocalSubstitution( - 'ros_specific_arguments[{}]'.format(ros_args_index), description='node namespace')] - ros_args_index += 1 + "ros_specific_arguments['name']", description='node name')] if parameters is not None: ensure_argument_type(parameters, (list), 'parameters', 'Node') # All elements in the list are paths to files with parameters (or substitutions that # evaluate to paths), or dictionaries of parameters (fields can be substitutions). i = 0 for param in parameters: - i += 1 cmd += [LocalSubstitution( - 'ros_specific_arguments[{}]'.format(ros_args_index), + "ros_specific_arguments['params'][{}]".format(i), description='parameter {}'.format(i))] - ros_args_index += 1 + i += 1 normalized_params = normalize_parameters(parameters) if remappings is not None: i = 0 for remapping in normalize_remap_rules(remappings): k, v = remapping - i += 1 cmd += [LocalSubstitution( - 'ros_specific_arguments[{}]'.format(ros_args_index), + "ros_specific_arguments['remaps'][{}]".format(i), description='remapping {}'.format(i))] - ros_args_index += 1 + i += 1 super().__init__(cmd=cmd, **kwargs) self.__package = package self.__node_executable = node_executable @@ -287,9 +283,16 @@ def _perform_substitutions(self, context: LaunchContext) -> None: self.__expanded_node_namespace = ( base_ns + '/' + self.__expanded_node_namespace ).rstrip('/') - if not self.__expanded_node_namespace.startswith('/'): + if ( + self.__expanded_node_namespace != '' and not + self.__expanded_node_namespace.startswith('/') + ): self.__expanded_node_namespace = '/' + self.__expanded_node_namespace - validate_namespace(self.__expanded_node_namespace) + if self.__expanded_node_namespace != '': + self.cmd.append(normalize_to_list_of_substitutions([ + LocalSubstitution("ros_specific_arguments['ns']") + ])) + validate_namespace(self.__expanded_node_namespace) except Exception: self.__logger.error( "Error while expanding or validating node name or namespace for '{}':" @@ -340,16 +343,23 @@ def execute(self, context: LaunchContext) -> Optional[List[Action]]: self._perform_substitutions(context) # Prepare the ros_specific_arguments list and add it to the context so that the # LocalSubstitution placeholders added to the the cmd can be expanded using the contents. - ros_specific_arguments = [] # type: List[Text] + ros_specific_arguments: Dict[str, Union[str, List[str]]] = {} if self.__node_name is not None: - ros_specific_arguments.append('__node:={}'.format(self.__expanded_node_name)) - ros_specific_arguments.append('__ns:={}'.format(self.__expanded_node_namespace)) + ros_specific_arguments['name'] = '__node:={}'.format(self.__expanded_node_name) + if self.__expanded_node_namespace != '': + ros_specific_arguments['ns'] = '__ns:={}'.format(self.__expanded_node_namespace) if self.__expanded_parameter_files is not None: + ros_specific_arguments['params'] = [] + param_arguments = cast(List[str], ros_specific_arguments['params']) for param_file_path in self.__expanded_parameter_files: - ros_specific_arguments.append('__params:={}'.format(param_file_path)) + param_arguments.append('__params:={}'.format(param_file_path)) if self.__expanded_remappings is not None: + ros_specific_arguments['remaps'] = [] for remapping_from, remapping_to in self.__expanded_remappings: - ros_specific_arguments.append('{}:={}'.format(remapping_from, remapping_to)) + remap_arguments = cast(List[str], ros_specific_arguments['remaps']) + remap_arguments.append( + '{}:={}'.format(remapping_from, remapping_to) + ) context.extend_locals({'ros_specific_arguments': ros_specific_arguments}) return super().execute(context) diff --git a/launch_ros/launch_ros/actions/push_ros_namespace.py b/launch_ros/launch_ros/actions/push_ros_namespace.py index 54694f2a..81791844 100644 --- a/launch_ros/launch_ros/actions/push_ros_namespace.py +++ b/launch_ros/launch_ros/actions/push_ros_namespace.py @@ -22,9 +22,12 @@ from launch.frontend import Parser from launch.launch_context import LaunchContext from launch.some_substitutions_type import SomeSubstitutionsType +from launch.substitutions import SubstitutionFailure from launch.utilities import normalize_to_list_of_substitutions from launch.utilities import perform_substitutions +from rclpy.validate_namespace import validate_namespace + class PushRosNamespace(Action): """ @@ -57,8 +60,20 @@ def namespace(self) -> List[Substitution]: def execute(self, context: LaunchContext): """Execute the action.""" - namespace = perform_substitutions(context, self.namespace) - if not namespace.startswith('/'): - namespace = context.launch_configurations.get('ros_namespace', '') \ - + '/' + namespace - context.launch_configurations['ros_namespace'] = namespace.rstrip('/') + pushed_namespace = perform_substitutions(context, self.namespace) + previous_namespace = context.launch_configurations.get('ros_namespace', '') + namespace = pushed_namespace + if not pushed_namespace.startswith('/'): + namespace = previous_namespace + '/' + pushed_namespace + namespace = namespace.rstrip('/') + if namespace != '': + try: + validate_namespace(namespace) + except Exception: + raise SubstitutionFailure( + 'The resulting namespace is invalid:' + " [previous_namespace='{}', pushed_namespace='{}']".format( + previous_namespace, pushed_namespace + ) + ) + context.launch_configurations['ros_namespace'] = namespace diff --git a/test_launch_ros/test/test_launch_ros/actions/test_push_ros_namespace.py b/test_launch_ros/test/test_launch_ros/actions/test_push_ros_namespace.py index 562f169b..567c0253 100644 --- a/test_launch_ros/test/test_launch_ros/actions/test_push_ros_namespace.py +++ b/test_launch_ros/test/test_launch_ros/actions/test_push_ros_namespace.py @@ -27,9 +27,9 @@ class Config: def __init__( self, *, - push_ns, - node_ns, - expected_ns, + node_ns='', + push_ns=None, + expected_ns=None, second_push_ns=None ): self.push_ns = push_ns @@ -65,11 +65,14 @@ def __init__( second_push_ns='/absolute_ns', node_ns='node_ns', expected_ns='/absolute_ns/node_ns'), + Config(), + Config(push_ns=''), )) def test_push_ros_namespace(config): lc = LaunchContext() - pns1 = PushRosNamespace(config.push_ns) - pns1.execute(lc) + if config.push_ns is not None: + pns1 = PushRosNamespace(config.push_ns) + pns1.execute(lc) if config.second_push_ns is not None: pns2 = PushRosNamespace(config.second_push_ns) pns2.execute(lc) @@ -79,5 +82,7 @@ def test_push_ros_namespace(config): node_namespace=config.node_ns, ) node._perform_substitutions(lc) - assert node.expanded_node_namespace == config.expected_ns - assert 2 == len(node.cmd) + expected_cmd_len = 2 if config.expected_ns is not None else 1 + assert expected_cmd_len == len(node.cmd) + expected_ns = config.expected_ns if config.expected_ns is not None else '' + assert expected_ns == node.expanded_node_namespace