Fix publisher advertiseImpl() and subscribeImpl() for compressed_image_transport, compressed_depth_transport and theora_image_transport

[ivanpauno]

Required after ros-perception/image_common#249 and ros-perception/image_common#252.

None of the parameters of this plugins were being declared because of this error.

[ivanpauno]

cc @jacobperron

[jacobperron]

LGTM, thanks 🙇

[jacobperron]

CI is complaining about some of the override keywords you added. Can you double check if those are necessary? Maybe there's a difference between Humble and Rolling,

[ijnek]

Duplicate of #105 (Draft), but happy to close the other if this gets merged first.
These changes would comply with new changes in publisher_plugin and subscriber_plugin from ros-perception/image_common#249 too 👍

[ivanpauno]

These changes would comply with new changes in publisher_plugin and subscriber_plugin from ros-perception/image_common#249 too +1

Yes, this needs a new release of image_common.

Actually, that PR is the one that broke this, right?
I'm not sure if it's a good idea the deprecation warning added there, as it's breaking downstream behavior completely.

[ivanpauno]

CI is complaining about some of the override keywords you added. Can you double check if those are necessary? Maybe there's a difference between Humble and Rolling,

This doesn't apply to humble, it's needed after ros-perception/image_common#249.

[ijnek]

Actually, that PR is the one that broke this, right?

That's the PR that would break this once the changes are released, yes. (Hence #105 still being a draft). Just to confirm, were you building both image_common and image_transport_plugins from source?

The original problem that ros-perception/image_common#249 solved was the error message that showed up at run time:

[ERROR] [1659558188.442971154] [rqt_gui_cpp_node_1318445]: SubscriberPlugin::subscribeImpl with five arguments has not been overridden

showed up when you called advertiseImpl without options, so there was a PR that hacked around this (ros-perception/image_common#243). In ros-perception/image_common#243 (comment), I suggested deprecating image transports that don't handle options correctly.

[ivanpauno]

That's the PR that would break this once the changes are released, yes. (Hence #105 still being a draft). Just to confirm, were you building both image_common and image_transport_plugins from source?

Yes, I'm building both from source.

[ivanpauno]

@jacobperron can you create a new branch for humble so this can be merged in the ros2 branch?
Thanks!

[jacobperron]

humble branch created. Related rosdistro PR: ros/rosdistro#34104

[eigendude]

I also needed this change to compile compressed depth image transport:

diff --git a/compressed_depth_image_transport/include/compressed_depth_image_transport/compressed_depth_publisher.h b/compressed_depth_image_transport/include/compressed_depth_image_transport/compressed_depth_publisher.h
index d67af74..b3feecc 100644
--- a/compressed_depth_image_transport/include/compressed_depth_image_transport/compressed_depth_publisher.h
+++ b/compressed_depth_image_transport/include/compressed_depth_image_transport/compressed_depth_publisher.h
@@ -53,7 +53,8 @@ protected:
   virtual void advertiseImpl(
           rclcpp::Node * node,
           const std::string &base_topic,
-          rmw_qos_profile_t custom_qos) override final;
+          rmw_qos_profile_t custom_qos,
+          rclcpp::PublisherOptions options) override final;
 
   virtual void publish(const sensor_msgs::msg::Image& message,
                        const PublishFn& publish_fn) const override final;
diff --git a/compressed_depth_image_transport/src/compressed_depth_publisher.cpp b/compressed_depth_image_transport/src/compressed_depth_publisher.cpp
index aca3741..4eb4678 100644
--- a/compressed_depth_image_transport/src/compressed_depth_publisher.cpp
+++ b/compressed_depth_image_transport/src/compressed_depth_publisher.cpp
@@ -60,10 +60,11 @@ namespace compressed_depth_image_transport
 void CompressedDepthPublisher::advertiseImpl(
   rclcpp::Node * node,
   const std::string& base_topic,
-  rmw_qos_profile_t custom_qos)
+  rmw_qos_profile_t custom_qos,
+  rclcpp::PublisherOptions options)
 {
   typedef image_transport::SimplePublisherPlugin<sensor_msgs::msg::CompressedImage> Base;
-  Base::advertiseImpl(node, base_topic, custom_qos);
+  Base::advertiseImpl(node, base_topic, custom_qos, options);
 
   node->get_parameter_or<int>("png_level", config_.png_level, kDefaultPngLevel);

[ivanpauno]

Thanks @eigendude, I didn't check compressed_depth_image_transport.
I will update the PR with that fixed.

[ivanpauno]

I needed some extra fixes, but this is working fine locally for me.
We need a new rolling release of image_common to get CI passing.

[clalancette]

@ros-pull-request-builder retest this please

[mhubii]

still receiving this error

[ERROR] [1719225304.277762595] [image_republisher]: SubscriberPlugin::subscribeImpl with five arguments has not been overridden

for compressed depth plugin on Ubuntu 22.04 ROS 2 Humble