-
Notifications
You must be signed in to change notification settings - Fork 422
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
change throw_from_rcl_error to throw from_rcl_error #647
Comments
Seems reasonable, at the very least to factor the creation of the exception and the throw into separate functions. |
Mind if I give this a go? I was thinking of renaming "throw_from_rcl_error" to either "from_rcl_error" or "gen_rcl_exception", then refactoring everything that called the original function to "throw <from_rcl_error>". Unless, of course, you want to do something besides throwing at any of the locations. Also, out of curiosity, is there a reason we aren't using enums for return codes? And should I put that in a separate question? |
Sure, please have a go at it.
I think I don't mind which you do, but the already proposed ones seem like the best name I've heard so far. Also, consider the line from the description:
Avoiding touching all the existing code would be a nice thing to have, while still enabling users to generate the exception without throwing it in cases where that's needed.
I think it's mostly a historical reason, but avoiding enums in C (and using |
I think this ticket is invalid as written. I assumed The original issue is having to work around cppcheck's false positive. I think a better solution would would be for Another option would be to add a cppcheck library config file for rclcpp that has |
Closing now that #678 is merged |
Signed-off-by: Stephen Brawner <[email protected]>
Feature request
Currently rclcpp has a function to throw an exception based on an rcl error.
rclcpp/rclcpp/src/rclcpp/clock.cpp
Line 134 in 3919ab1
This is a request to change that function to return an exception without throwing.
I think this could help static analysis tools like
cppcheck
recognize when code won't be executed due to an exception. This would remove the need for #646Implementation considerations
throw_from_rcl_error
could be refactored to usefrom_rcl_error
rather than changing all the code at once.The text was updated successfully, but these errors were encountered: