Support lifecycle node

[Benjamin-Tan]

To close #108,

Had a try with the templating approach similar to #167, but that did not go well without some major refactoring. The current implementation overloads the constructor with LifecycleNode instead, with minimal change to the design/architecture.

Others:

• Change raw pointers to shared pointers.
• Add lifecycle test

[Benjamin-Tan]

I have changed it to the template approach as per your suggestion.

[Benjamin-Tan]

I tried to compile this with clang and I get the following errors:

/usr/bin/ld: libimage_transport.so: undefined reference to `image_transport::CameraSubscriber<rclcpp::Node>::CameraSubscriber(std::__1::shared_ptr<rclcpp::Node>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::function<void (std::__1::shared_ptr<sensor_msgs::msg::Image_<std::__1::allocator<void> > const> const&, std::__1::shared_ptr<sensor_msgs::msg::CameraInfo_<std::__1::allocator<void> > const> const&)> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, rmw_qos_profile_s)'
/usr/bin/ld: libimage_transport.so: undefined reference to `image_transport::Subscriber<rclcpp_lifecycle::LifecycleNode>::Subscriber(std::__1::shared_ptr<rclcpp_lifecycle::LifecycleNode>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::function<void (std::__1::shared_ptr<sensor_msgs::msg::Image_<std::__1::allocator<void> > const> const&)> const&, std::__1::shared_ptr<pluginlib::ClassLoader<image_transport::SubscriberPlugin<rclcpp_lifecycle::LifecycleNode> > >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, rmw_qos_profile_s, rclcpp::SubscriptionOptionsWithAllocator<std::__1::allocator<void> >)'
/usr/bin/ld: libimage_transport.so: undefined reference to `image_transport::CameraSubscriber<rclcpp_lifecycle::LifecycleNode>::CameraSubscriber(std::__1::shared_ptr<rclcpp_lifecycle::LifecycleNode>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::function<void (std::__1::shared_ptr<sensor_msgs::msg::Image_<std::__1::allocator<void> > const> const&, std::__1::shared_ptr<sensor_msgs::msg::CameraInfo_<std::__1::allocator<void> > const> const&)> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, rmw_qos_profile_s)'
/usr/bin/ld: libimage_transport.so: undefined reference to `image_transport::Subscriber<rclcpp::Node>::Subscriber(std::__1::shared_ptr<rclcpp::Node>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::function<void (std::__1::shared_ptr<sensor_msgs::msg::Image_<std::__1::allocator<void> > const> const&)> const&, std::__1::shared_ptr<pluginlib::ClassLoader<image_transport::SubscriberPlugin<rclcpp::Node> > >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, rmw_qos_profile_s, rclcpp::SubscriptionOptionsWithAllocator<std::__1::allocator<void> >)'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
gmake[2]: *** [CMakeFiles/list_transports.dir/build.make:202: list_transports] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:197: CMakeFiles/list_transports.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
--- stderr: image_transport                              
/usr/bin/ld: libimage_transport.so: undefined reference to `image_transport::CameraSubscriber<rclcpp::Node>::CameraSubscriber(std::__1::shared_ptr<rclcpp::Node>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::function<void (std::__1::shared_ptr<sensor_msgs::msg::Image_<std::__1::allocator<void> > const> const&, std::__1::shared_ptr<sensor_msgs::msg::CameraInfo_<std::__1::allocator<void> > const> const&)> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, rmw_qos_profile_s)'
/usr/bin/ld: libimage_transport.so: undefined reference to `image_transport::Subscriber<rclcpp_lifecycle::LifecycleNode>::Subscriber(std::__1::shared_ptr<rclcpp_lifecycle::LifecycleNode>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::function<void (std::__1::shared_ptr<sensor_msgs::msg::Image_<std::__1::allocator<void> > const> const&)> const&, std::__1::shared_ptr<pluginlib::ClassLoader<image_transport::SubscriberPlugin<rclcpp_lifecycle::LifecycleNode> > >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, rmw_qos_profile_s, rclcpp::SubscriptionOptionsWithAllocator<std::__1::allocator<void> >)'
/usr/bin/ld: libimage_transport.so: undefined reference to `image_transport::CameraSubscriber<rclcpp_lifecycle::LifecycleNode>::CameraSubscriber(std::__1::shared_ptr<rclcpp_lifecycle::LifecycleNode>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::function<void (std::__1::shared_ptr<sensor_msgs::msg::Image_<std::__1::allocator<void> > const> const&, std::__1::shared_ptr<sensor_msgs::msg::CameraInfo_<std::__1::allocator<void> > const> const&)> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, rmw_qos_profile_s)'
/usr/bin/ld: libimage_transport.so: undefined reference to `image_transport::Subscriber<rclcpp::Node>::Subscriber(std::__1::shared_ptr<rclcpp::Node>, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::function<void (std::__1::shared_ptr<sensor_msgs::msg::Image_<std::__1::allocator<void> > const> const&)> const&, std::__1::shared_ptr<pluginlib::ClassLoader<image_transport::SubscriberPlugin<rclcpp::Node> > >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, rmw_qos_profile_s, rclcpp::SubscriptionOptionsWithAllocator<std::__1::allocator<void> >)'

Do you mind to take a look ? you just need to add this --mixin clang-libcxx to your colcon command

Another question: These changes require also to modify the image_transport_plugins?

Had the same issue when recompiling with clang, I have fix those and pushed it.

On your question about the need to modify image_transport_plugins, with the templating approach, it seems to be needed to pass the NodeType, unless you have any alternative to this approach. I tried not to change it but could not find a way around it.

[Benjamin-Tan]

To be backported, is the following acceptable?

By using the templating approach for the rest, except for the base class for publisher and subscriber plugin.hpp? I think this would keep the API the same, but go against your initial concern of having advertiseImpl(Node*,...), advertiseImpl(LifecycleNode*,...)

[authaldo]

Thanks for the implementation effort, finally a solution to the rather old issue #108 is in sight!

As a side note regarding the refactoring mentioned for a continuation of #167: I attempted to follow this route (based on #258) a while ago and switched to a implementation built around rclcpp::node_interfaces::NodeInterfaces in my forks of image_common and image_common_plugins. While this provides (at least in my subjective view) a cleaner implementation by using the interface classes for what they have been designed for, it has of course the drawback of breaking with the current API (at the interface between image_transport and the corresponding plugins).

Switching to the NodeInterfaces made it also necessary to adapt the message filters code, leading to the following (still open) PR. This PR was also the main reason for losing the interest in pushing the other two forks further.

[ahcorde]

This PR is quite big, @mikeferguson do you mind to take a look ?

[Benjamin-Tan]

there are some conflicts, by the way I'm not able to compile the plugins

Yeah I think after this PR has been reviewed, I would open a corresponding PR on the image_transport_plugins as well to update the implementation.