How to execute "cleanup action" after halting nodes

[alsora]

Let's consider the following scenario:

• we have an action node "drive robot" which moves a robot forward of a certain distance (this node will take a while to complete and it will return RUNNING until it's done).
• we have an action node that turns on or off the lights (for the sake of simplicity, let's say that this is a SYNC action).

The robot should have the lights off during normal operations, but they should be on while executing the "drive robot" action node.
A trivial implementation would be the following:

The last action node in the sequence (i.e. the one setting the lights to OFF) is what I call a "cleanup" action".
It must be executed (no matter what) when we stop running the "drive robot".

This can be problematic if using the trivial tree shown above as part of a larger behavior, because the execution of "drive robot" can be halted from many different parts of the behavior tree (e.g. this subtree could be inside of a reactive sequence that checks for obstacles).

I can think of some solutions, all which seem suboptimal:

1. Use post conditions such as _onSuccess=" lights_on:=true " and _onSuccess=" lights_on:=false " when we execute the different SetLights nodes. Then the BT can check this and call "turn off lights" if it realizes that "drive robot" was halted and we still have lights on. The problem here is that if there are multiple places where this can be halted from (e.g. nested reactive behaviors), we need to check the blackboard variable in multiple places.
2. Create a new control node that has 2 children: the first is the main child, the action to execute (drive robot), while the second is the cleanup action (turn off lights). The cleanup action is executed when the main child reaches a terminal state or when the control node is halted.
   At a first glance, this second approach seemed to me the ideal solution to the problem, but while implementing it, I noticed several issues.
   2.a) we are adding complexity to the halt operation; maybe other nodes would expect it to be instantaneous.
   2.b) how to deal with a FAILURE in the cleanup action? the halt method does not return anything. we could use a blackboard variable, but then we would be back to the scenario of 1). Ignoring the failure is equally bad, because then we would risk of "keep running with the lights on", whereas different parts of the behavior may decide to handle a failure there differently (e.g. retrying or aborting everything).

Is there a recommended approach to deal with this type of situations?
Thank you

[alsora]

This, unresolved, stack overflow question deals with the same problem https://gamedev.stackexchange.com/questions/189352/behaviour-trees-how-to-clean-up-when-a-sequence-is-interrupted

They propose a variation of the solution (1) I described above, i.e. whatever new behavior will start running after halting, will need to check its pre-conditions (e.g. are the lights in the expected configuration?) before starting.
I think that this makes sense semantically, but it may still require to add this check to a lot of places (that maybe shouldn't care about the fact that the behavior they halted had modified something and it didn't clean up).

[facontidavide]

I have been thinking about this and it lookas as a common problem.

To solve this and other problem, why not implement a SequenceAll Control Node?

The logic would be:

• execute ALL the children
• return FAILURE if at least 1 of them failed, SUCCESS otherwise.

What do you think?

[alsora]

What would this SequenceAll Control Node do if it receives an halt signal?
I think that making sure that the cleanup is still executed after an halt is the main pain-point.

[alsora]

I'm currently using a ActionWithCleanup Control Node with the following semantics:

• takes 2 children
• first child is the main action to execute, second child is the cleanup action
• when the main action terminates (success, failure or halt) the cleanup action is executed
• the cleanup action must be synchronous (i.e. it can't return RUNNING)
• if the cleanup action returns FAILURE, then throw a runtime error
• the output of the control node will coincide with the output of the main action

It's definitely not ideal.
More complex solutions would require to let nodes register "cleanup actions" in the tree.
When nodes are halted, whoever halted them will have to execute the registered cleanup action (if any) before proceeding with its business
This means that the cleanup is now executed in the subtree/context of the node that triggered the halt (rather than being executed as part of the halt operation itself as with the node I described before).

[facontidavide]

I Think Both option are equally valid, because if you pass only two children to my hypothetical SequenceAll, the overall effect would be mostly the same.

Solution 1: pre/post conditions

Solution 2: IfTheElse

Solution 3: SequenceAll (proposed)

[alsora]

Thank you for the illustrated solutions.
However, I still think that if the Sequence (or SequenceAll) is halted while it's executing DriveRobot, the SetLight cleanup action won't be executed.

That would require an explicit handling of the halt signal, which Sequence and IfThenElse don't do.
Would the new SequenceAll handle halt differently?

[facontidavide]

you are correct about halting, indeed, I did not think about that....

But given the name All, maybe the logic is that halting will be propagated to the RUNNING child, but the sequence will continue with the next on.

[zflat]

I think that due to the way behaviortree_cpp halts the tree, the best way to ensure cleanup taking halting into account is to perform this type of "transactional" functionality in a decorator.

The decorator will do the "setup" first, then tick the single child, then do "cleanup" when the child is finished. When the decorator is halted, it will halt the child (which is handled by calling the base class DecoratorNode::halt()) and then do the "cleanup" work.

If there are multiple setup and corresponding cleanup steps to perform they can be done in nested decorators. For example, there could be the following decorators:

• WithLightsOn: Sets the light on when running and off when not running
• WithTankPressurized: Builds pressure then runs the child node and releases pressure when the child finishes
• WithBrakeReleased: Sets the break released when the child node is running and then engages the break when the child node finishes

They would be nested like this:

<WithLightsOn>
    <WithTankPressurized pressure_setpoint_kpa="600" pressure_release_point_kpa="30">
        <WithBrakeReleased>
            <DriveRobot />
        </WithBrakeReleased>
    </WithTankPressurized>
</WithLightsOn>

Using a decorator like this ensures that halting will do the cleanup we expect. Using an ActionWithCleanup Control Node as described in #547 (comment) has the issue that the cleanup child can't be a sequence or other node that breaks the semantics of what is needed by ActionWithCleanup which seems hard to verify or enforce.

[gislers]

Same issue here. Let me share my thoughts:

• Decorators: I like the idea proposed by @zflat using the decorator nodes. However, I'm using this library in combination with ROS 2 and nav2 with existing implementations of action and decorator nodes. Mixing those two doesn't seem straightforward in that context.
• Universal halt: Then I was wondering why we don't simply send the halt to all nodes, not only RUNNING ones. I do understand that halting is a concept tied to asynchronous nodes, so it makes sense. But besides this, are there any reasons why we couldn't send halt to all nodes? Benefit here would be that every leaf node could easily implement its own cleanup logic in the halt.
• Signalling / No halt: Then I thought we could use some signalling mechanism in order to stop the robot. Think of every asynchronous action as a thread (just as a mental model): It runs until completion or it joins when we signal it from the outside (e.g. through blackboard). So in the original example, when the robot stops the movement (either because the task is done, or it is signalled to exit), we return SUCCESS and then do the cleanup. The benefit of such an approach would be that we use the standard logic of the BT, and it remains responsive in all cases. Further, we don't hide the cancel logic in the halt method, but make the intent obvious in the tree. I think this would be a clean solution in the given context, but it seems like a heavy paradigm shift (getting rid of the whole halting mechanisms) and it surely has its pitfalls, too.
  
• SequenceAll: I think SequenceAll or ActionWithCleanup is probably the easiest solution (I prefer SequenceAll proposed by @facontidavide as it is a little more general). What I don't like is the fact that the cleanup might be a longer action (this happens to be my use case). And this means that we block in the halt method waiting for the cleanup action to finish.

[doisyg]

Universal halt: Then I was wondering why we don't simply send the halt to all nodes, not only RUNNING ones. I do understand that halting is a concept tied to asynchronous nodes, so it makes sense. But besides this, are there any reasons why we couldn't send halt to all nodes? Benefit here would be that every leaf node could easily implement its own cleanup logic in the halt.

I believe this is already the case with the nav2 wrapper.
See https://github.com/ros-navigation/navigation2/blob/4e62a89d01aeb3cc88562a559cd142688305272b/nav2_behavior_tree/include/nav2_behavior_tree/bt_action_server_impl.hpp#L303
It may be hacky, but if you then override the halt() method of a BT::ActionNodeBase, then you can do your cleanup there. You will be guarantee that it is called at the end of the nav2 action (no matter the end state of the tree).

[gislers]

Hm, interesting. But I think that this still only halts the RUNNING nodes because the control and decorator nodes won't propagate that halt signal to non-RUNNING leafs. See control node base implementation for example:

BehaviorTree.CPP/src/control_node.cpp

Line 42 in 1fcb624

if(child->status() == NodeStatus::RUNNING)

Edit: I can see your point now. The recursive visitor here makes sure that all the nodes are halted. So, this is certainly useful when the whole tree stops. But in case of a sequence for example being halted by some reactive control node, I believe this won't help.