Correct way in ROS 2 to handle pub/sub from a plugin
Incorrectly originally posted on ROS discourse. Copying conversation here:
If, for example, you have a plugin that is used within a rclcpp::Node but needs to create it’s own publishers or subscribers (or timers, or clients, etc.) what is the correct way for said plugin to do that. It seems simple enough to just pass a reference or pointer to the node (*this or this) to the plugin so that it can just call create_subscriber or whatever else it needs to
But… objects like ImageTransport require a rclcpp::Node::SharedPtr and have a rather convoluted internal process for creating subscriptions, etc.
So two questions:
Is there any particular reason why you would only want to pass a Node::SharedPtr? Classes like AsyncParameterClient have a constructor version that takes just a Node*.
Is there a reason you would not want to just call methods like create_subscription from plugins, but use the Node interface pointers instead?
We’ve found by experience in Nav2 that passing a WeakPtr is the way to go. You can lock them to get a SharedPtr out but doesn’t maintain a reference count on destruction that can cause a chicken and egg problem if you’re not careful (or expect users to create their own plugins that might not be aware of some of the subtleties).
Our interface definitions have plenty of examples of that navigation2/nav2_core/include/nav2_core at main · ros-planning/navigation2
I guess in my current situation I was not concerned with the order destruction (plugins destructors would all get called before destructing the Node). But I can see using a WeakPtr might be a good standard.
My main concern with using a shared_ptr (or weak_ptr) is the creation of the original shared_ptr from within the object it’s managing. If we know that the object was created via a shared_ptr we can use std::enable_shared_from_this::shared_from_this to get a shared_ptr without worrying about a double-free fault, but that does not seem safe to assume, especially since calling shared_from_this on something that is not managed by a shared_ptr is undefined behavior in C++14. But spin et. al. only operate on Node::SharedPtr so maybe only creating nodes with shared_ptrs is ROS 2 standard, but that isn’t clear to me.
FWIW in the case where we are using ImageTransport we are just creating a shared_ptr to this and specifying an alternative, do-nothing destructor since we know that if the ImageTransport object not be alive if the holding Node object is not.
