BehaviorTree/BehaviorTree.ROS2

Why doesn't TreeExecutionServer directly expose rclcpp::Node::SharedPtr?

Closed this issue · 4 comments

When I implemented the override of void registerNodesIntoFactory(BT::BehaviorTreeFactory& factory), I wrote it like this:

BT::RosNodeParams params;
params.nh = std::dynamic_pointer_cast<rclcpp::Node>(nodeBaseInterface());

This caused the following exceptions during tree construction:

[ERROR] [1715013001.175801122] [bt_action_server]: Behavior Tree exception:bad_weak_ptr
[ERROR] [1715013002.693655469] [bt_action_server]: Behavior Tree exception:The ROS node went out of scope. RosNodeParams doesn't take the ownership of the node.

Instead, when I changed the API to directly assign rclcpp::Node::SharedPtr to nh, it worked perfectly.
Why doesn't TreeExecutionServer directly expose rclcpp::Node::SharedPtr?
If there are no issues, I would like to add an API to directly exposerclcpp::Node::SharedPtr.

To get access to to the node handle passed into your behavior via
BT::RosNodeParams& params
you need to convert the weak pointer by asking for a lock to it
auto node = params.nh.lock();

I'd like to provide additional information regarding the error message.

The bad_weak_ptr occurred when constructing a tree composed solely of a derived class of RosTopicPubNode.
Indeed, in RosTopicPubNode, it appears that weak_ptr is directly assigned to shared_ptr.
(I've checked other programs, and it seems that only RosTopicPubNode is using shared_ptr instead of weak_ptr.)

protected:
std::shared_ptr<rclcpp::Node> node_;

template <class T>
inline RosTopicPubNode<T>::RosTopicPubNode(const std::string& instance_name,
const NodeConfig& conf,
const RosNodeParams& params)
: BT::ConditionNode(instance_name, conf), node_(params.nh)

The error message:

Behavior Tree exception: The ROS node went out of scope. RosNodeParams doesn't take the ownership of the node.

was encountered when constructing a tree containing many nodes, so it may take some time to identify which node is causing the issue. It's possible that the implementation of my class, which directly inherits from the behaviortree.cpp class, might be flawed.

I requested to expose rclcpp::Node::SharedPtr, but it might also be acceptable to use std::weak_ptr<rclcpp::Node>. (The following code worked correctly when written as follows.)

std::weak_ptr<rclcpp::Node> TreeExecutionServer::node()
{
  return std::dynamic_pointer_cast<rclcpp::Node>(p_->node);
}
void registerNodesIntoFactory(BT::BehaviorTreeFactory& factory) {
  // Register Nodes
  BT::RosNodeParams params;
  params.nh = node();

I agree. I will expose that, much easier.