Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add preemption to rcl_action #679

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

naiveHobo
Copy link

@naiveHobo naiveHobo commented Jun 11, 2020

Signed-off-by: Sarthak Mittal sarthakmittal2608@gmail.com

Adds terminal state PREEMPTED and PREEMPT event to goal state machine in order to differentiate between a goal aborted due to genuine abort logic and goal aborted by the server due to preemption.

Based on the discussions in the following ticket: ros2/design#284
Depends on: ros2/rcl_interfaces#105

RCLActionDesign

Signed-off-by: Sarthak Mittal <sarthakmittal2608@gmail.com>
@naiveHobo naiveHobo marked this pull request as ready for review June 11, 2020 15:29
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.

mostly lgtm, but i am not so sure canceling should be preempted. I do not have any use case on this transition, could you share your thoughts?

},
[GOAL_STATE_CANCELING] = {
[GOAL_EVENT_SUCCEED] = _succeed_event_handler,
[GOAL_EVENT_ABORT] = _abort_event_handler,
[GOAL_EVENT_PREEMPT] = _preempt_event_handler,
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure canceling state can be preempted.

@naiveHobo
Copy link
Author

naiveHobo commented Jun 12, 2020

I assumed preemption should work the same way as abort since that's how preemption is handled right now. But I get your point, preemption makes sense only when a previous goal is executing.

@sloretz sloretz self-assigned this Jun 25, 2020
@ivanpauno
Copy link
Member

@sloretz friendly ping

@fujitatomoya
Copy link
Collaborator

It's been 2 months. @sloretz friendly ping.

@audrow audrow changed the base branch from master to rolling June 28, 2022 14:20
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants