From b8455043670ed832a9105bbdde5a25ba7becb42b Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 24 Jul 2020 17:59:16 -0300 Subject: [PATCH] Add a way to set remapping rules for all nodes in the same scope (#163) Signed-off-by: Ivan Santiago Paunovic --- launch_ros/launch_ros/actions/__init__.py | 3 + .../actions/load_composable_nodes.py | 17 ++- launch_ros/launch_ros/actions/node.py | 42 ++++--- launch_ros/launch_ros/actions/set_remap.py | 91 +++++++++++++++ .../test_launch_ros/actions/test_set_remap.py | 106 ++++++++++++++++++ 5 files changed, 231 insertions(+), 28 deletions(-) create mode 100644 launch_ros/launch_ros/actions/set_remap.py create mode 100644 test_launch_ros/test/test_launch_ros/actions/test_set_remap.py diff --git a/launch_ros/launch_ros/actions/__init__.py b/launch_ros/launch_ros/actions/__init__.py index e4367436..78655416 100644 --- a/launch_ros/launch_ros/actions/__init__.py +++ b/launch_ros/launch_ros/actions/__init__.py @@ -20,6 +20,8 @@ from .node import Node from .push_ros_namespace import PushRosNamespace from .set_parameter import SetParameter +from .set_remap import SetRemap + __all__ = [ 'ComposableNodeContainer', @@ -28,4 +30,5 @@ 'Node', 'PushRosNamespace', 'SetParameter', + 'SetRemap', ] diff --git a/launch_ros/launch_ros/actions/load_composable_nodes.py b/launch_ros/launch_ros/actions/load_composable_nodes.py index b9e2d5d0..cc1bca89 100644 --- a/launch_ros/launch_ros/actions/load_composable_nodes.py +++ b/launch_ros/launch_ros/actions/load_composable_nodes.py @@ -195,12 +195,17 @@ def get_composable_node_load_request( context, composable_node_description.node_namespace ) # request.log_level = perform_substitutions(context, node_description.log_level) - if composable_node_description.remappings is not None: - for from_, to in composable_node_description.remappings: - request.remap_rules.append('{}:={}'.format( - perform_substitutions(context, list(from_)), - perform_substitutions(context, list(to)), - )) + remappings = [] + global_remaps = context.launch_configurations.get('ros_remaps', None) + if global_remaps: + remappings.extend([f'{src}:={dst}' for src, dst in global_remaps]) + if composable_node_description.remappings: + remappings.extend([ + f'{perform_substitutions(context, src)}:={perform_substitutions(context, dst)}' + for src, dst in composable_node_description.remappings + ]) + if remappings: + request.remap_rules = remappings global_params = context.launch_configurations.get('ros_params', None) parameters = [] if global_params is not None: diff --git a/launch_ros/launch_ros/actions/node.py b/launch_ros/launch_ros/actions/node.py index 9c92fdfb..6019fda9 100644 --- a/launch_ros/launch_ros/actions/node.py +++ b/launch_ros/launch_ros/actions/node.py @@ -17,7 +17,6 @@ import os import pathlib from tempfile import NamedTemporaryFile -from typing import cast from typing import Dict from typing import Iterable from typing import List @@ -193,14 +192,6 @@ def __init__( # 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). normalized_params = normalize_parameters(parameters) - if remappings is not None: - i = 0 - for remapping in normalize_remap_rules(remappings): - k, v = remapping - cmd += ['-r', LocalSubstitution( - "ros_specific_arguments['remaps'][{}]".format(i), - description='remapping {}'.format(i))] - i += 1 # Forward 'exec_name' as to ExecuteProcess constructor kwargs['name'] = exec_name super().__init__(cmd=cmd, **kwargs) @@ -209,7 +200,7 @@ def __init__( self.__node_name = name self.__node_namespace = namespace self.__parameters = [] if parameters is None else normalized_params - self.__remappings = [] if remappings is None else remappings + self.__remappings = [] if remappings is None else list(normalize_remap_rules(remappings)) self.__arguments = arguments self.__expanded_node_name = self.UNSPECIFIED_NODE_NAME @@ -410,12 +401,21 @@ def _perform_substitutions(self, context: LaunchContext) -> None: cmd_extension = ['--params-file', f'{param_file_path}'] self.cmd.extend([normalize_to_list_of_substitutions(x) for x in cmd_extension]) # expand remappings too - if self.__remappings is not None: + global_remaps = context.launch_configurations.get('ros_remaps', None) + if global_remaps or self.__remappings: self.__expanded_remappings = [] - for k, v in self.__remappings: - key = perform_substitutions(context, normalize_to_list_of_substitutions(k)) - value = perform_substitutions(context, normalize_to_list_of_substitutions(v)) - self.__expanded_remappings.append((key, value)) + if global_remaps: + self.__expanded_remappings.extend(global_remaps) + if self.__remappings: + self.__expanded_remappings.extend([ + (perform_substitutions(context, src), perform_substitutions(context, dst)) + for src, dst in self.__remappings + ]) + if self.__expanded_remappings: + cmd_extension = [] + for src, dst in self.__expanded_remappings: + cmd_extension.extend(['-r', f'{src}:={dst}']) + self.cmd.extend([normalize_to_list_of_substitutions(x) for x in cmd_extension]) def execute(self, context: LaunchContext) -> Optional[List[Action]]: """ @@ -431,13 +431,6 @@ def execute(self, context: LaunchContext) -> Optional[List[Action]]: 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_remappings is not None: - ros_specific_arguments['remaps'] = [] - for remapping_from, remapping_to in self.__expanded_remappings: - 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}) ret = super().execute(context) @@ -457,3 +450,8 @@ def execute(self, context: LaunchContext) -> Optional[List[Action]]: def expanded_node_namespace(self): """Getter for expanded_node_namespace.""" return self.__expanded_node_namespace + + @property + def expanded_remapping_rules(self): + """Getter for expanded_remappings.""" + return self.__expanded_remappings diff --git a/launch_ros/launch_ros/actions/set_remap.py b/launch_ros/launch_ros/actions/set_remap.py new file mode 100644 index 00000000..a57e0dad --- /dev/null +++ b/launch_ros/launch_ros/actions/set_remap.py @@ -0,0 +1,91 @@ +# Copyright 2020 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Module for the `SetRemap` action.""" + +from typing import List + +from launch import Action +from launch import Substitution +from launch.frontend import Entity +from launch.frontend import expose_action +from launch.frontend import Parser +from launch.launch_context import LaunchContext +from launch.some_substitutions_type import SomeSubstitutionsType +from launch.utilities import normalize_to_list_of_substitutions +from launch.utilities import perform_substitutions + + +@expose_action('set_remap') +class SetRemap(Action): + """ + Action that sets a remapping rule in the current context. + + This remapping rule will be passed to all the nodes launched in the same scope, overriding + the ones specified in the `Node` action constructor. + e.g.: + ```python3 + LaunchDescription([ + ..., + GroupAction( + actions = [ + ..., + SetRemap(src='asd', dst='bsd'), + ..., + Node(...), // the remap rule will be passed to this node + ..., + ] + ), + Node(...), // here it won't be passed, as it's not in the same scope + ... + ]) + ``` + """ + + def __init__( + self, + src: SomeSubstitutionsType, + dst: SomeSubstitutionsType, + **kwargs + ) -> None: + """Create a SetRemap action.""" + super().__init__(**kwargs) + self.__src = normalize_to_list_of_substitutions(src) + self.__dst = normalize_to_list_of_substitutions(dst) + + @classmethod + def parse(cls, entity: Entity, parser: Parser): + """Return `SetRemap` action and kwargs for constructing it.""" + _, kwargs = super().parse(entity, parser) + kwargs['src'] = parser.parse_substitution(entity.get_attr('from')) + kwargs['dst'] = parser.parse_substitution(entity.get_attr('to')) + return cls, kwargs + + @property + def src(self) -> List[Substitution]: + """Getter for src.""" + return self.__src + + @property + def dst(self) -> List[Substitution]: + """Getter for dst.""" + return self.__dst + + def execute(self, context: LaunchContext): + """Execute the action.""" + src = perform_substitutions(context, self.__src) + dst = perform_substitutions(context, self.__dst) + global_remaps = context.launch_configurations.get('ros_remaps', []) + global_remaps.append((src, dst)) + context.launch_configurations['ros_remaps'] = global_remaps diff --git a/test_launch_ros/test/test_launch_ros/actions/test_set_remap.py b/test_launch_ros/test/test_launch_ros/actions/test_set_remap.py new file mode 100644 index 00000000..a7c820a3 --- /dev/null +++ b/test_launch_ros/test/test_launch_ros/actions/test_set_remap.py @@ -0,0 +1,106 @@ +# Copyright 2020 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for the SetRemap Action.""" + +from launch import LaunchContext +from launch.actions import PopLaunchConfigurations +from launch.actions import PushLaunchConfigurations + +from launch_ros.actions import Node +from launch_ros.actions import SetRemap +from launch_ros.actions.load_composable_nodes import get_composable_node_load_request +from launch_ros.descriptions import ComposableNode + +import pytest + + +class MockContext: + + def __init__(self): + self.launch_configurations = {} + + def perform_substitution(self, sub): + return sub.perform(None) + + +def get_set_remap_test_remaps(): + return [ + pytest.param( + [('from', 'to')], + id='One remapping rule' + ), + pytest.param( + [('from1', 'to1'), ('from2', 'to2')], + id='Two remapping rules' + ), + ] + + +@pytest.mark.parametrize( + 'remapping_rules', + get_set_remap_test_remaps() +) +def test_set_remap(remapping_rules): + lc = MockContext() + for src, dst in remapping_rules: + SetRemap(src, dst).execute(lc) + assert lc.launch_configurations == {'ros_remaps': remapping_rules} + + +def test_set_remap_is_scoped(): + lc = LaunchContext() + push_conf = PushLaunchConfigurations() + pop_conf = PopLaunchConfigurations() + set_remap = SetRemap('from', 'to') + + push_conf.execute(lc) + set_remap.execute(lc) + assert lc.launch_configurations == {'ros_remaps': [('from', 'to')]} + pop_conf.execute(lc) + assert lc.launch_configurations == {} + + +def test_set_remap_with_node(): + lc = MockContext() + node = Node( + package='asd', + executable='bsd', + name='my_node', + namespace='my_ns', + remappings=[('from2', 'to2')] + ) + set_remap = SetRemap('from1', 'to1') + set_remap.execute(lc) + node._perform_substitutions(lc) + assert len(node.expanded_remapping_rules) == 2 + assert node.expanded_remapping_rules == [('from1', 'to1'), ('from2', 'to2')] + + +def test_set_remap_with_composable_node(): + lc = MockContext() + node_description = ComposableNode( + package='asd', + plugin='my_plugin', + name='my_node', + namespace='my_ns', + remappings=[('from2', 'to2')] + ) + set_remap = SetRemap('from1', 'to1') + set_remap.execute(lc) + request = get_composable_node_load_request(node_description, lc) + remappings = request.remap_rules + assert len(remappings) == 2 + assert remappings[0] == 'from1:=to1' + assert remappings[1] == 'from2:=to2'