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

Reset the error on failing to find the C typesupport #274

Merged
merged 2 commits into from
Jan 5, 2021

Conversation

clalancette
Copy link
Contributor

The individual commits have more information, but essentially make sure to reset the error in between calls to probing for typesupport. This should fix the bulk of the test failures we are seeing in ros2/rmw#293 .

@eboasson Note that this is different than what we had discussed earlier. After talking to some more people, changing the contract of get_typesupport_handle_function is a little more dangerous than we want to attempt right now . So this is the more targeted, if less ideal, solution. I'll likely still open an issue to follow-up and change that contract, since the current contract seems a bit ugly.

CI for this change is in ros2/rmw#293 .

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
The rcutils error_string is meant to be used like errno; it
may or may not be set when entering a function, calling other
functions may or may not set it, and it is up to the caller
on how to handle all of that.  make_message_value_type() and
make_request_response_value_types() were not resetting the
error properly, and so could leave a failure state around.
Fix that here by mimicing code that exists elsewhere in
rmw_cyclonedds_cpp.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Copy link
Collaborator

@eboasson eboasson left a comment

Choose a reason for hiding this comment

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

Thanks @clalancette! It looks good to me and it definitely does the trick (ros2/rmw#293 confirmed it).

@clalancette
Copy link
Contributor Author

Thanks @clalancette! It looks good to me and it definitely does the trick (ros2/rmw#293 confirmed it).

Thanks for the review. I'm going to go ahead and make this a real PR and merge it. We can work on the rest of the failing tests over in ros2/rmw#293 .

@clalancette clalancette marked this pull request as ready for review January 5, 2021 14:10
@clalancette clalancette merged commit 8bce86b into master Jan 5, 2021
@delete-merged-branch delete-merged-branch bot deleted the clalancette/reset-error-on-type-support branch January 5, 2021 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants