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.

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

Create custom exceptions in _rclpy.c #31

Closed
mikaelarguedas opened this issue May 27, 2016 · 7 comments · Fixed by #478
Closed

Create custom exceptions in _rclpy.c #31

mikaelarguedas opened this issue May 27, 2016 · 7 comments · Fixed by #478
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@mikaelarguedas
Copy link
Member

For now every failure in rclpy raises a RuntimeError, we should create custom errors for different failure scenarios and avoid the tests of just catching random RuntimeErrors.

@suab321321
Copy link
Contributor

@mikaelarguedas I would like to like to contribute

@mikaelarguedas
Copy link
Member Author

@suab321321 I haven't been involved in this project for a long time so can't provide feedback about the status / relevance of this ticket with respect to the current code base.
Maybe current maintainers will be able to provide more up-to-date information.

@jacobperron
Copy link
Member

It looks like we have a few custom exceptions defined already:

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

But a quick search yields several places in _rclpy.c that raise RuntimeError where we might consider using custom exceptions. Whenever a call to the rcl layer errors, at the very least we could return a generic RCLError exception (I think this is what rclcpp does).

@suab321321 If you'd like to contribute to this ticket, I think a good first step would be to replace instances of RuntimeError with a new RCLError whenever there's a failed call to the rcl layer.
I suggest reviewing documentation on writing C-extensions for Python, in particular this section on errors and exceptions. You can add logic to initialize the new exception next to the other exceptions:

rclpy/rclpy/src/rclpy/_rclpy.c

Lines 5088 to 5099 in d375c84

RCLInvalidROSArgsError = PyErr_NewExceptionWithDoc(
"_rclpy.RCLInvalidROSArgsError",
"Thrown when invalid ROS arguments are found by rcl.",
PyExc_RuntimeError, NULL);
if (NULL == RCLInvalidROSArgsError) {
Py_DECREF(m);
return NULL;
}
if (PyModule_AddObject(m, "RCLInvalidROSArgsError", RCLInvalidROSArgsError) != 0) {
Py_DECREF(m);
return NULL;
}

@suab321321
Copy link
Contributor

@jacobperron okay sir I will proceed with this

@suab321321
Copy link
Contributor

@jacobperron sir I have to make RCLError first right?

@jacobperron
Copy link
Member

I have to make RCLError first right?

Correct. You can declare and initialize it in the two places of the code I referenced (#31 (comment)).

@suab321321
Copy link
Contributor

#478 here @jacobperron sir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants