Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration to use behaviortree_cpp #253

Open
ana-GT opened this issue Dec 7, 2022 · 8 comments
Open

Migration to use behaviortree_cpp #253

ana-GT opened this issue Dec 7, 2022 · 8 comments

Comments

@ana-GT
Copy link

ana-GT commented Dec 7, 2022

Hi, I was wondering if there are any plans in the near future to switch the dependency of ros2_planning_system from behaviortree_cpp_v3 to behaviortree_cpp. Thanks!

@fmrico
Copy link
Contributor

fmrico commented Dec 7, 2022

Hi @ana-GT

Yes, we had a short discussion about this in #130 (comment)

The new version of Behavior Trees offers some exciting features that I want to include in PlanSys2. I had no time to work on this, but it is definitely in my plans. If you want to contribute in this direction, even with a direct migration, I will be happy to help you.

Best
Francisco

@ana-GT
Copy link
Author

ana-GT commented Dec 8, 2022

Hi Francisco,

The migration looks straightforward, unless I am missing something. I forked and created a branch named bt_v4 with my changes: master...ana-GT:ros2_planning_system:bt_v4 . Changes can be resumed as:

  1. Replace behaviortree_cpp_v3 with behaviortree_cpp
  2. Replace NodeConfiguration with NodeConfig.
  3. Replace tickRoot() with tickOnce().
  4. Add BTCPP_format="4" wherever appropriate (in tree's xml files and in code that create trees on the fly).
  5. Replace success/failure _threshold with success/failure _count for the Parallel nodes that are created during BT construction.

The changes above were exclusively for migration purposes. I also added what I think is a fix for the is_valid_domain function in the popf_plan_solver. In its current form, if the user sends a valid domain and an empty problem, the function returns false, where I think it should return true instead. A unit test was failing for this reason.

Tested with the unit tests and with an example of my own that has 2 robots running in parallel, so I think this is working fine. I am happy to send a PR if that is appropriate at this time (wasn't sure given that the current ROS2/Nav2 is dependent on bt_cpp_v3).

Best,

Ana

@sarcasticnature
Copy link
Contributor

I would have some slight concerns about migrating quite yet, as according to Davide's ROSCon talk v4 is currently in alpha/beta form. That means that there is still the potential for significant (possibly breaking) changes that could make early adoption a bit of a pain

While I agree that there are some exciting features, I'm not sure that the time is right as of yet (but that is a personal opinion).

@ana-GT ana-GT changed the title Migation to use behaviortree_cpp Migration to use behaviortree_cpp Dec 8, 2022
@sarcasticnature
Copy link
Contributor

This issue faces many ROS projects, which also need to consider when to make the switch. While looking through some of the changes, I stumbled across two relevant threads in Nav2 and BT.CPP itself

Personally I agree with the idea to hold off on switching until Iron is released, @fmrico what are your thoughts?

@fmrico
Copy link
Contributor

fmrico commented Dec 14, 2022

Hi!!

My opinion:

  1. Let's wait for Iron
  2. Upgrade to 4.0 only if we take advance of the new functionalities. This requires a more extended discussion of what is new and how to use it.

best

@facontidavide
Copy link

Hi.

My plan is to release version 4.1 in Rolling and Humble the next week.
I will also coordinate with Nav2 to make sure that they use version 4 in Iron.

If you need assistance to migrate, please let me know.

@ana-GT
Copy link
Author

ana-GT commented Mar 17, 2023

Great news! Thanks for the update. Will keep an eye in Nav2 (and possibly PlanSys2 :) ).

@robodrome
Copy link
Contributor

I made a PR upgrading from BT.CPP v3 to v4. #293 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants