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.
Issue
We recently upgraded to latest version of Flexbe and noticed that
on_exitis no longer being called when a behavior is preempted. We have a number of states that send an actionlib goal inon_enterand cancel it if it's still running inon_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_exitwould be called exactly once for each call toon_enterso 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_exitfrom insideon_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_exitis still needed. I'm less sure abouton_resumesince 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_exitshould 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_enterandon_exitalways paired is helpful, it seems desirable to haveon_pauseandon_resumepaired. However, I can see why it would be a problem ifon_resumewas 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 callingon_pause, pausing would callon_exit. Similarlyon_resumewould callon_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 inon_enterbut 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_preemptedfunction that would be called instead ofon_exit. It seems likely that many users would forget to implement this, but it would allow choosing what should happen.