-
Notifications
You must be signed in to change notification settings - Fork 522
Description
Feature request
Feature description
This is an idea to simplify code that calls APIs using node interfaces. It proposes adding template classes NodeHandle<> and NodeHandle<>::SharedPtr who are given placeholder types that indicate the needed node interfaces.
When node interfaces are just needed for the lifetime of the call
create_service() needs a NodeBaseInterface and NodeServicesInterface. It doesn't need shared pointers to these interfaces; it just needs them for the duration of the call.
Before:
create_service(
std::shared_ptr<node_interfaces::NodeBaseInterface> node_base,
std::shared_ptr<node_interfaces::NodeServicesInterface> node_services,
// ...
// User calls with
auto service = create_service(
my_node_class->get_node_base_interface(),
my_node_class->get_node_services_interface(),
// ...after
create_service(
rclcpp::NodeHandle<Base, Services> node_handle,
// ...
// User calls with
auto service = create_service(
my_node_class,
// ...When node interfaces are needed long term
tf2_ros::TransformListener needs the node base interface for a long time, so it holds a shared pointer.
It currently does some logic in the header file and some in the cpp file, but the intent of this proposal is to enable moving all of it to the cpp file.
Before
TransformListener(
tf2::BufferCore & buffer,
NodeT && node,
// ...
// in header creates subscription and calls initThread(node->get_node_base_interface()); to store shared pointer to nodeAfter
TransformListener(
tf2::BufferCore & buffer,
rclcpp::NodeHandle<Base, Subscription>::SharedPtr node_handle,
// ...
); // Just declaration in headerImplementation considerations
Placeholder types should be created for each node interface. The purpose of these is to make the type of a NodeHandle more readable. In my opinion rclcpp::NodeHandle<Base> is easier to read than rclcpp::NodeHandleBase<node_interfaces::NodeBaseInterface>.
The types passed to NodeHandle<> would indicate the interfaces needed by a method. rclcpp::NodeHandle<Base, Topics> would replace get_node_base_interface and get_node_topics_interface(). Using similar template magic, it would get the base and topics interface off of whatever it's given at compile time. Code would access the interfaces with node_handle.base() or node_handle.topics() and those would return raw pointers to the interfaces.
rclcpp::NodeHandle<Base, Topics>::SharedPtr is a little more interesting because get_node_*_interface() don't address this use case, and I've seen it a couple times while porting packages. Some API's need shared pointers to the node interfaces so they can keep them alive for a while. NodeHandle<>::SharedPtr should be a shared pointer to an object that holds shared pointers to the interfaces. Interfaces would be accessed via node_handle->base() and node_handle->topics(), but the return type would be a shared pointer instead of a raw pointer. Since the return type is different, this is probably a different class internally (NodeHandle<>::SharedPtr = std::shared_ptr<NodeHandleCopyable<>>).