-
Notifications
You must be signed in to change notification settings - Fork 163
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 preemption to rcl_action #679
base: rolling
Are you sure you want to change the base?
Conversation
Signed-off-by: Sarthak Mittal <[email protected]>
There was a problem hiding this 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, |
There was a problem hiding this comment.
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.
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 friendly ping |
It's been 2 months. @sloretz friendly ping. |
Signed-off-by: Sarthak Mittal [email protected]
Adds terminal state
PREEMPTED
andPREEMPT
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