Composites do not check return status from self.update() call in tick()

[JimWhiteheadUCSC]

All Composite classes have logic inside their tick() function which calls self.update() before iterating through children and calling tick() on them.

The return value for the call to self.update() is not checked. Hence if the actions taken inside self.update result in the status being set to Status.FAILURE or Status.INVALID, this does not change the behavior of the Composite. In particular, it seems like the correct behavior would be to skip processing of all of the children, since the current node has expressed an error state.

[stonier]

Aha, you've opened the door on a skeleton. We had the update() method call in there from day one, because, it probably should be there, but over the course of a few years and three different teams, we never made use of it. Not surprising that it has some warts.

I've always wanted to rip it out, but that would make a special case of the composites since each one is a behaviour and thus should have that functionality. So a question then becomes, logically what should it be doing?

Some thoughts:

• If you are going to return on SUCCESS||FAILURE without running over the children, then it is imperative that any RUNNING children are terminated so you don't leave dangling runners.
• Does it make sense to have the update() return with RUNNING only to tick the children who might ultimately override this early status?

Could you shed some light on your use case? That might help understand where this needs to go.

[JimWhiteheadUCSC]

Oooh, if we're battling a skeleton, that must make us pirates!

My use case involves using behavior trees to interact with a web site via the Selenium library. I have a top level container (Sequence) that establishes a connection to a web browser via Selenium. This creates a session, whose id is written to the blackboard. Children of this node (also typically Sequence containers, each representing interactions with a specific web page) then use the session information to interact with the browser session via Selenium.

Creation of the web browser session takes place in either setup() or initialize(), whichever is called first. Since the interactions with the web site are repeatable, the tree can re-execute itself. Runs 2..n of the behavior tree will call initialize() but not setup(). Then, update() checks to see what happened in either setup() or initialize(), and returns the appropriate status code (SUCCESS or FAILURE).

Since this is really about initialization activity, the status code is generated before any child activity begins, and hence I'm not concerned about terminating running children. As I understand the current code, the container's update() is only called before any children are ticked.

It seems to me that if the container's update() returns RUNNING, then it should just proceed to ticking its children. Only in the situation of FAILURE || INVALID should it not proceed to ticking children.

A few thoughts.

• I suspect this kind of session-based behavior is relatively common (e.g., a database session, or perhaps a connection to ROS), so perhaps there is another preferred approach here. I could certainly imaging adding a sibling node that handles the initialization via a regular Behaviour. But, this would require adding a parent container, which feels unnecessary.

• Since the session creation is happening via setup() or initialize(), one could imagine just checking the return values from these. I'll note that the current code doesn't check the return value from the initialize() call inside tick(), which also feels like a problem. How should a failed initialization affect composite behavior? But, just fixing this doesn't completely solve my problem, since initialization can be happening in either setup() or initialize(), and only one of these is called from tick().

[stonier]

Some thoughts how I'd usually do things which may help.

Composite Customisation

The reason we have not customised composites is because they are critical to visualising (either in design or at runtime) the flow of the graph. e.g. 'see a Selector, know it's a priority decider'. 'see a sequence, know it's a sequential work container'. As soon as you add custom magic under the hood, the visualised graph flow becomes unreadable.

The same reason applies to avoiding proliferation of composites

Sessions Always Available

i.e. you want the session available to the tree from it's first tick to it's last. I'd simply create a behaviour that runs up-front and underneath a root that is a parallel.

[||] Root
  --> Session Manager
  [-] Rest of The Application

With session manager doing session creation in setup and connection checking in update:

class SessionManager(py_trees.behaviour.Behaviour):
    def __init__(name="SessionManager", session_properties={}):
        super().__init__(name=name)
        self.session_properties = session_properties
        self.session = None

    def setup():
        try:
            self.session = create_selenium_session(session_properties)
        except Exception:
            self.session = None
            raise  # might want to provide a more informative exception/message here

    def update():
        # check if the session has been created
        if self.session is None:
            raise RuntimeError("No session, did you run setup() successfully?")
        # check if the session is still alive (depends if selenium has something for this)
        if not self.session.is_alive:
            raise RuntimeError("Session died")
        return py_trees.common.Status.SUCCESS

    def terminate()
        close_selenium_session(self.session)

The reason for doing it in setup here is that session creation has no impact on decision making flow, and oft times takes more time than you would wish in a tree tick. It also cleanly separates init flows from execution flows.

If you pack your root into a py_trees.trees.BehaviourTree, then you get convenient handles for managing your setup:

tree = py_trees.trees.BehaviourTree(root = root)
try:
    tree.setup(timeout=15)  # automagically finds your session manager setup()
    tree.tick_tock(period_ms=100)
except SelenimunException as e:
   console.logerror("Failed to create a Selenium session, aborting [{}]".format(str(e))
   sys.exit(1)
except RuntimeError as e:
   console.logerror("Tree execution failed, aborting [{}]".format(str(e))
   sys.exit(1)

If the validity of your connection is up-down and you don't want to abort on failure, drop the session.is_alive check here and create a behaviour out of that that returns FAILURE||SUCCESS depending on connection validity. You can then use it as a guard for various tree events.

Another benefit of session management as a behaviour is that it visualises it clearly for you on your tree graph.

Sessions On-Demand

This is more complicated, especially if your session creation takes a considerable amount of time. If not, you can implement session creation in initialise() and session closure in terminate() OR create a behaviour for session creation and another for session termination (possibly transferring data from one to the other across the blackboard). If your session takes a while, you'll have to initiate the session in initialise() and check for it's completion in update() - with RUNNING while pending and SUCCESS on completion. Again, under a parallel in front of the rest of your subtree that governs what happens when in session.

If you do wrap session creation and termination in one behaviour you get one nice thing for free - higher priority tree interrupts will automatically trigger the termination of your session.

[stonier]

Aging out. The approach above, or the more recent eternal guard can and should solve this problem. #375 in fact, documents as much in the update() method and completely unwires it from established composite tick() implementations.