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 when rcl_take_response failed #396

Closed
wants to merge 1 commit into from

Conversation

minggangw
Copy link

@minggangw minggangw commented Nov 13, 2017

[edit dhood] connect to #397

When the return value of rcl_take_response() is not equal to RCL_RET_OK,
we need reset the error to avoid being overwritten.

When the return value of rcl_take_response() is not equal to RCL_RET_OK,
we need reset the error to avoid being overwritten.
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

It's not really clear to me that we want to change this to an else statement. Looking through this file, the error check for all calls to rcl_take_* have a similar construction, and I'm assuming that is for a reason. Unfortunately I don't have enough context to know exactly what that reason is, but maybe someone else from @ros2/team has a better idea and can comment about it.

@dhood
Copy link
Member

dhood commented Nov 13, 2017

Thank you for the patch @minggangw

rcl_take_response returning RCL_RET_CLIENT_TAKE_FAILED is not considered an error state, which is why we don't print the error in this case.

As mentioned in #397 (comment), the error message should actually not have been set by rcl in this situation, and then the clearing of the message in the client libraries will not be necessary. I'll close this PR for that reason.

@dhood dhood closed this Nov 13, 2017
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
* decrease subscription expected msgs

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* change workflow to test this branch

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* change expected number too

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* linter

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* change workflow to autotest feature branch

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* revert workflow

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* run workflow more frequently to test stability

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* revert github workflow

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* cleanup

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* define constants

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* disable test on windows

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>

* disable play_filters_by_topic on windows

Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
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

3 participants