Skip to content

Commit

Permalink
missed some capsule names
Browse files Browse the repository at this point in the history
  • Loading branch information
mikaelarguedas committed Oct 25, 2017
1 parent 104baeb commit 095267c
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions rclpy/src/rclpy/_rclpy.c
Original file line number Diff line number Diff line change
Expand Up @@ -1176,8 +1176,8 @@ rclpy_create_subscription(PyObject * Py_UNUSED(self), PyObject * args)
PyObject * pysubscription = PyCapsule_New(subscription, "rcl_subscription_t", NULL);
rcl_subscription_options_t subscription_ops = rcl_subscription_get_default_options();

if (PyCapsule_IsValid(pyqos_profile, NULL)) {
void * p = PyCapsule_GetPointer(pyqos_profile, NULL);
if (PyCapsule_IsValid(pyqos_profile, "rmw_qos_profile_t")) {
void * p = PyCapsule_GetPointer(pyqos_profile, "rmw_qos_profile_t");
rmw_qos_profile_t * qos_profile = (rmw_qos_profile_t *)p;
subscription_ops.qos = *qos_profile;
PyMem_Free(p);
Expand Down Expand Up @@ -1454,7 +1454,7 @@ rclpy_create_service(PyObject * Py_UNUSED(self), PyObject * args)

/// Publish a response message
/**
* Raises ValueError if pyservice is not a service capsule
* Raises ValueError if the capsules are not the correct types
* Raises RuntimeError if the response could not be sent
*
* \param[in] pyservice Capsule pointing to the service
Expand Down Expand Up @@ -2299,6 +2299,7 @@ rclpy_get_node_names(PyObject * Py_UNUSED(self), PyObject * args)

/// Get the list of topics discovered by the provided node
/**
* Raises ValueError if pynode is not a node capsule
* Raises RuntimeError if there is an rcl error
*
* \param[in] pynode Capsule pointing to the node
Expand Down Expand Up @@ -2369,6 +2370,7 @@ rclpy_get_topic_names_and_types(PyObject * Py_UNUSED(self), PyObject * args)

/// Get the list of services discovered by the provided node
/**
* Raises ValueError if pynode is not a node capsule
* Raises RuntimeError if there is an rcl error
*
* \param[in] pynode Capsule pointing to the node
Expand All @@ -2384,7 +2386,7 @@ rclpy_get_service_names_and_types(PyObject * Py_UNUSED(self), PyObject * args)
return NULL;
}

rcl_node_t * node = (rcl_node_t *)PyCapsule_GetPointer(pynode, NULL);
rcl_node_t * node = (rcl_node_t *)PyCapsule_GetPointer(pynode, "rcl_node_t");
if (!node) {
return NULL;
}
Expand Down

5 comments on commit 095267c

@dirk-thomas
Copy link
Member

Choose a reason for hiding this comment

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

That leaves us with the question why the CI builds passed before??

@mikaelarguedas
Copy link
Member Author

Choose a reason for hiding this comment

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

Depend which change you;re referring to:
I dont think there is anything in CI exercising service_names_and_types.

For the qos profile, I think it just have been ignored (because if the capsule is invalid we use the default qos profile). Given that we dont test qos settings in test_communication yet, no test would have caught it

@sloretz
Copy link
Contributor

Choose a reason for hiding this comment

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

I added a test to make sure node.service_names_and_types() doesn't raise in 76f4359. It fails without the changes in this commit.

@mikaelarguedas
Copy link
Member Author

Choose a reason for hiding this comment

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

Nice, is it possible to add one for topic_names_and_types as well ?

@sloretz
Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. I threw in get_node_names too d4048f6

Please sign in to comment.