From 2968a85c2ab47e45eeda0b2e338e30e1a2ed29cd Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Fri, 30 Aug 2019 17:00:07 -0300 Subject: [PATCH 1/5] Raise custom exception when rclpy_get_*_names_and_types_by_node doesn't find a node name Signed-off-by: ivanpauno --- rclpy/rclpy/__init__.py | 9 ++++++++- rclpy/rclpy/node.py | 8 ++++++++ rclpy/src/rclpy/_rclpy.c | 36 ++++++++++++++++++++++++++++++------ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/rclpy/rclpy/__init__.py b/rclpy/rclpy/__init__.py index 38f6cd406..aab26b488 100644 --- a/rclpy/rclpy/__init__.py +++ b/rclpy/rclpy/__init__.py @@ -45,6 +45,7 @@ from typing import TYPE_CHECKING from rclpy.context import Context +# from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy from rclpy.parameter import Parameter from rclpy.task import Future from rclpy.utilities import get_default_context @@ -58,6 +59,10 @@ from rclpy.executors import Executor # noqa: F401 from rclpy.node import Node # noqa: F401 +# `rclpy.node.Node.get_*_names_and_types_by_node` methods may raise this error. +# It is populated in `init` function. +NodeNameNonExistentError = None + def init(*, args: List[str] = None, context: Context = None) -> None: """ @@ -70,7 +75,9 @@ def init(*, args: List[str] = None, context: Context = None) -> None: context = get_default_context() if context is None else context # imported locally to avoid loading extensions on module import from rclpy.impl.implementation_singleton import rclpy_implementation - return rclpy_implementation.rclpy_init(args if args is not None else sys.argv, context.handle) + global NodeNameNonExistentError + rclpy_implementation.rclpy_init(args if args is not None else sys.argv, context.handle) + NodeNameNonExistentError = rclpy_implementation.NodeNameNonExistentError # The global spin functions need an executor to do the work diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py index c95115e0c..673763d9e 100644 --- a/rclpy/rclpy/node.py +++ b/rclpy/rclpy/node.py @@ -1463,6 +1463,8 @@ def get_publisher_names_and_types_by_node( :return: List of tuples. The first element of each tuple is the topic name and the second element is a list of topic types. + :raise rclpy.NonExistentNodeNameError: If the node wasn't found. + :raise RuntimeError: Unexpected failure. """ with self.handle as capsule: return _rclpy.rclpy_get_publisher_names_and_types_by_node( @@ -1483,6 +1485,8 @@ def get_subscriber_names_and_types_by_node( :return: List of tuples. The first element of each tuple is the topic name and the second element is a list of topic types. + :raise rclpy.NonExistentNodeNameError: If the node wasn't found. + :raise RuntimeError: Unexpected failure. """ with self.handle as capsule: return _rclpy.rclpy_get_subscriber_names_and_types_by_node( @@ -1501,6 +1505,8 @@ def get_service_names_and_types_by_node( :return: List of tuples. The first element of each tuple is the service server name and the second element is a list of service types. + :raise rclpy.NonExistentNodeNameError: If the node wasn't found. + :raise RuntimeError: Unexpected failure. """ with self.handle as capsule: return _rclpy.rclpy_get_service_names_and_types_by_node( @@ -1519,6 +1525,8 @@ def get_client_names_and_types_by_node( :return: List of tuples. The fist element of each tuple is the service client name and the second element is a list of service client types. + :raise rclpy.NonExistentNodeNameError: If the node wasn't found. + :raise RuntimeError: Unexpected failure. """ with self.handle as capsule: return _rclpy.rclpy_get_client_names_and_types_by_node( diff --git a/rclpy/src/rclpy/_rclpy.c b/rclpy/src/rclpy/_rclpy.c index dca227364..c72d3edf0 100644 --- a/rclpy/src/rclpy/_rclpy.c +++ b/rclpy/src/rclpy/_rclpy.c @@ -41,6 +41,7 @@ static PyObject * RCLInvalidROSArgsError; static PyObject * UnknownROSArgsError; +static PyObject * _rclpy_node_name_non_existent_error; void _rclpy_context_capsule_destructor(PyObject * capsule) @@ -3213,10 +3214,13 @@ rclpy_get_service_names_and_types_by_node(PyObject * Py_UNUSED(self), PyObject * rcl_get_service_names_and_types_by_node(node, &allocator, node_name, node_namespace, &service_names_and_types); if (ret != RCL_RET_OK) { - PyErr_Format(PyExc_RuntimeError, + PyObject * error = PyExc_RuntimeError; + if (ret == RCL_RET_NODE_NAME_NON_EXISTENT) { + error = _rclpy_node_name_non_existent_error; + } + PyErr_Format(error, "Failed to get_service_names_and_types: %s", rcl_get_error_string().str); rcl_reset_error(); - rclpy_names_and_types_fini(&service_names_and_types); return NULL; } @@ -3262,10 +3266,13 @@ rclpy_get_client_names_and_types_by_node(PyObject * Py_UNUSED(self), PyObject * rcl_get_client_names_and_types_by_node(node, &allocator, node_name, node_namespace, &client_names_and_types); if (ret != RCL_RET_OK) { - PyErr_Format(PyExc_RuntimeError, + PyObject * error = PyExc_RuntimeError; + if (ret == RCL_RET_NODE_NAME_NON_EXISTENT) { + error = _rclpy_node_name_non_existent_error; + } + PyErr_Format(error, "Failed to get_client_names_and_types: %s", rcl_get_error_string().str); rcl_reset_error(); - rclpy_names_and_types_fini(&client_names_and_types); return NULL; } @@ -3313,7 +3320,11 @@ rclpy_get_subscriber_names_and_types_by_node(PyObject * Py_UNUSED(self), PyObjec rcl_get_subscriber_names_and_types_by_node(node, &allocator, no_demangle, node_name, node_namespace, &topic_names_and_types); if (ret != RCL_RET_OK) { - PyErr_Format(PyExc_RuntimeError, + PyObject * error = PyExc_RuntimeError; + if (ret == RCL_RET_NODE_NAME_NON_EXISTENT) { + error = _rclpy_node_name_non_existent_error; + } + PyErr_Format(error, "Failed to get_subscriber_names_and_types: %s", rcl_get_error_string().str); rcl_reset_error(); return NULL; @@ -3362,7 +3373,11 @@ rclpy_get_publisher_names_and_types_by_node(PyObject * Py_UNUSED(self), PyObject rcl_get_publisher_names_and_types_by_node(node, &allocator, no_demangle, node_name, node_namespace, &topic_names_and_types); if (ret != RCL_RET_OK) { - PyErr_Format(PyExc_RuntimeError, + PyObject * error = PyExc_RuntimeError; + if (ret == RCL_RET_NODE_NAME_NON_EXISTENT) { + error = _rclpy_node_name_non_existent_error; + } + PyErr_Format(error, "Failed to get_publisher_names_and_types: %s", rcl_get_error_string().str); rcl_reset_error(); return NULL; @@ -5081,6 +5096,15 @@ PyMODINIT_FUNC PyInit__rclpy(void) return NULL; } + _rclpy_node_name_non_existent_error = PyErr_NewExceptionWithDoc( + "_rclpy.NodeNameNonExistentError", + "Thrown when a node name is not found.", + PyExc_RuntimeError, NULL); + if (PyModule_AddObject(m, "NodeNameNonExistentError", _rclpy_node_name_non_existent_error)) { + Py_DECREF(m); + return NULL; + } + if (PyErr_Occurred()) { Py_DECREF(m); return NULL; From 14531412678549ba803f4cf505bc8506bfff20d6 Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Fri, 30 Aug 2019 17:06:09 -0300 Subject: [PATCH 2/5] Delete commented line Signed-off-by: ivanpauno --- rclpy/rclpy/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rclpy/rclpy/__init__.py b/rclpy/rclpy/__init__.py index aab26b488..40616d2c3 100644 --- a/rclpy/rclpy/__init__.py +++ b/rclpy/rclpy/__init__.py @@ -45,7 +45,6 @@ from typing import TYPE_CHECKING from rclpy.context import Context -# from rclpy.impl.implementation_singleton import rclpy_implementation as _rclpy from rclpy.parameter import Parameter from rclpy.task import Future from rclpy.utilities import get_default_context From 666c421c0bd23aef8f028524c7f51bead27f9481 Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Mon, 16 Sep 2019 16:15:41 -0300 Subject: [PATCH 3/5] Move new exception to node.py Signed-off-by: ivanpauno --- rclpy/rclpy/__init__.py | 7 +------ rclpy/rclpy/node.py | 13 +++++++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/rclpy/rclpy/__init__.py b/rclpy/rclpy/__init__.py index 40616d2c3..a4dc96bff 100644 --- a/rclpy/rclpy/__init__.py +++ b/rclpy/rclpy/__init__.py @@ -58,10 +58,6 @@ from rclpy.executors import Executor # noqa: F401 from rclpy.node import Node # noqa: F401 -# `rclpy.node.Node.get_*_names_and_types_by_node` methods may raise this error. -# It is populated in `init` function. -NodeNameNonExistentError = None - def init(*, args: List[str] = None, context: Context = None) -> None: """ @@ -75,8 +71,7 @@ def init(*, args: List[str] = None, context: Context = None) -> None: # imported locally to avoid loading extensions on module import from rclpy.impl.implementation_singleton import rclpy_implementation global NodeNameNonExistentError - rclpy_implementation.rclpy_init(args if args is not None else sys.argv, context.handle) - NodeNameNonExistentError = rclpy_implementation.NodeNameNonExistentError + return rclpy_implementation.rclpy_init(args if args is not None else sys.argv, context.handle) # The global spin functions need an executor to do the work diff --git a/rclpy/rclpy/node.py b/rclpy/rclpy/node.py index 673763d9e..5a693e078 100644 --- a/rclpy/rclpy/node.py +++ b/rclpy/rclpy/node.py @@ -34,6 +34,7 @@ from rcl_interfaces.msg import ParameterEvent from rcl_interfaces.msg import ParameterValue from rcl_interfaces.msg import SetParametersResult + from rclpy.callback_groups import CallbackGroup from rclpy.callback_groups import MutuallyExclusiveCallbackGroup from rclpy.client import Client @@ -83,6 +84,10 @@ SrvTypeRequest = TypeVar('SrvTypeRequest') SrvTypeResponse = TypeVar('SrvTypeResponse') +# Re-export exception defined in _rclpy C extension. +# `Node.get_*_names_and_types_by_node` methods may raise this error. +NodeNameNonExistentError = _rclpy.NodeNameNonExistentError + class Node: @@ -1463,7 +1468,7 @@ def get_publisher_names_and_types_by_node( :return: List of tuples. The first element of each tuple is the topic name and the second element is a list of topic types. - :raise rclpy.NonExistentNodeNameError: If the node wasn't found. + :raise NodeNameNonExistentError: If the node wasn't found. :raise RuntimeError: Unexpected failure. """ with self.handle as capsule: @@ -1485,7 +1490,7 @@ def get_subscriber_names_and_types_by_node( :return: List of tuples. The first element of each tuple is the topic name and the second element is a list of topic types. - :raise rclpy.NonExistentNodeNameError: If the node wasn't found. + :raise NodeNameNonExistentError: If the node wasn't found. :raise RuntimeError: Unexpected failure. """ with self.handle as capsule: @@ -1505,7 +1510,7 @@ def get_service_names_and_types_by_node( :return: List of tuples. The first element of each tuple is the service server name and the second element is a list of service types. - :raise rclpy.NonExistentNodeNameError: If the node wasn't found. + :raise NodeNameNonExistentError: If the node wasn't found. :raise RuntimeError: Unexpected failure. """ with self.handle as capsule: @@ -1525,7 +1530,7 @@ def get_client_names_and_types_by_node( :return: List of tuples. The fist element of each tuple is the service client name and the second element is a list of service client types. - :raise rclpy.NonExistentNodeNameError: If the node wasn't found. + :raise NodeNameNonExistentError: If the node wasn't found. :raise RuntimeError: Unexpected failure. """ with self.handle as capsule: From 0a4fe6f0b998398ecb9657a977aab2fb550d68ba Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Mon, 16 Sep 2019 16:26:15 -0300 Subject: [PATCH 4/5] Use CamelCase instead of snake_case in exception names Signed-off-by: ivanpauno --- rclpy/src/rclpy/_rclpy.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rclpy/src/rclpy/_rclpy.c b/rclpy/src/rclpy/_rclpy.c index c72d3edf0..d096bd8de 100644 --- a/rclpy/src/rclpy/_rclpy.c +++ b/rclpy/src/rclpy/_rclpy.c @@ -41,7 +41,7 @@ static PyObject * RCLInvalidROSArgsError; static PyObject * UnknownROSArgsError; -static PyObject * _rclpy_node_name_non_existent_error; +static PyObject * NodeNameNonExistentError; void _rclpy_context_capsule_destructor(PyObject * capsule) @@ -3216,7 +3216,7 @@ rclpy_get_service_names_and_types_by_node(PyObject * Py_UNUSED(self), PyObject * if (ret != RCL_RET_OK) { PyObject * error = PyExc_RuntimeError; if (ret == RCL_RET_NODE_NAME_NON_EXISTENT) { - error = _rclpy_node_name_non_existent_error; + error = NodeNameNonExistentError; } PyErr_Format(error, "Failed to get_service_names_and_types: %s", rcl_get_error_string().str); @@ -3268,7 +3268,7 @@ rclpy_get_client_names_and_types_by_node(PyObject * Py_UNUSED(self), PyObject * if (ret != RCL_RET_OK) { PyObject * error = PyExc_RuntimeError; if (ret == RCL_RET_NODE_NAME_NON_EXISTENT) { - error = _rclpy_node_name_non_existent_error; + error = NodeNameNonExistentError; } PyErr_Format(error, "Failed to get_client_names_and_types: %s", rcl_get_error_string().str); @@ -3322,7 +3322,7 @@ rclpy_get_subscriber_names_and_types_by_node(PyObject * Py_UNUSED(self), PyObjec if (ret != RCL_RET_OK) { PyObject * error = PyExc_RuntimeError; if (ret == RCL_RET_NODE_NAME_NON_EXISTENT) { - error = _rclpy_node_name_non_existent_error; + error = NodeNameNonExistentError; } PyErr_Format(error, "Failed to get_subscriber_names_and_types: %s", rcl_get_error_string().str); @@ -3375,7 +3375,7 @@ rclpy_get_publisher_names_and_types_by_node(PyObject * Py_UNUSED(self), PyObject if (ret != RCL_RET_OK) { PyObject * error = PyExc_RuntimeError; if (ret == RCL_RET_NODE_NAME_NON_EXISTENT) { - error = _rclpy_node_name_non_existent_error; + error = NodeNameNonExistentError; } PyErr_Format(error, "Failed to get_publisher_names_and_types: %s", rcl_get_error_string().str); @@ -5096,11 +5096,11 @@ PyMODINIT_FUNC PyInit__rclpy(void) return NULL; } - _rclpy_node_name_non_existent_error = PyErr_NewExceptionWithDoc( + NodeNameNonExistentError = PyErr_NewExceptionWithDoc( "_rclpy.NodeNameNonExistentError", "Thrown when a node name is not found.", PyExc_RuntimeError, NULL); - if (PyModule_AddObject(m, "NodeNameNonExistentError", _rclpy_node_name_non_existent_error)) { + if (PyModule_AddObject(m, "NodeNameNonExistentError", NodeNameNonExistentError)) { Py_DECREF(m); return NULL; } From bdcbe25332ce9ccbbc76b63efa3782fb0c08edb5 Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Mon, 16 Sep 2019 16:29:00 -0300 Subject: [PATCH 5/5] Delete old code Signed-off-by: ivanpauno --- rclpy/rclpy/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rclpy/rclpy/__init__.py b/rclpy/rclpy/__init__.py index a4dc96bff..38f6cd406 100644 --- a/rclpy/rclpy/__init__.py +++ b/rclpy/rclpy/__init__.py @@ -70,7 +70,6 @@ def init(*, args: List[str] = None, context: Context = None) -> None: context = get_default_context() if context is None else context # imported locally to avoid loading extensions on module import from rclpy.impl.implementation_singleton import rclpy_implementation - global NodeNameNonExistentError return rclpy_implementation.rclpy_init(args if args is not None else sys.argv, context.handle)