Use node namespace if no other was specified (#51)

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

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)
 

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

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