Migrate from deprecated BehaviorTree.CPP APIs

[ruffsl]

Looks like navigation2 is using a few deprecated APIs from BehaviorTree.CPP. Looks like there where some important changes from v3.3.0. The latest release as of writing is v3.4.0.

Need to update https://github.com/ros-planning/navigation2/blob/master/tools/ros2_dependencies.repos#L5 to the new version.

Building b2c1661 agents https://github.com/BehaviorTree/BehaviorTree.CPP/releases/tag/3.4.0

 ---> Running in 0dd8bbd22e7f
[connext_cmake_module] Warning: The location at which Connext was found when the workspace was built [[/opt/rti.com/rti_connext_dds-5.3.1]] does not point to a valid directory, and the NDDSHOME environment variable has not been set. Support for Connext will not be available.
Starting >>> nav2_common
Starting >>> nav_2d_msgs
Starting >>> nav2_gazebo_spawner
Finished <<< nav2_gazebo_spawner [0.91s]
Finished <<< nav2_common [1.30s]
Starting >>> nav2_msgs
Starting >>> nav2_voxel_grid
Finished <<< nav_2d_msgs [14.3s]
Starting >>> dwb_msgs
Finished <<< nav2_voxel_grid [16.2s]
Finished <<< dwb_msgs [23.4s]
Finished <<< nav2_msgs [53.8s]
Starting >>> nav2_util
Finished <<< nav2_util [23.1s]
Starting >>> nav_2d_utils
Starting >>> nav2_behavior_tree
Starting >>> nav2_lifecycle_manager
Starting >>> nav2_map_server
Starting >>> nav2_amcl
Starting >>> nav2_waypoint_follower
Finished <<< nav_2d_utils [8.92s]
Finished <<< nav2_lifecycle_manager [19.3s]
Starting >>> nav2_rviz_plugins
Finished <<< nav2_waypoint_follower [19.9s]
--- stderr: nav2_behavior_tree
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/decorator/rate_controller.cpp: In member function ‘virtual BT::NodeStatus nav2_behavior_tree::RateController::tick()’:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/decorator/rate_controller.cpp:89:52: error: ‘void BT::TreeNode::setStatus(BT::NodeStatus)’ is protected within this context
   89 |         child_node_->setStatus(BT::NodeStatus::IDLE);
      |                                                    ^
In file included from /opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/decorator_node.h:4,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/decorator/rate_controller.cpp:21:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/tree_node.h:166:10: note: declared protected here
  166 |     void setStatus(NodeStatus new_status);
      |          ^~~~~~~~~
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/decorator/rate_controller.cpp:95:52: error: ‘void BT::TreeNode::setStatus(BT::NodeStatus)’ is protected within this context
   95 |         child_node_->setStatus(BT::NodeStatus::IDLE);
      |                                                    ^
In file included from /opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/decorator_node.h:4,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/decorator/rate_controller.cpp:21:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/tree_node.h:166:10: note: declared protected here
  166 |     void setStatus(NodeStatus new_status);
      |          ^~~~~~~~~
make[2]: *** [CMakeFiles/nav2_rate_controller_bt_node.dir/build.make:63: CMakeFiles/nav2_rate_controller_bt_node.dir/plugins/decorator/rate_controller.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:193: CMakeFiles/nav2_rate_controller_bt_node.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/round_robin_node.cpp: In member function ‘virtual BT::NodeStatus nav2_behavior_tree::RoundRobinNode::tick()’:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/round_robin_node.cpp:49:23: error: ‘void BT::ControlNode::haltChildren(size_t)’ is deprecated: deprecated: please use explicitly haltChildren() or haltChild(i) [-Werror=deprecated-declarations]
   49 |         haltChildren(0);
      |                       ^
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/round_robin_node.cpp:17:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/control_node.h:49:10: note: declared here
   49 |     void haltChildren(size_t first);
      |          ^~~~~~~~~~~~
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/round_robin_node.cpp:53:23: error: ‘void BT::ControlNode::haltChildren(size_t)’ is deprecated: deprecated: please use explicitly haltChildren() or haltChild(i) [-Werror=deprecated-declarations]
   53 |         haltChildren(0);
      |                       ^
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/round_robin_node.cpp:17:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/control_node.h:49:10: note: declared here
   49 |     void haltChildren(size_t first);
      |          ^~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [CMakeFiles/nav2_round_robin_node_bt_node.dir/build.make:63: CMakeFiles/nav2_round_robin_node_bt_node.dir/plugins/control/round_robin_node.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:112: CMakeFiles/nav2_round_robin_node_bt_node.dir/all] Error 2
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/pipeline_sequence.cpp: In member function ‘virtual BT::NodeStatus nav2_behavior_tree::PipelineSequence::tick()’:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/pipeline_sequence.cpp:85:23: error: ‘void BT::ControlNode::haltChildren(size_t)’ is deprecated: deprecated: please use explicitly haltChildren() or haltChild(i) [-Werror=deprecated-declarations]
   85 |         haltChildren(0);
      |                       ^
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/pipeline_sequence.cpp:18:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/control_node.h:49:10: note: declared here
   49 |     void haltChildren(size_t first);
      |          ^~~~~~~~~~~~
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/pipeline_sequence.cpp:109:17: error: ‘void BT::ControlNode::haltChildren(size_t)’ is deprecated: deprecated: please use explicitly haltChildren() or haltChild(i) [-Werror=deprecated-declarations]
  109 |   haltChildren(0);
      |                 ^
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/pipeline_sequence.cpp:18:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/control_node.h:49:10: note: declared here
   49 |     void haltChildren(size_t first);
      |          ^~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [CMakeFiles/nav2_pipeline_sequence_bt_node.dir/build.make:63: CMakeFiles/nav2_pipeline_sequence_bt_node.dir/plugins/control/pipeline_sequence.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:517: CMakeFiles/nav2_pipeline_sequence_bt_node.dir/all] Error 2
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/recovery_node.cpp: In member function ‘virtual BT::NodeStatus nav2_behavior_tree::RecoveryNode::tick()’:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/recovery_node.cpp:91:31: error: ‘void BT::ControlNode::haltChildren(size_t)’ is deprecated: deprecated: please use explicitly haltChildren() or haltChild(i) [-Werror=deprecated-declarations]
   91 |                 haltChildren(0);
      |                               ^
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/recovery_node.cpp:19:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/control_node.h:49:10: note: declared here
   49 |     void haltChildren(size_t first);
      |          ^~~~~~~~~~~~
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/recovery_node.cpp:114:29: error: ‘void BT::ControlNode::haltChildren(size_t)’ is deprecated: deprecated: please use explicitly haltChildren() or haltChild(i) [-Werror=deprecated-declarations]
  114 |               haltChildren(1);
      |                             ^
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/control/recovery_node.cpp:19:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/control_node.h:49:10: note: declared here
   49 |     void haltChildren(size_t first);
      |          ^~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [CMakeFiles/nav2_recovery_node_bt_node.dir/build.make:63: CMakeFiles/nav2_recovery_node_bt_node.dir/plugins/control/recovery_node.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:166: CMakeFiles/nav2_recovery_node_bt_node.dir/all] Error 2
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/src/behavior_tree_engine.cpp:15:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/behavior_tree_engine.hpp: In member function ‘void nav2_behavior_tree::BehaviorTreeEngine::haltAllActions(BT::TreeNode*)’:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/behavior_tree_engine.hpp:52:46: error: ‘void BT::TreeNode::setStatus(BT::NodeStatus)’ is protected within this context
   52 |     root_node->setStatus(BT::NodeStatus::IDLE);
      |                                              ^
In file included from /opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/control_node.h:18,
                 from /opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/controls/parallel_node.h:18,
                 from /opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/behavior_tree.h:17,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/behavior_tree_engine.hpp:22,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/src/behavior_tree_engine.cpp:15:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/tree_node.h:166:10: note: declared protected here
  166 |     void setStatus(NodeStatus new_status);
      |          ^~~~~~~~~
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/src/behavior_tree_engine.cpp:15:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/behavior_tree_engine.hpp: In lambda function:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/behavior_tree_engine.hpp:58:47: error: ‘void BT::TreeNode::setStatus(BT::NodeStatus)’ is protected within this context
   58 |           node->setStatus(BT::NodeStatus::IDLE);
      |                                               ^
In file included from /opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/control_node.h:18,
                 from /opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/controls/parallel_node.h:18,
                 from /opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/behavior_tree.h:17,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/behavior_tree_engine.hpp:22,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/src/behavior_tree_engine.cpp:15:
/opt/underlay_ws/install/behaviortree_cpp_v3/include/behaviortree_cpp_v3/tree_node.h:166:10: note: declared protected here
  166 |     void setStatus(NodeStatus new_status);
      |          ^~~~~~~~~
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/src/behavior_tree_engine.cpp: In member function ‘nav2_behavior_tree::BtStatus nav2_behavior_tree::BehaviorTreeEngine::run(BT::Tree*, std::function<void()>, std::function<bool()>, std::chrono::milliseconds)’:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/src/behavior_tree_engine.cpp:50:13: error: ‘class BT::Tree’ has no member named ‘root_node’; did you mean ‘rootNode’?
   50 |       tree->root_node->halt();
      |             ^~~~~~~~~
      |             rootNode
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/src/behavior_tree_engine.cpp:54:20: error: ‘class BT::Tree’ has no member named ‘root_node’; did you mean ‘rootNode’?
   54 |     result = tree->root_node->executeTick();
      |                    ^~~~~~~~~
      |                    rootNode
make[2]: *** [CMakeFiles/nav2_behavior_tree.dir/build.make:63: CMakeFiles/nav2_behavior_tree.dir/src/behavior_tree_engine.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:301: CMakeFiles/nav2_behavior_tree.dir/all] Error 2
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/action/reinitialize_global_localization_service.cpp:22:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/bt_service_node.hpp: In member function ‘virtual BT::NodeStatus nav2_behavior_tree::BtServiceNode<ServiceT>::check_future(std::shared_future<typename ServiceT::Response::SharedPtr>)’:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/bt_service_node.hpp:101:40: error: ‘using FutureReturnCode = enum class rclcpp::FutureReturnCode’ is deprecated: use rclcpp::FutureReturnCode instead [-Werror=deprecated-declarations]
  101 |     rclcpp::executor::FutureReturnCode rc;
      |                                        ^~
In file included from /opt/ros/foxy/include/rclcpp/executor.hpp:34,
                 from /opt/ros/foxy/include/rclcpp/executors/multi_threaded_executor.hpp:25,
                 from /opt/ros/foxy/include/rclcpp/executors.hpp:21,
                 from /opt/ros/foxy/include/rclcpp/rclcpp.hpp:145,
                 from /opt/overlay_ws/install/nav2_util/include/nav2_util/node_utils.hpp:19,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/bt_service_node.hpp:22,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/action/reinitialize_global_localization_service.cpp:22:
/opt/ros/foxy/include/rclcpp/future_return_code.hpp:48:7: note: declared here
   48 | using FutureReturnCode [[deprecated("use rclcpp::FutureReturnCode instead")]] = FutureReturnCode;
      |       ^~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [CMakeFiles/nav2_reinitialize_global_localization_service_bt_node.dir/build.make:63: CMakeFiles/nav2_reinitialize_global_localization_service_bt_node.dir/plugins/action/reinitialize_global_localization_service.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:220: CMakeFiles/nav2_reinitialize_global_localization_service_bt_node.dir/all] Error 2
In file included from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/action/clear_costmap_service.cpp:22:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/bt_service_node.hpp: In member function ‘virtual BT::NodeStatus nav2_behavior_tree::BtServiceNode<ServiceT>::check_future(std::shared_future<typename ServiceT::Response::SharedPtr>)’:
/opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/bt_service_node.hpp:101:40: error: ‘using FutureReturnCode = enum class rclcpp::FutureReturnCode’ is deprecated: use rclcpp::FutureReturnCode instead [-Werror=deprecated-declarations]
  101 |     rclcpp::executor::FutureReturnCode rc;
      |                                        ^~
In file included from /opt/ros/foxy/include/rclcpp/executor.hpp:34,
                 from /opt/ros/foxy/include/rclcpp/executors/multi_threaded_executor.hpp:25,
                 from /opt/ros/foxy/include/rclcpp/executors.hpp:21,
                 from /opt/ros/foxy/include/rclcpp/rclcpp.hpp:145,
                 from /opt/overlay_ws/install/nav2_util/include/nav2_util/node_utils.hpp:19,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/include/nav2_behavior_tree/bt_service_node.hpp:22,
                 from /opt/overlay_ws/src/navigation2/nav2_behavior_tree/plugins/action/clear_costmap_service.cpp:22:
/opt/ros/foxy/include/rclcpp/future_return_code.hpp:48:7: note: declared here
   48 | using FutureReturnCode [[deprecated("use rclcpp::FutureReturnCode instead")]] = FutureReturnCode;
      |       ^~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [CMakeFiles/nav2_clear_costmap_service_bt_node.dir/build.make:63: CMakeFiles/nav2_clear_costmap_service_bt_node.dir/plugins/action/clear_costmap_service.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:598: CMakeFiles/nav2_clear_costmap_service_bt_node.dir/all] Error 2
make: *** [Makefile:141: all] Error 2
---
Failed   <<< nav2_behavior_tree	[ Exited with code 2 ]
Aborted  <<< nav2_rviz_plugins
Aborted  <<< nav2_map_server
Aborted  <<< nav2_amcl

Summary: 10 packages finished [1min 52s]
  1 package failed: nav2_behavior_tree
  3 packages aborted: nav2_amcl nav2_map_server nav2_rviz_plugins
  2 packages had stderr output: nav2_behavior_tree nav2_rviz_plugins
  15 packages not processed
The command '/bin/sh -c . $UNDERLAY_WS/install/setup.sh &&     colcon build       --symlink-install       --mixin $OVERLAY_MIXINS     || touch build_failed &&     if [ -f build_failed ] && [ -n "$FAIL_ON_BUILD_FAILURE" ]; then       exit 1;     fi' returned a non-zero code: 1

Related: BehaviorTree/BehaviorTree.CPP#106

[SteveMacenski]

@facontidavide any thoughts on how to remove these setStatus calls that you made protected in 3.3.0 (https://github.com/BehaviorTree/BehaviorTree.CPP/releases)? I think that really messes up some of our custom control flow nodes and haltAllAction logic with the recursive visitor.

[facontidavide]

First: your decorator does NOT need to set the status of the children.

https://github.com/BehaviorTree/BehaviorTree.CPP/blob/0f51631dadd7186a415f6c86f01066fadbabdb7c/src/decorator_node.cpp#L69-L78

Just remove child_node_->setStatus(BT::NodeStatus::IDLE)

Second: haltChildren(0) is a very bad API that makes me ashamed as a developer. Bad, bad Davide.

Use instead: void ControlNode::haltChildren();

Third: ...and this is apparent in the new tutorials.

result = tree->root_node->executeTick();
becomes
result = tree->tickRoot();

You can (should) also remove the line
root_node->setStatus(BT::NodeStatus::IDLE);
because it is done under the hood.

Finally:

tree->root_node->halt()
becomes
tree->rootNode()->halt()

[facontidavide]

@SteveMacenski : I think that really messes up some of our custom control flow nodes and haltAllAction logic with the recursive visitor.

I do not believe you will notice any difference in the control flow. If you do, please let me know.

[SteveMacenski]

Awesome. Its become clear to me that someone really needs to take a really long, deep look at our use of BT.CPP, this isn't the first time our implementation is abusing things or making things redundant. I'm dissatisfied to say that I don't know this section of the code particularly well. A refactor / redesign may be in order to clean house of these problems and try to simplify.

[deepanshubansal01]

I am working on this behavior tree task.

[SteveMacenski]

@deepanshubansal01 I had to do this to unblock CI, apologies

[deepanshubansal01]

@SteveMacenski No problem