Meta PR: try inheritance instead of callbacks #6
Conversation
| @@ -41,9 +41,13 @@ ActionServerBT::ActionServerBT(const rclcpp::NodeOptions& options, const UserCal | |||
|
|
|||
| // register the users Plugins and BehaviorTree.xml files into the factory | |||
| register_behavior_trees(params_, factory_, node_); | |||
| registerNodesIntoFactory(factory_); | |||
|
|
|||
| // No one "own" this blackboard | |||
There was a problem hiding this comment.
we want this to persist, by default
There was a problem hiding this comment.
How would a user go about clearing the global blackboard? Seems like some runs they could want values to persist but others should start clean. Should we consider adding an option to the Action goal?
There was a problem hiding this comment.
I think this also leads into the feature of the user waning to clear and ImportBlackboardFromJSON. Another nice addition to the Action goal would be the ability to specify a path to a JSON file that should be loaded before running the tree.
There was a problem hiding this comment.
you can still access it and clean it in onTreeCreated.
I would use that callback to do all this initialization and let the user take care of it, instead of tryingto figure out all the potential things they may want to do
There was a problem hiding this comment.
So I was thinking about that as an option too but it could lead to a few situations. Let's say they offer a service to set the blackboard. What would stop them from mutating the BB while the tree is running? Does BT.CPP have guards to protect from reading and writing BB variables at the same time?
There was a problem hiding this comment.
I really like how clean this ended up, great work @facontidavide!
| @@ -41,9 +41,13 @@ ActionServerBT::ActionServerBT(const rclcpp::NodeOptions& options, const UserCal | |||
|
|
|||
| // register the users Plugins and BehaviorTree.xml files into the factory | |||
| register_behavior_trees(params_, factory_, node_); | |||
| registerNodesIntoFactory(factory_); | |||
|
|
|||
| // No one "own" this blackboard | |||
There was a problem hiding this comment.
How would a user go about clearing the global blackboard? Seems like some runs they could want values to persist but others should start clean. Should we consider adding an option to the Action goal?
I just removed the method Brute force cleanup static auto clean_bb = BT::Blackboard::create();
clean_bb->cloneInto(*global_blackboard); |
| RCLCPP_ERROR(kLogger, action_result->error_message.c_str()); | ||
| tree.haltTree(); | ||
| goal_handle->canceled(action_result); | ||
| abort_action(status, "Action Server canceling and halting Behavior Tree"); |
There was a problem hiding this comment.
Technically this should return goal_handle->canceled(). If we move this statement out of the lambda we can support both canceled and aborted.
What about this?