rclpy_init() passes command line arguments to rcl_init()#179
Conversation
wjwwood
left a comment
There was a problem hiding this comment.
Other than some questions (mostly just curious), this lgtm.
| // Expect one argument which is a list of strings | ||
| PyObject * pyargs; | ||
| if (!PyArg_ParseTuple(args, "O", &pyargs)) { | ||
| return NULL; |
There was a problem hiding this comment.
Just to check, these functions setup the exceptions right? That's why only returning null here is fine?
There was a problem hiding this comment.
Yup, PyArg_ParseTuple returns false and sets an exception on failure.
There was a problem hiding this comment.
Added comments about exceptions already being set in 023ccaf
rclpy/src/rclpy/_rclpy.c
Outdated
| if (NULL == pyargs) { | ||
| return NULL; | ||
| } | ||
| Py_ssize_t num_args = PyList_Size(pyargs); |
There was a problem hiding this comment.
It can raise SystemError and return -1 https://github.com/python/cpython/blob/745dc65b17b3936e3f9f4099f735f174d30c4e0c/Objects/listobject.c#L188 if PyList_Check returns false, but I don't think that's possible if PySequence_List above succeeds.
|
New CI after fixing a windows warning
|
| } | ||
| // Borrows a pointer, do not free arg_values[i] | ||
| arg_values[i] = PyUnicode_AsUTF8(pyarg); | ||
| } |
There was a problem hiding this comment.
can we just skip this entire block (L180 to 197) when num_args is 0 and pass a NULL pointer to rcl_init instead?
|
CI with 93c634b
|
…size Fix 178 subscriber buffer size
This passes command line arguments through to rcl. Previously there was only a TODO message. This PR allows python nodes to be remapped using ros2/rcl#217.
CI
connects to ros2/ros2#450