search

splintered-reality / py_trees_ros

ROS extensions and implementations for py_trees
Other
143 stars 40 forks source link

RuntimeError: if the child already has a parent #205

Closed kijongGil closed 1 year ago

kijongGil commented 1 year ago

Hi.

I've used py-trees-ros for my project and it worked perfectly well on ROS2 dashing. Recently, however, the system needs to be upgraded to ROS2foxy, and the error "Runtime Error: If the child already has a parent" occurred.

I know how to resolve this error by not using one behaviour on multiple composites, but I wonder why you need this error message. In my case, I create a 'running' behaviour in the tree and use it for multiple events when needed. I know that if I use it like that, the pie-tree-ros-viewer will make a strange-looking diagram, but I don't think I need to make the same behaviours that work the same.

Please let me know if I misunderstood the concept of BT or missed the specific issue.

Thanks!

stonier commented 1 year ago

There's quite a few elements that rely on that particular behaviour tree constraint (it's not specifically a py_trees constraint).

As you've observed, viz is one, tip is another. Both of those are fairly mild in terms of ramifications, but the primary reason for it is that unless you're extremely careful you can easily introduce bugs into the flow of the tree that are destructive / hard to debug - especially if you have a single behaviour flip-flopping it's state across a single tree tick.

I had a robotics group in Cambridge naively reusing behaviours across a tree (thinking that reuse was a good thing) and they suddenly found themselves confronted with such problems.

You might get away with reusing a Running behaviour like this (flip-flopping between INVALID and RUNNING might still cause issues), but in general, we felt it was better to just help people avoid this particular set of problems. It wasn't designed to work this way and it's impossible to guarantee it won't cause problems. Decorators and Composites haven't been designed to be aware of, e.g. a RUNNING child suddenly becoming INVALID due to a change earlier in the tree tick. It would be difficult/impossible to define proper behaviour for these cases.

A few extra lines of code is well worth the peace of mind :)

stonier commented 1 year ago

but I don't think I need to make the same behaviours that work the same.

Key point here is that these behaviours have state. Even for the Running behaviour, it has a status which can be INVALID or RUNNING. The same behaviour type can and will need to be able to manage different states in different parts of the tree. If the behaviour was a strictly stateless, you'd have no problem.

This is somewhat similar to the idea of re-entrancy. Tree behaviours are not re-entrant over the course of a single tree tick.

kijongGil commented 1 year ago

Thank you for your reply. As you said, I had a problem with Running behaviour status having INVALID, and I solved this problem by redesigned the tree. However, it seems better to modify it not to re-entrancy. Thank you :)