move_base cancelGoal doesn't work while executing recovery behavior

[ros-planning]

move_base cancelGoal doesn't work while executing recovery behavior

[jack-oquin]

Q&A page is here: http://answers.ros.org/question/61720/move_base-cannot-cancel-while-executing-recovery-behavior/

[hershwg]

Definitely a bug. The API will have to change to support this though. Currently the runBehavior call does its whole thing in one thread in the one function call, so there's no way to interrupt it.

I could give that function a return value ( boolean) which says whether it is done or not. Then the recovery behavior will need to have its loop opened up so it returns false after each cycle until it's done.

[jack-oquin]

Is there some way to hack it by setting a global boolean variable for the behavior to check periodically, returning if it's set?

[hershwg]

Well, we could always hack it, but I would love to avoid that.

Options I'm seeing so far:

• change RecoveryBehavior API so each RecoveryBehavior gets a pointer to a MoveBase object or a NodeHandle or even just a boolean that lets it query whether it has been cancelled.
• change RecoveryBehavior API so runBehavior() returns a boolean indicating "done" or "not done" and the docs are updated to indicated that every call to runBehavior() should take only a short (sub-second) amount of time. This may mean opening loops and implementing state machines to break up long-running recovery behavior functions.
• Leave the API the same, but run the RecoveryBehavior in a separate thread. When a pre-empt or cancel arrives and the behavior is still running, the thread gets killed. This "option" looks pretty dangerous and non-portable, based on my readings. On Windows you can get situations where the child thread is in the middle of allocating memory and it has a lock on that and then just dies, leaving the memory-allocation lock locked (!). Under Posix threads the situation is a bit better, but still pretty awkward.
• It is true that we could make a global variable which move_base would write to and which recovery behaviors could read from if they wanted. This is just a backwards-compatible way to change the API.
• API for RecoveryBehavior could have an added pair of functions: bool isCancelled() and setIsCancelledCallback( boost::function<...> ). Then MoveBase can call setIsCancelledCallback() on every recovery behavior at initialization, and recovery behavior implementers can add calls to isCancelled() at their leisure (when they run into this problem, most likely). This is another backwards-compatible way to change the API which does not involve a global variable. :)

So after typing all that out, my favorite is the last one.

[jack-oquin]

My naive opinion: your last option looks good.

[corot]

Sorry, I'm not familiar with RecoveryBehavior API, so I cannot have an informed opinion.

When do you plan to attack this issue?

[Tutorgaming]

Hello from 2017, Is there any solution implemented or available to fix this issue?

[ccattywampus]

@Tutorgaming Hello from 2018! Did you ever find a way to work around this?

[Tutorgaming]

It seems that I disabled the original RecoveryBehavior and try to recover the robot outside the move_base. Everything works fine .

in the recovery behavior need to somehow connected to the move_base server