robodrome commented 9 months ago

Upgrade to `Behaviortree.CPP v4`

Overview

Changes have been made across the plansys2_bt_actions, plansys2_executor, and plansys2_tests packages. The updates include transitioning from behaviortree_cpp_v3 to behaviortree_cpp and updating behavior tree configurations and API usages. Note that ZeroMQ dependencies have been removed (for now) because v4 uses a different mechanism for that.

Changes Made

Transition from `behaviortree_cpp_v3` to `behaviortree_cpp` (v4)

CMakeLists.txt: Updated find_package calls across all affected packages to use behaviortree_cpp instead of behaviortree_cpp_v3.
package.xml: Modified package dependencies to reference behaviortree_cpp in place of behaviortree_cpp_v3.

Removal of ZeroMQ Dependencies

CMakeLists.txt: Removed calls to find and link ZeroMQ libraries.
package.xml: Eliminated references to libzmq3-dev and other ZeroMQ-related dependencies.
Source Files: Deleted all conditional compilation blocks and configurations related to ZeroMQ, streamlining the codebase.

Behavior Tree Configuration and API Adjustments

Include Statements: Updated all include directives from behaviortree_cpp_v3/* to behaviortree_cpp/* to reflect the new library.
Behavior Tree Nodes and Calls: Adjusted behavior tree node configurations, types, and method calls to comply with the updated API and functionality of behaviortree_cpp.
Behavior Tree XML Files: Modified behavior tree XML files to use the updated Parallel node attributes (success_count and failure_count) and added the BTCPP_format="4" attribute to indicate compatibility with the latest BehaviorTree.CPP format.

Affected Files

CMakeLists.txt and package.xml in plansys2_bt_actions, plansys2_executor, and plansys2_tests packages.
Various .cpp and .hpp files within plansys2_bt_actions and plansys2_executor packages, specifically those related to behavior tree node implementations and configurations.

Tests

All tests pass

Conclusion

These updates modernize the ROS 2 Planning System's usage of the BehaviorTree.CPP (v4) library, remove outdated dependencies, and enhance the system's configurability and maintainability. This transition supports the latest developments in behavior tree-based planning and execution for robotic applications.

fmrico commented 9 months ago

Except the one commented out in executor_test.cpp namely TEST(executor, action_timeout). Note that this test also fails in the rolling branch, so this is not caused by this upgrade.

Yes, I have just fixed this in a PR, and accepting them has produced some conflicts. Could you fix them, please, @robodrome ?

robodrome commented 9 months ago

I applied your changes manually to my branch. The executor test succeeds now when i run it locally. PS. i committed with my other github account i see.

fmrico commented 9 months ago

LGTM

Great work @robodrome

Merging!! 🚀

robodrome commented 9 months ago

Thanks! I also updated the example here: https://github.com/PlanSys2/ros2_planning_system_examples/pull/43#issue-2147708582 That should be passing next workflow run.

DaniGarciaLopez commented 9 months ago

Thanks @robodrome for this PR! However, to my knowledge, BehaviorTree.CPP 4 is only compatible with Groot 2, which is capped at 20 nodes for real-time monitoring under the free license. This could result in downgrades for projects not using the Groot 2 PRO license. Have you considered this? Otherwise, I guess we should keep using BT 3 or maintain both versions.

fmrico commented 9 months ago

Maybe it would be nice to support both versions in any way 🤔

robodrome commented 9 months ago

Maybe it would be nice to support both versions in any way 🤔

This should be possible. It would make things more complex though. I guess you would have to add compile time definitions in the code.

It's also possible to just create releases.

robodrome commented 9 months ago

Thanks @robodrome for this PR! However, to my knowledge, BehaviorTree.CPP 4 is only compatible with Groot 2, which is capped at 20 nodes for real-time monitoring under the free license. This could result in downgrades for projects not using the Groot 2 PRO license. Have you considered this? Otherwise, I guess we should keep using BT 3 or maintain both versions.

I understand your point. I also dislike the fact that Groot has been abandoned. I can partly understand that, but it kind of breaks the open source mindset.

Another solution could be to adapt the Groot tool to have some basic btcpp4 functionality and leave the extra (enterprise) features for Groot2? Without getting in the way of @facontidavide

facontidavide commented 9 months ago

Jumping into this conversation :)

@DaniGarciaLopez I am glad that you consider the monitoring feature valuable!

If anybody wants to fork the old Groot to support logging from BT.CPP 4, you can and you are welcome. It is not even that much work. I really mean it, I am not being sarcastic :smile:

But I will not do it myself. I will not review your PRs, if you submit them. I will invest 0% of my time in the old Groot.

I work on many projects, for free, in my spare time: BehaviorTree.CPP, PlotJuggler, DataTamer, Bonxai, MCAP editor...

Groot2 is the approach I decided to adopt to give a sustainable future to BehaviorTree.CPP (where "sustainable future" means a way to pay my rent, groceries, etc.).

If a world where companies support economically open-source developers, I would not do that, but we don't live in that world.

robodrome commented 9 months ago

Jumping into this conversation :)

@DaniGarciaLopez I am glad that you consider the monitoring feature valuable!

If anybody wants to fork the old Groot to support logging from BT.CPP 4, you can and you are welcome. It is not even that much work. I really mean it, I am not being sarcastic 😄

But I will not do it myself. I will not review your PRs, if you submit them. I will invest 0% of my time in the old Groot.

I work on many projects, for free, in my spare time: BehaviorTree.CPP, PlotJuggler, DataTamer, Bonxai, MCAP editor...

Groot2 is the approach I decided to adopt to give a sustainable future to BehaviorTree.CPP (where "sustainable future" means a way to pay my rent, groceries, etc.).

If a world where companies support economically open-source developers, I would not do that, but we don't live in that world.

Good to hear from you.

I can totally understand that. We are working in a niche, but we all know that Behaviortree.CPP is being used in commercial products without them sponsoring the project. I know for a fact that e.g. Xiaomi uses BT.CPP in - at least some of - their robot vacuum cleaners.

That's why I think we shouldn't get in your way. Your work is highly appreciated. So perhaps we could adopt the Groot to a Groot-lite version. Just for some basic functionality. Like you said, it shouldn't be that much work.

DaniGarciaLopez commented 9 months ago

We highly value the monitoring feature! That's one of the main reasons we haven't updated to v4 yet, although some of the new features would be very beneficial. For startups and small teams like ours, the cost of the PRO license for Groot 2 is somewhat pricey, and the 20 node cap feels restrictive. However, we fully appreciate your stance, @facontidavide, and we deeply value all your contributions to the robotics community!

I've also reviewed your latest PR for Nav2 migrating to version 4. Is Nav2 planning to support both BT.CPP versions, @SteveMacenski? Perhaps it would be beneficial for both packages to consider lifting the 20-node restriction while retaining all other features that are less critical in the PRO license. What are your thoughts on this @facontidavide?

If this doesn't happen, adapting Groot 1 to work with BT 4 seems like the most viable option. Although personally, I don't have enough time to do it now, perhaps in the future.

facontidavide commented 9 months ago

Sorry @DaniGarciaLopez , but I can not remove the 20 nodes restriction, otherwise there will be little incentive to upgrade.

I am writing this for everyone reading this thread, not just you:

I always knew that this limitation would not make some users happy, but if Monitoring and Log Replay do save you development time, then I am sure we can find an agreement.

Said differently:

option A: "no, those features do not save me and my team time". In that case, you don't care if those features are available, since it makes little difference.
option B: "yes, they save me and my team a lot of time". Therefore, that saved time has an economic value and I invite you to reach out and discuss the available options (if your company is very small, I will be happy to offer a discount).

Use the email dfaconti@aurynrobotics.com

fmrico commented 9 months ago

Hi all,

The migration to BT.CPP 4 lets us use all the new features that @facontidavide includes. The current migration in this PR is very straightforward, but soon, we will start to take advantage of these new features. Also, I suppose Davide will not support both versions forever.

I respect Davide's opinion and agree with his business model. If something is critical for making money in your company, paying for that can be part of your equation, and I know Davide is very flexible with small companies and academia.

I don't quite understand why visualization a plan is so critical for you. The Behavior Tree that encodes the plan is really complex to monitor with Groot, and PlanSys2 already has tools for monitoring the execution of a plan. If it is because of the Behavior Trees in your actions, you can continue using BT.CPP 3 on them.

robodrome commented 9 months ago

For what it is worth, I agree with @facontidavide and @fmrico. This is your call @fmrico.

PlanSys2 / ros2_planning_system