behavior_tree: migration to BT.CPP 4.5.x

[facontidavide]

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#4059
Primary OS tested on	Ubuntu 22.04, Rolling
Robotic platform tested on	none
Does this PR contain AI generated software?	No

---

Description of contribution in a few bullet points

Moving from BT.CPP 3.8 to BT.CPP 4.5 (latest in Rolling).

TODO:

• Compile without errors.
• Fix failing unit tests.
• Handle NodeStatus::SKIPPED in Control Nodes and Decorators.
• Add documentation in "Migration guide".

List of changes:

• XML: adding BTCPP_format="4" to root and removing the redundant main_tree_to_execute
• blackboard->get is [[nodiscard]] in BT.CPP 4.x.
• Fix the different behavior of ReactiveSequence and ReactiveFallback
• tickRoot renamed to tickOnce

Description of documentation updates required from your changes

None of these changes should affect the user's code, nor in terms of Behavior or API.

But we need to add the migration guide to the website.
I will also need to update mine here: https://www.behaviortree.dev/docs/migration

---

Future work that may be required in bullet points

1. Groot2 publisher to be added to the BT executor.
2. Reminder that using blackboard->get prevent the use of SubTrees, but not the topic of this PR.

For Maintainers:

• Check that any new parameters added are updated in navigation.ros.org
• Check that any significant change is added to the migration guide
• Check that any new features OR changes to existing behaviors are reflected in the tuning guide
• Check that any new functions have Doxygen added
• Check that any new features have test coverage
• Check that any new plugins is added to the plugins page
• If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

[SteveMacenski]

The fastest I've ever reviewed 93 files 😆 I'll pick up the rest tomorrow

[facontidavide]

I am realizing that this is more work than I expected and converted this to DRAFT.

I will keep you posted on the progress; in the meantime, I added a list of TODOs in the description above

[SteveMacenski]

OK, LMK explicitly when you want me to review again, I'll ignore updates until then

Was tickRoot just renamed to tickOnce or is there a real behavioral change you're making there?

[mergify]

@facontidavide, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

[facontidavide]

Was tickRoot just renamed to tickOnce or is there a real behavioral change you're making there?

tickRoot now offers more options, being tickWhileRunning my suggested default.
tickOnce is the equivalent to the old behavior.

[facontidavide]

@SteveMacenski I believe that the tests on CI might be flaky, and I can not spot any relationship between those errors and my changes. Can you?

[SteveMacenski]

@facontidavide your branch may be out of sync by several weeks. I fixed the spin tests if you rebase (something in the rclcpp libs changed in rolling and exposed some issues that this is just an application manifestation of). That spin test failure and the WPF flaky test were resolved in mainline - but either way you're probably right if you're only seeing those 2 that its not related to you. But, I do want to see coverage metrics after this transition, so rebase so that it'll generate

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.04%. Comparing base (55c952d) to head (4122287).
Report is 3 commits behind head on main.

❗ Current head 4122287 differs from pull request most recent head 76a2b67. Consider uploading reports for the commit 76a2b67 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4128      +/-   ##
==========================================
- Coverage   90.11%   90.04%   -0.07%     
==========================================
  Files         432      432              
  Lines       19406    19407       +1     
==========================================
- Hits        17487    17475      -12     
- Misses       1919     1932      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[facontidavide]

@SteveMacenski waiting for CI to confirm but tests should pass now.

I added all the changes that I wanted to, also some test cases.

let me know what you think. If you need more information about the SKIPPED status, just ask

[SteveMacenski]

There's a DWB test that seems to be failing deterministically on this PR (2/2) which is odd. Its a known flaky test, but flaky means like 1/500 I see it, seeing it 2x in row here is extremely unusual. But, I think safely not your fault unless it fails for a third time right now.

Let me know when you're ready.

[facontidavide]

Code is ready to be reviewed, but I will investigate that

[SteveMacenski]

Please explain for my knowledge + file a separate ticket for this so its not forgotten. Also, why twice?

[facontidavide]

the two lines are not the same :)

The explanation can be found here: BehaviorTree/BehaviorTree.CPP#755

In short, I changed the way these nodes work, but that change made things worse, so I am reverting it in 4.6.

[facontidavide]

I added this specifically because you mentioned that you don't want to use the "latest version".

This is the workaround, if you want to use 4.5 (a small price to pay)

[SteveMacenski]

OK - please file a separate ticket as a reminder to us both about this - we don't accept inline FIXME / TODOs as proper documentation of issues (there are some in the codebase, but those all predate my tenure)

[mergify]

@facontidavide, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

[SteveMacenski]

Last change - otherwise I went through the rest of the files and I approve

[SteveMacenski]

Question: What if a post-callback fails? The skipped is for pre-conditions not being met, do post-conditions throw or something? (also not mentioned in the v3 to v4 guide what is SOP)

[mergify]

@facontidavide, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

[fmros]

Do you plan to backport this PR to humble? Thanks

[SteveMacenski]

No, its not ABI compatible.