magazino / move_base_flex

Move Base Flex: a backwards-compatible replacement for move_base
https://uos.github.io/mbf_docs/
BSD 3-Clause "New" or "Revised" License
434 stars 154 forks source link

Update running controller goal also when we are preempting it #345

Closed corot closed 1 month ago

corot commented 1 month ago

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

  1. force_stop_on_cancel is set to false (we let the controller to smoothly stop when canceled)
  2. goal running
  3. we cancel it; the controller slows down gently
  4. before stopping, a new controller goal arrives
  5. we cancel the previous execution and join the thread, waiting for the robot to stop
  6. we keep calling the canceled controller to get slowing down speeds
  7. BUT as we have blocked the main thread, the velocity we pass to it (provided by /odom topic) won't change
  8. If the controller happens to relay on this velocity to control it's slowing down and stopping, we can deadlock

So instead of canceling and joining, with this PR, things change from point 5

  1. we reuse the same slot and change the path of the current execution
  2. it's not canceled anymore, so it executes the new path
corot commented 1 month ago

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.

correct yes, it makes sense to keep the issue as open thanks for the review!