Open discussion: Ports and Subtrees about behaviortree.cpp HOT 15 CLOSED

@Thordreck @miccol @mjeronimo @Masadow @DJuego and anyone else...

I will keep this issue open until the 21th of January. You have 12 days to contribute with suggestions and share your opinion, afterward I will implement this new feature and I hopefully have version 3 ready and documented) by the 1st of February.

from behaviortree.cpp.

I like the syntax proposed.

Another option would be:

<BehaviorTree ID="main">
    <SequenceStar>
        <Walk>
           <port external="position_ID"   internal="target_pos"/>
           <port external="orientation_ID"   internal="target_angle"/>
         </Walk>
    </SequenceStar>
</BehaviorTree>

It's more verbose, but if you are performing lots of remappings, it can be better visually. Plus it gives you the option to add more attributes in the future if need be.

Regarding the value/reference semantic, I don't fully understand the question, what is {Walk::target}, does it reference an example that I may have missed?

from behaviortree.cpp.

I like it, but I wonder if, since the internal refers to a port, the syntax {target_pos} and {target_pos} should be used instead.

What about:

<BehaviorTree ID="main">
    <SequenceStar>
        <Walk>
           <input_port from="position_ID"     to="{target_pos}"/>
           <input_port from="orientation_ID"   to="{target_angle}"/>
           <output_port from="{error_code}"     to="{walk_error_code}"/>
         </Walk>
    </SequenceStar>
</BehaviorTree>

Added error_code to show the syntax.

from behaviortree.cpp.

Just added "scoped/hierarchical" blackboards.

What is left is to define the syntax and semantic of ports. Let's consider an example:
This is the current way to pass data to a SubTree (based on comments from @Masadow in #45):

<BehaviorTree ID="Walk">
    <SequenceStar>
        <MoveTo target="{target_pos}" />
        <FaceTo target="{target_angle}" />
    </SequenceStar>
</BehaviorTree>

<BehaviorTree ID="main">
    <SequenceStar>
        <SetBlackboard output_key="target_pos" value="position_ID" />
        <SetBlackboard output_key="target_angle" value="orientation_ID" />
        <Walk />
    </SequenceStar>
</BehaviorTree>

The problem with this was that the BB key {target_pos} or {target_angle} might be used somewhere else, not just in Walk.

Do not forget that, in this example, "position_ID" and "orientation_ID" are strings, but they could be pointers to other BB entries.

The natural way to look at the problem is to add ports to Subtrees as many of us agree. I propose this syntax:

<BehaviorTree ID="Walk" input_port="target_pos;target_angle" >
    <SequenceStar>
        <MoveTo position="{target_pos}" />
        <FaceTo  angle="{target_angle}" />
    </SequenceStar>
</BehaviorTree>

<BehaviorTree ID="main">
    <SequenceStar>
        <Walk  target_pos="position_ID" target_angle="orientation_ID" />
    </SequenceStar>
</BehaviorTree>

With the new implementation BB are scoped {target} inside Walk can not be seen "outside" Walk itself (no name collision then).

The redirection works as follows:

• Port position of MoveTo points to BB entry target_pos that is redirected to the value "position_ID".
• Port angle of FaceTo points to BB entry target_angle that is redirected to the value "orientation_ID".

Can any of you suggest a better syntax or you like this one?

Do you think that the "mapping" from {walk_target} to {Walk::target} should be value semantic (copy) or reference semantic (pointer) ?

I prefer value semantic, because it adds neglegible overhead in most cases and it is easier to implement.

from behaviortree.cpp.

Just to keep things more clear and visual.
I think this component diagram (UML) shows, more or less, what we want to achieve.

A Subtree contains an arbitrary numbers of Nodes (the diagram represent composition and dataflow, not the behaviort tree itself).
From the "outside" we can access only the blue ports, whilst the red one are 100% hidden and free of "name clashing".

from behaviortree.cpp.

@v-lopez

Regarding the value/reference semantic, I don't fully understand the question.

What I meant is...

So far we called "remapping" how a "port" in the C++ code is mapped to an entry of the BB.

For instance:

   target="{target_pos}" 

Means that port called "target" in the C++ code actually read/write the entry with key "target_pos" in the BB.
This Subtree we want to add another level of indirection, i.e. entry target_posin the scoped BB of Walk actually read write another entry in another BB, let's call it walk_targe_ID.

The question is: do we copy the content of walk_targe_ID into target_pos, or, we use some reference/pointer mechanism to avoid copying?

I invite the community to please prioritize correctness, usability and "the minimum surprise principle" above performance.

from behaviortree.cpp.

I would go with reference/pointer, because in some situations you may have parallelism and you want to update the contents of the entry from outside the subtree during it's execution.

Plus, if they were on the same BB, it would be reference/pointer right? I think it's less surprising if they have the same behaviour.

from behaviortree.cpp.

I would go with reference/pointer, because in some situations you may have parallelism and you want to update the contents of the entry from outside the subtree during it's execution.

I would go with reference/pointer too.

from behaviortree.cpp.

Soooooooooooooooo.....

The very basic unit test seems to work properly: see here.

https://github.com/BehaviorTree/BehaviorTree.CPP/blob/ver_3/gtest/gtest_factory.cpp#L170-L193

As you may notice this new syntax is used for Subtrees (suggested by @v-lopez )

      <SubTree ID="TalkToMe">
            <remap external="talk_hello" internal="hello_msg" />
            <remap external="talk_bye"   internal="bye_msg" />
            <remap external="talk_out"   internal="output" />
      </SubTree>

When the nodes in TalkToMe try to read/write a port locally, they are actually writing into the blackboard of the Maintree.

This should eventually solve all the problems that you guys reported ;)

On my side, I will spend the week doing more unit test to check corner cases and error report.

from behaviortree.cpp.

Looking great!

I'm migrating some of our smach_c code to BT using the ver_3 branch, so I'll let you know if I find any issues, I know it's still in development and things can break.

from behaviortree.cpp.

FYI:

I decided that I will also support this syntax:

<SubTree ID="TalkToMe" hello_msg="{talk_hello}" bye_msg="{talk_bye}" output="{talk_out}" />

That is more compact and readable when only 1 or 2 ports are remapped.

Implementation wise, I think I have done. I need to rewrite tutorials and documentation before the release (trying to release it the 1st of February).

from behaviortree.cpp.

I think it is done, eventually. You can check here https://github.com/BehaviorTree/BehaviorTree.CPP/blob/ver_3/examples/t06_subtree_port_remapping.cpp

I am 95% confident that version 3.0 beta is ready...

Thanks all of you for the great suggestions. Documentation an tutorials are also reasonably up to date.

@miccol it would be VERY nice if you in particular can review the new tutorial and provide some feedback.

from behaviortree.cpp.

@facontidavide sure, I will (hopefully in the next week)!

from behaviortree.cpp.

@facontidavide I have some feedback on the tutorial. Shall I send it over email or do you prefer an issue?
MC

from behaviortree.cpp.

probably email makes more sense. thanks

from behaviortree.cpp.