New behavior tree #91
Conversation
| @@ -18,19 +18,26 @@ find_package(nav2_tasks REQUIRED) | |||
|
|
|||
| include_directories( | |||
| include | |||
| /usr/local/include/BTpp | |||
There was a problem hiding this comment.
Can we use find _library(BTpp) instead of a hard-coded path?
| ) | ||
|
|
||
| target_link_libraries(${executable_name} | ||
| /usr/local/BTpp/lib/libBTppLib.a |
There was a problem hiding this comment.
Same here, we shouldn't hard-code the path to libBTppLib
| { | ||
| public: | ||
| BtActionNode( | ||
| rclcpp::Node::SharedPtr node, const std::string & actionName, |
There was a problem hiding this comment.
Please separate constructor params onto different lines for easy readability
| @@ -18,19 +18,26 @@ find_package(nav2_planning_msgs REQUIRED) | |||
|
|
|||
| include_directories( | |||
| include | |||
| /usr/local/include/BTpp | |||
There was a problem hiding this comment.
Same comment as other CMakeLists files, need to use CMake find_library for the BTpp library
There was a problem hiding this comment.
@mjeronimo Do you have a sequence diagram or something like that to show how the cancel mechanism works?
| @@ -0,0 +1,113 @@ | |||
| // Copyright (c) 2018 Intel Corporation | |||
There was a problem hiding this comment.
All these BT specific files shouldn't go in the nav2_tasks folder should they? I thought the tasks were supposed to be independent of the implementation of the execution engine, whether it be BT, SMACH or whatever.
| @@ -35,7 +35,7 @@ template<class CommandMsg, class ResultMsg> | |||
| class TaskClient | |||
| { | |||
| public: | |||
| explicit TaskClient(rclcpp::Node * node) | |||
| explicit TaskClient(rclcpp::Node::SharedPtr node) | |||
There was a problem hiding this comment.
I've been using shared_ptr & in cases like these. Without the reference, we create a temporary shared_ptr for the parameter, then another one for the node_. Then we have to destroy the temporary. By making it a &, the temporary shared_ptr goes away.
It might be possible to make it a const &, ideally, but I'm not sure if you can assign a const shared_ptr to a non-const.
|
Since this is all new code, we should probably include some unit tests at least. |
…starting the parallel nodes
|
@mjeronimo - I believe this is still WIP, do you mind closing and re-opening when you're ready for review, that way we'll know when to take another look at it. |
|
Sure. |
Background info for this change
The Navigation 2 software stack can be driven by a Mission Executor module that receives a description of the desired behavior tree as an XML document via its input command. The XML is contained in a MissionPlan message as a string. The Mission Executor receives the plan and builds the tree according to this XML description and then dynamically executes the resulting tree. Currently, the nav2 software has one Task that it can perform, NavigateToPose. In the future, other tasks can be implemented and included in the mission plan.
NavigateToPose is implemented by two alternative modules, the SimpleNavigator and the BtNavigator. These implement the NavigateToPose task by invoking sub-tasks, ComputePathToPose and FollowPath. Each of these are themselves separate TaskServers (what will be ROS Actions), so the Navigators have corresponding TaskClients to communicate with the task servers.
The SimpleNavigator has a hand-coded execute method where it invokes first the ComputePathToPose and then FollowPose. In contrast, the BtNavigator itself uses a behavior tree to implement similar functionality. However, the behavior tree is more easily extended and can do things, such as run the ComputePathToPose (the global planner) in parallel with the FollowPose (the local planner). That is, the global planner could be run at a desired frequency, sending the updates paths to the local planner. Also, using a behavior tree, there could be recovery actions upon failure. To implement the behavior tree functionality, we're using a new version of the behavior tree library that was previously using in ROS1's ROS-Behavior-Tree. There are several useful additions, including XML parsing & automatic tree building and blackboards.
Description of the change
For now, this commit gets basic behavior trees in place for the MissionExecutor and the BtNavigator. These will soon be extended.
Fixes #51. This was an issue about not referring to the library directly in CMakeLists.txt, but using find_package instead.
Other issues, that will be addressed as we develop the mission plan capability, including #58, #41, #43, and #42.