-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add convenience error handling macros #421
Add convenience error handling macros #421
Conversation
Signed-off-by: methylDragon <methylDragon@gmail.com>
1f750fa
to
fed5d64
Compare
Signed-off-by: methylDragon <methylDragon@gmail.com>
Signed-off-by: methylDragon <methylDragon@gmail.com>
Before going too far forward with this, I think we should talk about the error handling in our C layers in a more general sense. Mich filed #269 3 years ago to consider this issue. I think we should come up with a general strategy before we introduce new code here. Maybe we can talk about it at the ROS 2 weekly tomorrow? |
I think we can do that, but some of my other PRs pending the rmw freeze are using code from this PR... I was thinking that since these are slight extensions to the existing macros, it's not too much of an issue to merge them in first, then consider a more general strategy after the freeze? |
So I thought that everything remaining to be merged was "bonus", no? If that's the case, then we can leave them out of Iron and target them for J-Turtle. If that's not the case, can you point to what still needs to get in? |
I think it's the prs attached to ros2/ros2#1405. I think we should try to get them in. If this pr doesn't get in, I think the others can still go in, he'll just have to expand the macros in this pr manually. Which honestly is what is done in the code base today (there are lots of places where we do what these macros do already). |
For what it's worth, the code that these new macros do is already present in the code base, and was my suggestion for what should be done right now: #269 (comment) Example of it already in the code base: I think #269 is more about a better way to do this stuff, which I think is a worthwhile thing to discuss, but what's in this pr is just making macros to de-duplicate existing code, at least in my opinion. |
Needed for: ros2/ros2#1405
This adds a bunch of macros to append error strings.
So before you would do:
Now you can do:
(And similarly for setting error with format strings, and also safe fprint)
If no old error was set, then the behavior is just like RCUTILS_SET_ERROR_MSG