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.

Sign up for GitHub

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

Jump to bottom

fixed rcl_wait return error when timer cancelled #1003

Merged
merged 8 commits into from
Sep 5, 2022
Merged

fixed rcl_wait return error when timer cancelled #1003

merged 8 commits into from
Sep 5, 2022

Conversation

ladianchad
Copy link
Contributor

@ladianchad ladianchad commented Aug 25, 2022 •
edited
Loading

when timer canclled, rcl_timer_get_time_until_next_call return RCL_RET_TIMER_CANCELED
rcl_wait return with "error not set" like below ( we checked rcl_timer_is_canceled but timer is atomic, we can miss this)

[planner_server-3] terminate called after throwing an instance of 'rclcpp::exceptions::RCLError'
[planner_server-3]   what():  rcl_wait() failed: error not set

so, I changed check timer canceled by rcl_timer_get_time_until_next_call's return value.

Would these solutions be appropriate?

@ladianchad ladianchad closed this Aug 25, 2022
@ladianchad ladianchad reopened this Aug 25, 2022
@ladianchad
Copy link
Contributor Author

ros2/rclcpp#1977
this bug also mentioned at this issue

@ladianchad ladianchad mentioned this pull request Aug 25, 2022
Closed
fujitatomoya
fujitatomoya approved these changes Aug 25, 2022
View reviewed changes
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm!

@fujitatomoya
Copy link
Collaborator

@ladianchad thanks for the contribution, this looks good to me. can you address DCO error?

@fujitatomoya
Copy link
Collaborator

@iuhilnehc-ynos requesting another review on this.

CC: @ivanpauno

@ladianchad
Copy link
Contributor Author

ladianchad commented Aug 26, 2022 •
edited
Loading

@ladianchad thanks for the contribution, this looks good to me. can you address DCO error?

yes, I fixed DCO error and rebased.

@iuhilnehc-ynos
Copy link
Collaborator

iuhilnehc-ynos commented Aug 26, 2022 •
edited
Loading

LGTM. The default branch to push is rolling, then we can backport it to humble.

@ladianchad
Copy link
Contributor Author

oh. thanks i changed pr branch to humble

@ladianchad ladianchad changed the base branch from humble to rolling August 26, 2022 01:20
ivanpauno
ivanpauno approved these changes Aug 26, 2022
View reviewed changes
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM!

rcl/src/rcl/wait.c Outdated Show resolved Hide resolved
@fujitatomoya
Copy link
Collaborator

@ladianchad can you address @ivanpauno comment ? then i will start CI.

@ladianchad
removed empty line
1554188 
Signed-off-by: ladianchad <[email protected]>
@ladianchad
Copy link
Contributor Author

@ladianchad can you address @ivanpauno comment ? then i will start CI.

I applied review in 1554188 :)

@fujitatomoya
Copy link
Collaborator

@ladianchad thanks for the quick response.

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@ladianchad
Copy link
Contributor Author

Linux Build Status is unstable, is it ok?? I don't quite understand ROS CI yet.

@fujitatomoya
Copy link
Collaborator

I do not think that is related.

CI(Retry Linux):

  • Linux Build Status

@ladianchad
Copy link
Contributor Author

Thanks!

@fujitatomoya fujitatomoya merged commit 4b125b1 into ros2:rolling Sep 5, 2022
@fujitatomoya
Copy link
Collaborator

@Mergifyio backport humble galactic foxy

@mergify mergify bot mentioned this pull request Sep 5, 2022
Merged
mergify bot pushed a commit that referenced this pull request Sep 5, 2022
@ladianchad @mergify
fixed rcl_wait return error when timer cancelled (#1003)
aa43e8e 
* add error msg in rcl wait

Signed-off-by: kevin <[email protected]>

* error not set reason changed

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* rcl wait bug when timer is cancelled

Signed-off-by: kevin <[email protected]>

* change step of check timer cancelled

Signed-off-by: kevin <[email protected]>

* fixed rcl_wait when timer cancelled

Signed-off-by: kevin <[email protected]>

* removed empty line

Signed-off-by: ladianchad <[email protected]>

Signed-off-by: kevin <[email protected]>
Signed-off-by: ladianchad <[email protected]>
Co-authored-by: kevin <[email protected]>
(cherry picked from commit 4b125b1)
mergify bot pushed a commit that referenced this pull request Sep 5, 2022
@ladianchad @mergify
fixed rcl_wait return error when timer cancelled (#1003)
78cf2c9 
* add error msg in rcl wait

Signed-off-by: kevin <[email protected]>

* error not set reason changed

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* rcl wait bug when timer is cancelled

Signed-off-by: kevin <[email protected]>

* change step of check timer cancelled

Signed-off-by: kevin <[email protected]>

* fixed rcl_wait when timer cancelled

Signed-off-by: kevin <[email protected]>

* removed empty line

Signed-off-by: ladianchad <[email protected]>

Signed-off-by: kevin <[email protected]>
Signed-off-by: ladianchad <[email protected]>
Co-authored-by: kevin <[email protected]>
(cherry picked from commit 4b125b1)
This was referenced Sep 5, 2022
Closed
Closed
@mergify
Copy link

mergify bot commented Sep 5, 2022

backport humble galactic foxy

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Sep 5, 2022
@ladianchad @mergify
fixed rcl_wait return error when timer cancelled (#1003)
3a47d7e 
* add error msg in rcl wait

Signed-off-by: kevin <[email protected]>

* error not set reason changed

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* rcl wait bug when timer is cancelled

Signed-off-by: kevin <[email protected]>

* change step of check timer cancelled

Signed-off-by: kevin <[email protected]>

* fixed rcl_wait when timer cancelled

Signed-off-by: kevin <[email protected]>

* removed empty line

Signed-off-by: ladianchad <[email protected]>

Signed-off-by: kevin <[email protected]>
Signed-off-by: ladianchad <[email protected]>
Co-authored-by: kevin <[email protected]>
(cherry picked from commit 4b125b1)
fujitatomoya pushed a commit that referenced this pull request Sep 6, 2022
@mergify @ladianchad
fixed rcl_wait return error when timer cancelled (#1003) (#1006)
b7cf928 
* add error msg in rcl wait

Signed-off-by: kevin <[email protected]>

* error not set reason changed

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* rcl wait bug when timer is cancelled

Signed-off-by: kevin <[email protected]>

* change step of check timer cancelled

Signed-off-by: kevin <[email protected]>

* fixed rcl_wait when timer cancelled

Signed-off-by: kevin <[email protected]>

* removed empty line

Signed-off-by: ladianchad <[email protected]>

Signed-off-by: kevin <[email protected]>
Signed-off-by: ladianchad <[email protected]>
Co-authored-by: kevin <[email protected]>
(cherry picked from commit 4b125b1)

Co-authored-by: 정찬희 <[email protected]>
@ErikOrjehag
Copy link

Thank you for fixing this one! :D

clalancette pushed a commit that referenced this pull request Oct 13, 2022
@ladianchad @kevin-floatic @clalancette
fixed rcl_wait return error when timer cancelled (#1003)
9d6c9ba 
* add error msg in rcl wait

Signed-off-by: kevin <[email protected]>

* error not set reason changed

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* change error msg

Signed-off-by: kevin <[email protected]>

* rcl wait bug when timer is cancelled

Signed-off-by: kevin <[email protected]>

* change step of check timer cancelled

Signed-off-by: kevin <[email protected]>

* fixed rcl_wait when timer cancelled

Signed-off-by: kevin <[email protected]>

* removed empty line

Signed-off-by: ladianchad <[email protected]>

Signed-off-by: kevin <[email protected]>
Signed-off-by: ladianchad <[email protected]>
Co-authored-by: kevin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fujitatomoya fujitatomoya fujitatomoya approved these changes

@ivanpauno ivanpauno ivanpauno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ladianchad @fujitatomoya @iuhilnehc-ynos @ErikOrjehag @ivanpauno @kevin-floatic