Make namespace parameter mandatory in LifecycleNode constructor (#157)

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

Author: ivanpauno

launch_ros/examples/lifecycle_pub_sub_launch.py

@@ -36,7 +36,7 @@ def main(argv=sys.argv[1:]):
 
     # Prepare the talker node.
     talker_node = launch_ros.actions.LifecycleNode(
-        node_name='talker',
+        name='talker', namespace='',
         package='lifecycle', executable='lifecycle_talker', output='screen')
 
     # When the talker reaches the 'inactive' state, make it take the 'activate' transition.
@@ -62,7 +62,7 @@ def main(argv=sys.argv[1:]):
                 launch.actions.LogInfo(
                     msg="node 'talker' reached the 'active' state, launching 'listener'."),
                 launch_ros.actions.LifecycleNode(
-                    node_name='listener',
+                    name='listener', namespace='',
                     package='lifecycle', executable='lifecycle_listener', output='screen'),
             ],
         )

launch_ros/launch_ros/actions/lifecycle_node.py

@@ -19,10 +19,10 @@
 from typing import cast
 from typing import List
 from typing import Optional
-from typing import Text
 import warnings
 
 import launch
+from launch import SomeSubstitutionsType
 from launch.action import Action
 import launch.logging
 
@@ -42,8 +42,9 @@ class LifecycleNode(Node):
     def __init__(
         self,
         *,
-        name: Optional[Text] = None,
-        node_name: Optional[Text] = None,
+        name: Optional[SomeSubstitutionsType] = None,
+        namespace: SomeSubstitutionsType,
+        node_name: Optional[SomeSubstitutionsType] = None,
         **kwargs
     ) -> None:
         """
@@ -87,7 +88,7 @@ def __init__(
         # TODO(jacobperron): Remove default value and this check when deprecated API is removed
         if name is None:
             raise RuntimeError("'name' must not be None.'")
-        super().__init__(name=name, **kwargs)
+        super().__init__(name=name, namespace=namespace, **kwargs)
         self.__logger = launch.logging.get_logger(__name__)
         self.__rclpy_subscription = None
         self.__current_state = \

test_launch_ros/test/test_launch_ros/actions/test_lifecycle_node.py

@@ -0,0 +1,57 @@
+# 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 LifecycleNode Action."""
+
+from launch import LaunchContext
+from launch_ros.actions import LifecycleNode
+
+import pytest
+
+
+def test_lifecycle_node_constructor():
+    # Construction without namespace
+    with pytest.raises(TypeError):
+        LifecycleNode(
+            package='asd',
+            executable='bsd',
+            name='my_node',
+        )
+    # Construction without name
+    # TODO(ivanpauno): This should raise TypeError.
+    with pytest.raises(RuntimeError):
+        LifecycleNode(
+            package='asd',
+            executable='bsd',
+            namespace='my_ns',
+        )
+    # Successfull construction
+    LifecycleNode(
+        package='asd',
+        executable='bsd',
+        name='my_node',
+        namespace='my_ns',
+    )
+
+
+def test_node_name():
+    node_object = LifecycleNode(
+        package='asd',
+        executable='bsd',
+        name='my_node',
+        namespace='my_ns',
+    )
+    lc = LaunchContext()
+    node_object._perform_substitutions(lc)
+    assert node_object.is_node_name_fully_specified() is True

test_launch_ros/test/test_launch_ros/actions/test_node.py

@@ -19,11 +19,13 @@
 import unittest
 import warnings
 
+from launch import LaunchContext
 from launch import LaunchDescription
 from launch import LaunchService
 from launch.actions import Shutdown
 from launch.substitutions import EnvironmentVariable
 import launch_ros.actions.node
+import pytest
 import yaml
 
 
@@ -44,8 +46,6 @@ def _assert_launch_no_errors(self, actions):
     def _create_node(self, *, parameters=None, remappings=None):
         return launch_ros.actions.Node(
             package='demo_nodes_py', executable='talker_qos', output='screen',
-            # The node name is required for parameter dicts.
-            # See https://github.com/ros2/launch/issues/139.
             name='my_node', namespace='my_ns',
             exec_name='my_node_process',
             arguments=['--number_of_cycles', '1'],
@@ -300,3 +300,46 @@ def test_launch_node_with_invalid_parameter_dicts(self):
                     },
                 },
             }])
+
+
+def get_test_node_name_parameters():
+    return [
+        pytest.param(
+            launch_ros.actions.Node(
+                package='asd',
+                executable='bsd',
+                name='my_node',
+            ),
+            False,
+            id='Node without namespace'
+        ),
+        pytest.param(
+            launch_ros.actions.Node(
+                package='asd',
+                executable='bsd',
+                namespace='my_ns',
+            ),
+            False,
+            id='Node without name'
+        ),
+        pytest.param(
+            launch_ros.actions.Node(
+                package='asd',
+                executable='bsd',
+                name='my_node',
+                namespace='my_ns',
+            ),
+            True,
+            id='Node with fully qualified name'
+        ),
+    ]
+
+
+@pytest.mark.parametrize(
+    'node_object, expected_result',
+    get_test_node_name_parameters()
+)
+def test_node_name(node_object, expected_result):
+    lc = LaunchContext()
+    node_object._perform_substitutions(lc)
+    assert node_object.is_node_name_fully_specified() is expected_result