Fixes & improments following the code review comments.

* Update the meta info in package.xml and setup.py
* Try-except executor.spin instead of main in executables
* Use f string to format infos
* Pass error message to response objectc instead of logging it
* Use node's logger instead of init a global one
* Call node.get_qualified_name() to assign response obj's full_node_name
* Other super tiny changes

Signed-off-by: Zhen Ju <[email protected]>

rclpy_components/package.xml

@@ -2,17 +2,18 @@
 <?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="3">
   <name>rclpy_components</name>
-  <version>0.0.0</version>
-  <description>TODO: Package description</description>
-  <maintainer email="[email protected]">root</maintainer>
-  <license>TODO: License declaration</license>
+  <version>1.1.0</version>
+  <description>The dynamic node management package</description>
+  <maintainer email="[email protected]">Zhen Ju</maintainer>
+  <license>Apache License 2.0</license>
 
   <depend>rclpy</depend>
-  <depend>std_msgs</depend>
+  <depend>composition_interfaces</depend>
 
   <test_depend>ament_copyright</test_depend>
   <test_depend>ament_flake8</test_depend>
   <test_depend>ament_pep257</test_depend>
+  <test_depend>composition_interfaces</test_depend>
   <test_depend>python3-pytest</test_depend>
 
   <export>

rclpy_components/rclpy_components/__init__.py

@@ -10,4 +10,4 @@
 # 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.
+# limitations under the License.

rclpy_components/rclpy_components/component_container.py

@@ -12,29 +12,25 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import signal
 import rclpy
 from rclpy.executors import SingleThreadedExecutor
 from .component_manager import ComponentManager
 
 
 def main():
-    try:
-        _main()
-    except KeyboardInterrupt:
-        print('KeyboardInterrupt received, exit')
-        return signal.SIGINT
-
-
-def _main():
     rclpy.init()
 
     executor = SingleThreadedExecutor()
     component_manager = ComponentManager(executor, "PyComponentManager")
 
     executor.add_node(component_manager)
-    executor.spin()
+    try:
+        executor.spin()
+    except KeyboardInterrupt:
+        print('KeyboardInterrupt received, exit')
+        pass
 
+    component_manager.destroy_node()
     rclpy.shutdown()
 
 

rclpy_components/rclpy_components/component_container_mt.py

@@ -12,29 +12,26 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import signal
 import rclpy
 from rclpy.executors import MultiThreadedExecutor
-from rclpy.component import ComponentManager
+from .component_manager import ComponentManager
 
 
 def main():
-    try:
-        _main()
-    except KeyboardInterrupt:
-        print('KeyboardInterrupt received, exit')
-        return signal.SIGINT
-
-
-def _main():
     rclpy.init()
 
     executor = MultiThreadedExecutor()
     component_manager = ComponentManager(executor, "PyComponentManager")
 
     executor.add_node(component_manager)
-    executor.spin()
 
+    try:
+        executor.spin()
+    except KeyboardInterrupt:
+        print('KeyboardInterrupt received, exit')
+        pass
+
+    component_manager.destroy_node()
     rclpy.shutdown()
 
 

rclpy_components/rclpy_components/component_manager.py

@@ -14,35 +14,31 @@
 
 from rclpy.node import Node
 from rclpy.executors import Executor
-from rclpy.logging import get_logger
 from rclpy.exceptions import InvalidNodeNameException, InvalidNamespaceException
 from composition_interfaces.srv import ListNodes, LoadNode, UnloadNode
 try:
     from importlib.metadata import entry_points
 except ImportError:
     from importlib_metadata import entry_points
 
-RCLPY_COMPONENTS = 'rclpy_components'
-logger = get_logger('ComponentManager')
-
 
 class ComponentManager(Node):
 
     def __init__(self, executor: Executor, name="py_component_manager", **kwargs):
-        # TODO Handle the py args equivalent to rclcpp 'NodeOptions'
         super().__init__(name, **kwargs)
         self.executor = executor
         # Implement the 3 services described in
         # http://design.ros2.org/articles/roslaunch.html#command-line-arguments
-        self.list_node_srv_ = self.create_service(ListNodes, "~/_container/list_nodes", self.on_list_node)
-        self.load_node_srv_ = self.create_service(LoadNode, "~/_container/load_node", self.on_load_node)
-        self.unload_node_srv_ = self.create_service(UnloadNode, "~/_container/unload_node", self.on_unload_node)
+        self._list_node_srv = self.create_service(
+            ListNodes, "~/_container/list_nodes", self.on_list_node)
+        self._load_node_srv = self.create_service(
+            LoadNode, "~/_container/load_node", self.on_load_node)
+        self._unload_node_srv = self.create_service(
+            UnloadNode, "~/_container/unload_node", self.on_unload_node)
 
         self.components = {}  # key: unique_id, value: full node name and component instance
         self.unique_id_index = 0
 
-        self.executor.spin()
-
     def gen_unique_id(self):
         self.unique_id_index += 1
         return self.unique_id_index
@@ -54,10 +50,10 @@ def on_list_node(self, req: ListNodes.Request, res: ListNodes.Response):
         return res
 
     def on_load_node(self, req: LoadNode.Request, res: LoadNode.Response):
-        component_entry_points = entry_points().get(RCLPY_COMPONENTS, None)
+        component_entry_points = entry_points().get('rclpy_components', None)
         if not component_entry_points:
-            logger.error('No rclpy components registered')
             res.success = False
+            res.error_message = 'No rclpy components registered'
             return res
 
         component_entry_point = None
@@ -67,15 +63,15 @@ def on_load_node(self, req: LoadNode.Request, res: LoadNode.Response):
                 break
 
         if not component_entry_point:
-            logger.error('No rclpy component found by %s' % req.plugin_name)
             res.success = False
+            res.error_message = f'No rclpy component found by {req.plugin_name}'
             return res
 
         component_class = component_entry_point.load()
         node_name = req.node_name if req.node_name else \
             str.lower(str.split(component_entry_point.value, ':')[1])
 
-        params_dict = {'use_global_arguments': False}
+        params_dict = {'use_global_arguments': False, 'context': self.context}
         if req.parameters:
             params_dict['parameter_overrides'] = req.parameters
 
@@ -88,30 +84,27 @@ def on_load_node(self, req: LoadNode.Request, res: LoadNode.Response):
                 params_dict['cli_args'].extend(['-r', rule])
 
         try:
-            logger.info('Instantiating {} with {}, {}'.format(component_entry_point.value, node_name, params_dict))
+            self.get_logger().info(
+                f'Instantiating {component_entry_point.value} with {node_name}, {params_dict}')
             component = component_class(node_name, **params_dict)
-
             res.unique_id = self.gen_unique_id()
-            # TODO Assign the full_node_name with node.get_fully_qualified_name
-            res.full_node_name = '/{}'.format(node_name)
-            if req.node_namespace:
-                res.full_node_name = '/{}{}'.format(req.node_namespace, res.full_node_name)
+            res.full_node_name = component.get_fully_qualified_name()
             res.success = True
-            self.components[str(res.unique_id)] = (res.full_node_name, component)
+            self.components[res.unique_id] = (res.full_node_name, component)
             self.executor.add_node(component)
             return res
         except (InvalidNodeNameException, InvalidNamespaceException, TypeError) as e:
             error_message = str(e)
-            logger.error('Failed to load node: %s' % error_message)
+            self.get_logger().error(f'Failed to load node: {error_message}')
             res.success = False
             res.error_message = error_message
             return res
 
     def on_unload_node(self, req: UnloadNode.Request, res: UnloadNode.Response):
-        uid = str(req.unique_id)
+        uid = req.unique_id
         if uid not in self.components:
-            res._error_message = 'No node found with unique_id: %s' % uid
             res.success = False
+            res.error_message = f'No node found with unique_id: {uid}'
             return res
 
         _, component_instance = self.components.pop(uid)

rclpy_components/setup.py

@@ -1,22 +1,22 @@
-from setuptools import setup
+from setuptools import setup, find_packages
 
 package_name = 'rclpy_components'
 
 setup(
     name=package_name,
-    version='0.0.0',
-    packages=[package_name],
+    version='1.1.0',
+    packages=find_packages(),
     data_files=[
         ('share/ament_index/resource_index/packages',
             ['resource/' + package_name]),
         ('share/' + package_name, ['package.xml']),
     ],
     install_requires=['setuptools'],
     zip_safe=True,
-    maintainer='root',
-    maintainer_email='[email protected]',
-    description='TODO: Package description',
-    license='TODO: License declaration',
+    maintainer='Zhen Ju',
+    maintainer_email='[email protected]',
+    description='The dynamic node management package',
+    license='Apache License 2.0',
     tests_require=['pytest'],
     entry_points={
         'console_scripts': [