Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Raise custom error when node name is not found #413

Merged
merged 5 commits into from
Sep 17, 2019
Merged

Raise custom error when node name is not found #413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2968a85
Raise custom exception when rclpy_get_*_names_and_types_by_node doesn…
ivanpauno Aug 30, 2019
1453141
Delete commented line
ivanpauno Aug 30, 2019
666c421
Move new exception to node.py
ivanpauno Sep 16, 2019
0a4fe6f
Use CamelCase instead of snake_case in exception names
ivanpauno Sep 16, 2019
bdcbe25
Delete old code
ivanpauno Sep 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions rclpy/rclpy/node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ivanpauno there was a previous comment from @dirk-thomas about not exporting to Python exceptions defined in C but the other way around. Since modules like rclpy.qos already import C extensions at the module level, this does not worsen the situation in any way.

So I'd say that we merge this as-is, to solve ros2/build_farmer#223, and then we work towards normalizing C extension use in rclpy -- whatever that means. There's a PR, #422, for it already. @dirk-thomas do you agree?



class Node:

Expand Down Expand Up @@ -1463,6 +1468,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 NodeNameNonExistentError: 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(
Expand All @@ -1483,6 +1490,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 NodeNameNonExistentError: 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(
Expand All @@ -1501,6 +1510,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 NodeNameNonExistentError: 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(
Expand All @@ -1519,6 +1530,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 NodeNameNonExistentError: 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(
Expand Down
36 changes: 30 additions & 6 deletions rclpy/src/rclpy/_rclpy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

static PyObject * RCLInvalidROSArgsError;
static PyObject * UnknownROSArgsError;
static PyObject * NodeNameNonExistentError;

void
_rclpy_context_capsule_destructor(PyObject * capsule)
Expand Down Expand Up @@ -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 = NodeNameNonExistentError;
}
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;
}

Expand Down Expand Up @@ -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 = NodeNameNonExistentError;
}
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;
}

Expand Down Expand Up @@ -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 = NodeNameNonExistentError;
}
PyErr_Format(error,
"Failed to get_subscriber_names_and_types: %s", rcl_get_error_string().str);
rcl_reset_error();
return NULL;
Expand Down Expand Up @@ -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 = NodeNameNonExistentError;
}
PyErr_Format(error,
"Failed to get_publisher_names_and_types: %s", rcl_get_error_string().str);
rcl_reset_error();
return NULL;
Expand Down Expand Up @@ -5081,6 +5096,15 @@ PyMODINIT_FUNC PyInit__rclpy(void)
return NULL;
}

NodeNameNonExistentError = PyErr_NewExceptionWithDoc(
"_rclpy.NodeNameNonExistentError",
"Thrown when a node name is not found.",
PyExc_RuntimeError, NULL);
if (PyModule_AddObject(m, "NodeNameNonExistentError", NodeNameNonExistentError)) {
Py_DECREF(m);
return NULL;
}

if (PyErr_Occurred()) {
Py_DECREF(m);
return NULL;
Expand Down