WIP Fixes for failing tests.

Distro A, OPSEC #4584. You may have additional rights; please see https://rosmilitary.org/faq/?category=ros-2-license

Signed-off-by: matthew.lanting <[email protected]>

Author: mlanting

launch_ros/launch_ros/actions/node.py

@@ -28,7 +28,6 @@
 from launch.frontend import Parser
 from launch.frontend.type_utils import get_data_type_from_identifier
 
-from launch.launch_context import LaunchContext
 from launch.some_substitutions_type import SomeSubstitutionsType
 
 from launch_ros.descriptions import Node as NodeDescription
@@ -130,15 +129,15 @@ def __init__(
         """
         self.__node_desc = NodeDescription(node_name=name, node_namespace=namespace,
                                            parameters=parameters, remappings=remappings)
-        ros_exec_kwargs = {'additional_env': additional_env, 'arguments': arguments}
+        ros_exec_kwargs = {'additional_env': additional_env}
         self.__ros_exec = RosExecutable(package=package, executable=executable,
-                                        nodes=[self.__node_desc])
+                                        arguments=arguments, ros_arguments=ros_arguments,
+                                        nodes=[self.__node_desc], **ros_exec_kwargs)
         super().__init__(process_description=self.__ros_exec, **kwargs)
 
     def is_node_name_fully_specified(self):
         return self.__node_desc.is_node_name_fully_specified()
 
-
     @staticmethod
     def parse_nested_parameters(params, parser):
         """Normalize parameters as expected by Node constructor argument."""

launch_ros/launch_ros/descriptions/node.py

@@ -56,8 +56,8 @@
 from launch_ros.utilities import make_namespace_absolute
 from launch_ros.utilities import normalize_parameters
 from launch_ros.utilities import normalize_remap_rules
-from launch_ros.utilities import prefix_namespace
 from launch_ros.utilities import plugin_support
+from launch_ros.utilities import prefix_namespace
 
 from rclpy.validate_namespace import validate_namespace
 from rclpy.validate_node_name import validate_node_name
@@ -89,7 +89,13 @@ def __init__(self):
         super(NodeActionExtension, self).__init__()
         plugin_support.satisfies_version(self.EXTENSION_POINT_VERSION, '^0.1')
 
-    def prepare_for_execute(self, context, ros_specific_arguments, ros_executable, node_description):
+    def prepare_for_execute(
+        self,
+        context,
+        ros_specific_arguments,
+        ros_executable,
+        node_description
+    ):
         """
         Perform any actions prior to the node's process being launched.
 
@@ -271,7 +277,7 @@ def _get_parameter_rule(self, param: 'Parameter', context: LaunchContext):
 
     def prepare(self, context: LaunchContext, executable: Executable, action: Action) -> None:
         self._perform_substitutions(context, executable)
-        
+
         # Prepare any traits which may be defined for this node
         if self.__traits is not None:
             for trait in self.__traits:
@@ -374,7 +380,7 @@ def _perform_substitutions(self, context: LaunchContext, executable: Executable)
         # 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: Dict[str, Union[str, List[str]]] = {}
-        original_name_prefix = ""
+        original_name_prefix = ''
         if self.__expanded_original_node_name is not self.UNSPECIFIED_NODE_NAME:
             original_name_prefix = '{}:'.format(self.__expanded_original_node_name)
         if self.__node_name is not None:
@@ -386,6 +392,7 @@ def _perform_substitutions(self, context: LaunchContext, executable: Executable)
                 original_name_prefix, self.__expanded_node_namespace
             )
         # Give extensions a chance to prepare for execution
+        cmd_extension = []
         for extension in self.__extensions.values():
             cmd_extension, ros_specific_arguments = extension.prepare_for_execute(
                 context,

launch_ros/launch_ros/descriptions/ros_executable.py

@@ -43,6 +43,8 @@ def __init__(
             executable: Optional[SomeSubstitutionsType] = None,
             package: Optional[SomeSubstitutionsType] = None,
             nodes: Iterable[Node] = None,
+            ros_arguments: Optional[Iterable[SomeSubstitutionsType]] = None,
+            arguments: Optional[Iterable[SomeSubstitutionsType]] = None,
             **kwargs
     ) -> None:
         """
@@ -58,10 +60,13 @@ def __init__(
         else:
             cmd = [executable]
 
+        if ros_arguments is not None:
+            arguments += ['--ros-args'] + ros_arguments
+
         self.__package = package
         self.__executable = executable
         self.__nodes = nodes
-        super().__init__(cmd=cmd, **kwargs)
+        super().__init__(cmd=cmd, arguments=arguments, **kwargs)
 
     @property
     def package(self):

test_launch_ros/test/test_launch_ros/actions/test_lifecycle_node.py

@@ -52,5 +52,5 @@ def test_node_name():
     )
     lc = LaunchContext()
     for node in node_object.ros_exec.nodes:
-        node._perform_substitutions(lc, [])
+        node._perform_substitutions(lc, node_object.ros_exec)
     assert node_object.is_node_name_fully_specified() is True

test_launch_ros/test/test_launch_ros/actions/test_node.py

@@ -195,11 +195,11 @@ def test_launch_node_with_parameter_descriptions(self):
             name, value = item.split(':=')
             parameters.append((name, yaml.safe_load(value)))
         assert parameters == [
-            ('param1', 'param1_value'),
-            ('param2', ['param2', '_value']),
-            ('param_group1.list_params', [1.2, 3.4]),
-            ('param_group1.param_group2.param2_values', ['param2_value']),
-            ('param3', ''),
+            ('my_node:param1', 'param1_value'),
+            ('my_node:param2', ['param2', '_value']),
+            ('my_node:param_group1.list_params', [1.2, 3.4]),
+            ('my_node:param_group1.param_group2.param2_values', ['param2_value']),
+            ('my_node:param3', ''),
         ]
 
     def test_launch_node_with_parameter_dict(self):
@@ -361,5 +361,5 @@ def get_test_node_name_parameters():
 def test_node_name(node_object, expected_result):
     lc = LaunchContext()
     for node in node_object.ros_exec.nodes:
-        node._perform_substitutions(lc, [])
+        node._perform_substitutions(lc, node_object.ros_exec)
     assert node_object.is_node_name_fully_specified() is expected_result

test_launch_ros/test/test_launch_ros/actions/test_push_ros_namespace.py

@@ -121,14 +121,14 @@ def test_push_ros_namespace(config):
     if config.second_push_ns is not None:
         pns2 = PushRosNamespace(config.second_push_ns)
         pns2.execute(lc)
-    node = Node(
+    node_object = Node(
         package='dont_care',
         executable='whatever',
         namespace=config.node_ns,
         name=config.node_name
     )
     for node in node_object.ros_exec.nodes:
-        node._perform_substitutions(lc, [])
+        node._perform_substitutions(lc, node_object.ros_exec)
     expected_ns = (
         config.expected_ns if config.expected_ns is not None else
         NodeDescription.UNSPECIFIED_NODE_NAMESPACE

test_launch_ros/test/test_launch_ros/actions/test_set_parameter.py

@@ -120,9 +120,9 @@ def test_set_param_with_node():
     set_param = SetParameter(name='my_param', value='my_value')
     set_param.execute(lc)
     for node_instance in node.ros_exec.nodes:
-        node_instance._perform_substitutions(lc, [])
+        node_instance._perform_substitutions(lc, node.ros_exec)
     actual_command = [perform_substitutions(lc, item) for item in
-                      node.cmd if type(item[0]) == TextSubstitution]
+                      node.ros_exec.cmd if type(item[0]) == TextSubstitution]
     assert actual_command.count('--params-file') == 1
     assert actual_command.count('-p') == 1
 

test_launch_ros/test/test_launch_ros/actions/test_set_parameters_from_file.py

@@ -16,6 +16,8 @@
 
 import os
 
+from _collections import defaultdict
+
 from launch import LaunchContext
 from launch.actions import PopLaunchConfigurations
 from launch.actions import PushLaunchConfigurations
@@ -34,6 +36,14 @@ class MockContext:
 
     def __init__(self):
         self.launch_configurations = {}
+        self.locals = lambda: None
+        self.locals.unique_ros_node_names = defaultdict(int)
+
+    def extend_globals(self, val):
+        pass
+
+    def extend_locals(self, val):
+        pass
 
     def perform_substitution(self, sub):
         return sub.perform(None)
@@ -74,14 +84,18 @@ def test_set_param_with_node():
     set_param_single = SetParameter(name='my_param', value='my_value')
     set_param_file.execute(lc)
     set_param_single.execute(lc)
-    node_1._perform_substitutions(lc)
-    node_2._perform_substitutions(lc)
+    # node_1._perform_substitutions(lc)
+    for node_description in node_1.ros_exec.nodes:
+        node_description._perform_substitutions(lc, node_1.ros_exec)
+    # node_2._perform_substitutions(lc)
+    for node_description in node_2.ros_exec.nodes:
+        node_description._perform_substitutions(lc, node_2.ros_exec)
 
     actual_command_1 = [perform_substitutions(lc, item) for item in
-                        node_1.cmd if type(item[0]) == TextSubstitution]
+                        node_1.ros_exec.cmd if type(item[0]) == TextSubstitution]
 
     actual_command_2 = [perform_substitutions(lc, item) for item in
-                        node_2.cmd if type(item[0]) == TextSubstitution]
+                        node_2.ros_exec.cmd if type(item[0]) == TextSubstitution]
 
     assert actual_command_1[3] == '--params-file'
     assert os.path.isfile(actual_command_1[4])

test_launch_ros/test/test_launch_ros/actions/test_set_remap.py

@@ -94,7 +94,7 @@ def test_set_remap_with_node():
     set_remap = SetRemap('from1', 'to1')
     set_remap.execute(lc)
     for node_instance in node.ros_exec.nodes:
-        node_instance._perform_substitutions(lc, [])
+        node_instance._perform_substitutions(lc, node.ros_exec)
     assert len(node.expanded_remapping_rules) == 2
     assert node.expanded_remapping_rules == [('from1', 'to1'), ('from2', 'to2')]