Add a way to set remapping rules for all nodes in the same scope (#163) (#203)

Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Ivan Santiago Paunovic <[email protected]>

Author: jacobperron

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',
 ]

launch_ros/launch_ros/actions/load_composable_nodes.py

@@ -200,12 +200,17 @@ def get_composable_node_load_request(
     if combined_ns is not None:
         request.node_namespace = combined_ns
     # 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:

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
@@ -195,14 +194,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)
@@ -211,7 +202,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
@@ -401,12 +392,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]]:
         """
@@ -422,13 +422,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)
 
@@ -448,3 +441,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

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

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'