BehaviorTree / BehaviorTree.CPP

Behavior Trees Library in C++. Batteries included.
https://www.behaviortree.dev
MIT License
3.02k stars 661 forks source link

Default Input port values #56

Closed v-lopez closed 5 years ago

v-lopez commented 5 years ago

Talking about ver_3

InputPort could be easily extended to have an optional default value (in the same sense as Groot custom node has).

When calling getInput, if nothing is found, and a default value is specified, it is returned.

For BidirectionalPorts, I guess it would only be used for the "input" part of the port. Although is debatable.

I can make a PR with this if you are interested.

facontidavide commented 5 years ago

be patient, I am working like crazy to complete ver_3 and this option will probably be there... at some point ;)

v-lopez commented 5 years ago

Thanks! If you need a hand let me know!

facontidavide commented 5 years ago

I just pushed a commit to address this. Remember that we have an "intrinsic problem":

But...

In other words, we might configure Groot to add a default value, but I am not 100% sure this can be enforced in the C++ side. I have to think about it.

v-lopez commented 5 years ago

I was thinking about modifying the CreatePort and similar functions to allow specifying there the default value. I shouldn't have mentioned Groot since it's not strictly related to that.

So I can do something like: BT::InputPort<double>("aruco_size", "Size in meters of aruco marker", 0.1)

This allows me to reuse a node for different aruco sizes, even though 90% of the time I'll just use the same size which is the default one.

facontidavide commented 5 years ago

unfortunately the default MUST be expressed as string, not the native type, because otherwise there is no way to make it work with Groot reliably.

So you must do instead

BT::InputPort<double>("aruco_size", "Size in meters of aruco marker", "0.1")

On the other hand, I might use std::to_string() under the hood and if it is defined for a particular type, it works, otherwise it doesn't compile.

In your example, it would work just fine for doubles.

v-lopez commented 5 years ago

I believe that's a good compromise, if we want to end the discussion here for now.

But if you want to go on a bit further, I'm not sure I agree with the connection between default in BT and Groot.

IMO the default values of the Nodes at C++ level should not be displayed in Groot. If you want to use the default values, you just leave the input blank on Groot.

As far as I understand, with what we're discussing, if I create a node with Groot of type ArucoDetector, in the GUI i'll see aruco_size and the default value 0.1. And when I save it, this 0.1 will be written to the xml.

But if I later want to make a global change and modify the default value for aruco_size, all my Groot created trees will have to be manually updated. Whereas if it was blank, I would not need to modify any xml.

hlzl commented 5 years ago

be patient, I am working like crazy to complete ver_3 and this option will probably be there... at some point ;)

Hi Davide,

sorry that this is slightly off-topic but I just saw you're comment and was wondering if there is a rough schedule on when ver_3 will be released? I really like what you're doing here, but as I understood the new version will not be backwards compatible and it would be interesting to know if the release is close of will still take some time. Please take your time though :-)

facontidavide commented 5 years ago

@v-lopez Take a look to this commit https://github.com/BehaviorTree/BehaviorTree.CPP/commit/5fea315bed0be5ce0095596ad521d1b3953c9d1d

The next Monday I will write a unit test to see if it works correctly

facontidavide commented 5 years ago

@hlzl there are huge differences between version 2 and 3 and version 2 will not be supported anymore. I will release version 3 the 1st of march, if everything goes according to the plans.

v-lopez commented 5 years ago

Awesome @facontidavide , everything is looking great!

shivaang12 commented 4 years ago

@facontidavide I am using default value for input ports but couldn't able to make it work. Can you point me to unit test which test this feature?

Thanks!