State exits without calling on_exit when preempted

[fgreen]

Issue

We recently upgraded to latest version of Flexbe and noticed that on_exit is no longer being called when a behavior is preempted. We have a number of states that send an actionlib goal in on_enter and cancel it if it's still running in on_exit. See example_action_state as an example of this pattern.

Based on http://wiki.ros.org/flexbe/Tutorials/The%20State%20Lifecycle our assumption was that on_exit would be called exactly once for each call to on_enter so that we could use those functions to manage lifecycle of actions or other resources.

Workaround

As a workaround, we should be able to call on_exit from inside on_stop. However we will need add some extra bookkeeping to make sure multiple states aren't trying to clean up the same resources. I don't think this is a good long term solution.

Suspected Cause

It appears that 1ba1b73 introduced a change to how preempts were handled. The initial report in the linked issue indicated that it wasn't possible to cancel ('stop execution'?) because execution had resumed. In general it doesn't seem like states need to be paused in order to stop execution so I'm wondering if this was actually an issue with a specific state that perhaps had a blocking execute function? Contrary to the comment there, I would argue that on_exit is still needed. I'm less sure about on_resume since I can see why it would cause problems in many use cases.

Proposed Solution

I can see the argument for having 'stop execution' exit the behavior as quickly as possible, but I personally feel that giving states an opportunity to clean up is more important. I think that on_exit should be called as part of preempting.

We don't use pausing or resuming states much so I'm hesitant to suggest any changes that would affect those. For the same reasons that having on_enter and on_exit always paired is helpful, it seems desirable to have on_pause and on_resume paired. However, I can see why it would be a problem if on_resume was called in response to clicking 'stop execution' in a paused state. It might break too much outside code, but it's interesting to think about pausing and resuming as transitions to a virtual 'paused' state. Instead of calling on_pause, pausing would call on_exit. Similarly on_resume would call on_enter. If that was the case, 'stop execution' for a paused state wouldn't need to pass control back to the state since it would have already exited. This might not cover all uses cases, e.g. a trajectory execution state might make a motion plan in on_enter but might be able to pause and resume motion along the same plan. Perhaps just having a default implementation for pause and resume in terms of enter/exit, e.g. def on_pause(self): self.on_exit()? That starts to get messy again when figuring out if state needs to be resumed before it is preempted.

Another possible solution might be to add an on_preempted function that would be called instead of on_exit. It seems likely that many users would forget to implement this, but it would allow choosing what should happen.

[RhysMcK]

On an older version (1.2.4) the on_exit was being called twice in some cases. For example, if a state inside a concurrency container had finished (and as such, had called its' on_exit) but was waiting for another state to finish before exiting the concurrency container, if it was preempted its on_exit would get called again.

I'm not sure if these changes were to fix this, if so, I believe there needs to be a check if a states on_exit function has already been called to not call it again when being preempted.