-
Notifications
You must be signed in to change notification settings - Fork 155
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
Update running controller goal also when we are preempting it #345
Conversation
fdaa96b
to
4c79d68
Compare
if ((slot_it->second.execution->getName() == goal_handle.getGoal()->controller || | ||
goal_handle.getGoal()->controller.empty()) && | ||
slot_it->second.goal_handle.getGoalStatus().status == actionlib_msgs::GoalStatus::ACTIVE) | ||
(slot_status == actionlib_msgs::GoalStatus::ACTIVE || slot_status == actionlib_msgs::GoalStatus::PREEMPTING)) |
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.
equivalent to isActive method on SimpleActionServer
@@ -143,7 +143,7 @@ class AbstractActionBase | |||
// if there is already a plugin running on the same slot, cancel it | |||
slot_it->second.execution->cancel(); | |||
|
|||
// TODO + WARNING: this will block the main thread for an arbitrary time during which we won't execute callbacks | |||
// WARNING: this will block the main thread for an arbitrary time during which we won't execute callbacks |
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.
Still true, e.g. if the old and new goals have different controller.
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.
This PR only fixes the issue if the new goal has the same controller as the currently cancelling goal right? Maybe better to leave the issue open then even after merging this PR.
Glad to see the graceful cancel bug at least partially addressed though! 🥳
correct |
This PR partially addresses issue #323, for the case of preempting a controller execution with the same controller.
The problem is still there if the old and new goals have different controller.
I re-state here the problem and the solution
So instead of canceling and joining, with this PR, things change from point 5