Support lifecycle node - NodeInterfaces by ahcorde · Pull Request #352 · ros-perception/image_common

ahcorde · 2025-04-08T19:15:28Z

I created a new PR using NodeInterfaces based on #351 and #304. I can add you as coauthor if you send me your emails

As you have already worked on this I will appreciate if you can review it.

The only test that it's failing it's 12 - image_transport-qos_override_lifecycle (Failed)

The node is not able to override the qos from parameters

12: /home/ahcorde/ros2_rolling/src/ros-perception/image_common/image_transport/test/test_qos_override_lifecycle.cpp:181: Failure
12: Expected equality of these values:
12:   endpoint_info_vec[0].qos_profile().reliability()
12:     Which is: 4-byte object <01-00 00-00>
12:   rclcpp::ReliabilityPolicy::BestEffort
12:     Which is: 4-byte object <02-00 00-00>

Related PRs

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde · 2025-04-08T19:16:47Z

@mikeferguson If you have some time I also appreciate your review.

I don't know if we should deprecate the old API, what do you think?

authaldo

Thanks for tackling this! Regarding the offer to add me as a co-author: Thanks, I would appreciate this. You can simply use the e-mail address I used for signing-off the other commits.

As a summary of thoughts:

I've only chosen pass-by-value for the NodeInterfaces as a simple start, did you think about whether passing it as a pointer would be more suitable?
Since image_transport_plugins seem to depend only on the SimpleXXXPlugin classes which offer a default implementation for the new NodeInterface methods, the changes should compile with the rolling version of image_transport_plugins (pretty nice!). Maybe one could use this pattern also for deprecating the node pointer? I think in the end it would be nice to use only the NodeInterfaces, although this might require a deprecation cycle

image_transport/CMakeLists.txt

authaldo · 2025-04-09T06:49:14Z

image_transport/src/camera_subscriber.cpp

    std::bind(&Impl::checkImagesSynchronized, impl_.get()));
 }

+CameraSubscriber::CameraSubscriber(


That seems like a lot of code duplication. Maybe we can avoid this by using the fact that the node pointer should always be usable for obtaining the corresponding NodeInterfaces struct? In my view the main difference between the old and the new constructors is only whether the node pointer or the node interfaces are passed to the Impl-constructor.

...and ideally we'd remove the Node pointer variant in the next distro.

Benjamin-Tan · 2025-04-09T11:56:23Z

@ahcorde thanks for pushing this forward. My email can be found from my profile. I will have a look when I got back.

Benjamin-Tan · 2025-04-10T16:50:08Z

image_transport/src/camera_publisher.cpp

 struct CameraPublisher::Impl
 {
+  using NodeLoggingInterface = rclcpp::node_interfaces::NodeLoggingInterface;
+  using RequiredInterfaces = rclcpp::node_interfaces::NodeInterfaces<NodeLoggingInterface>;


will this RequiredInterfaces introduce confusion over the one defined in image_transport namespace?

Benjamin-Tan · 2025-04-10T17:10:39Z

image_transport/include/image_transport/camera_subscriber.hpp

Has compilation error:

/ros2_ws/src/image_common/image_transport/src/camera_subscriber.cpp: In constructor ‘image_transport::CameraSubscriber::CameraSubscriber(image_transport::RequiredInterfaces, const std::string&, const Callback&, const std::string&, rmw_qos_profile_t)’: /ros2_ws/src/image_common/image_transport/src/camera_subscriber.cpp:162:29: error: no matching function for call to ‘message_filters::Subscriber<sensor_msgs::msg::CameraInfo_<std::allocator<void> > >::subscribe(image_transport::RequiredInterfaces&, std::string&, rmw_qos_profile_t&)’ 162 | impl_->info_sub_.subscribe(required_test_interfaces, info_topic, custom_qos); | ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /ros2_ws/src/image_common/image_transport/src/camera_subscriber.cpp:34: /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:365:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe(NodePtr, const std::string&, const rclcpp::QoS&) [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node; NodePtr = std::shared_ptr<rclcpp::Node>; std::string = std::__cxx11::basic_string<char>]’ 365 | void subscribe( | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:366:13: note: no known conversion for argument 1 from ‘image_transport::RequiredInterfaces’ {aka ‘rclcpp::node_interfaces::NodeInterfaces<rclcpp::node_interfaces::NodeBaseInterface, rclcpp::node_interfaces::NodeParametersInterface, rclcpp::node_interfaces::NodeLoggingInterface, rclcpp::node_interfaces::NodeTimersInterface, rclcpp::node_interfaces::NodeTopicsInterface>’} to ‘message_filters::Subscriber<sensor_msgs::msg::CameraInfo_<std::allocator<void> > >::NodePtr’ {aka ‘std::shared_ptr<rclcpp::Node>’} 366 | NodePtr node, const std::string & topic, | ~~~~~~~~^~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:381:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe(NodeType*, const std::string&, const rclcpp::QoS&) [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node; std::string = std::__cxx11::basic_string<char>]’ 381 | void subscribe( | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:382:16: note: no known conversion for argument 1 from ‘image_transport::RequiredInterfaces’ {aka ‘rclcpp::node_interfaces::NodeInterfaces<rclcpp::node_interfaces::NodeBaseInterface, rclcpp::node_interfaces::NodeParametersInterface, rclcpp::node_interfaces::NodeLoggingInterface, rclcpp::node_interfaces::NodeTimersInterface, rclcpp::node_interfaces::NodeTopicsInterface>’} to ‘rclcpp::Node*’ 382 | NodeType * node, const std::string & topic, | ~~~~~~~~~~~^~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:398:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe(NodePtr, const std::string&, const rclcpp::QoS&, rclcpp::SubscriptionOptions) [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node; NodePtr = std::shared_ptr<rclcpp::Node>; std::string = std::__cxx11::basic_string<char>; rclcpp::SubscriptionOptions = rclcpp::SubscriptionOptionsWithAllocator<std::allocator<void> >]’ 398 | void subscribe( | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:398:8: note: candidate expects 4 arguments, 3 provided /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:419:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe(NodeType*, const std::string&, const rclcpp::QoS&, rclcpp::SubscriptionOptions) [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node; std::string = std::__cxx11::basic_string<char>; rclcpp::SubscriptionOptions = rclcpp::SubscriptionOptionsWithAllocator<std::allocator<void> >]’ 419 | void subscribe( | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:419:8: note: candidate expects 4 arguments, 3 provided /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:451:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe(NodePtr, const std::string&, rmw_qos_profile_t) [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node; NodePtr = std::shared_ptr<rclcpp::Node>; std::string = std::__cxx11::basic_string<char>; rmw_qos_profile_t = rmw_qos_profile_s]’ 451 | void subscribe( | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:452:13: note: no known conversion for argument 1 from ‘image_transport::RequiredInterfaces’ {aka ‘rclcpp::node_interfaces::NodeInterfaces<rclcpp::node_interfaces::NodeBaseInterface, rclcpp::node_interfaces::NodeParametersInterface, rclcpp::node_interfaces::NodeLoggingInterface, rclcpp::node_interfaces::NodeTimersInterface, rclcpp::node_interfaces::NodeTopicsInterface>’} to ‘message_filters::Subscriber<sensor_msgs::msg::CameraInfo_<std::allocator<void> > >::NodePtr’ {aka ‘std::shared_ptr<rclcpp::Node>’} 452 | NodePtr node, const std::string & topic, | ~~~~~~~~^~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:470:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe(NodeType*, const std::string&, rmw_qos_profile_t) [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node; std::string = std::__cxx11::basic_string<char>; rmw_qos_profile_t = rmw_qos_profile_s]’ 470 | void subscribe( | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:471:16: note: no known conversion for argument 1 from ‘image_transport::RequiredInterfaces’ {aka ‘rclcpp::node_interfaces::NodeInterfaces<rclcpp::node_interfaces::NodeBaseInterface, rclcpp::node_interfaces::NodeParametersInterface, rclcpp::node_interfaces::NodeLoggingInterface, rclcpp::node_interfaces::NodeTimersInterface, rclcpp::node_interfaces::NodeTopicsInterface>’} to ‘rclcpp::Node*’ 471 | NodeType * node, const std::string & topic, | ~~~~~~~~~~~^~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:490:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe(NodePtr, const std::string&, rmw_qos_profile_t, rclcpp::SubscriptionOptions) [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node; NodePtr = std::shared_ptr<rclcpp::Node>; std::string = std::__cxx11::basic_string<char>; rmw_qos_profile_t = rmw_qos_profile_s; rclcpp::SubscriptionOptions = rclcpp::SubscriptionOptionsWithAllocator<std::allocator<void> >]’ 490 | void subscribe( | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:490:8: note: candidate expects 4 arguments, 3 provided /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:512:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe(NodeType*, const std::string&, rmw_qos_profile_t, rclcpp::SubscriptionOptions) [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node; std::string = std::__cxx11::basic_string<char>; rmw_qos_profile_t = rmw_qos_profile_s; rclcpp::SubscriptionOptions = rclcpp::SubscriptionOptionsWithAllocator<std::allocator<void> >]’ 512 | void subscribe( | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:512:8: note: candidate expects 4 arguments, 3 provided /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:538:8: note: candidate: ‘void message_filters::Subscriber<M, NodeType>::subscribe() [with M = sensor_msgs::msg::CameraInfo_<std::allocator<void> >; NodeType = rclcpp::Node]’ 538 | void subscribe() override | ^~~~~~~~~ /opt/ros/rolling/include/message_filters/message_filters/subscriber.hpp:538:8: note: candidate expects 0 arguments, 3 provided gmake[2]: *** [CMakeFiles/image_transport.dir/build.make:146: CMakeFiles/image_transport.dir/src/camera_subscriber.cpp.o] Error 1 gmake[1]: *** [CMakeFiles/Makefile2:189: CMakeFiles/image_transport.dir/all] Error 2 gmake: *** [Makefile:146: all] Error 2 --- Failed <<< image_transport [6.97s, exited with code 2]

I had no build problems on my machine, could it be that you are using an older version of the message filter package? The PR adapting the interface to rclcpp::NodeInterfaces (this commit) has just been merged a few days ago, thus, you'll likely need the current rolling branch for compatibility.

You are right. It is good when I build message_filters from source since the binary is not released just yet.

image_transport/test/test_qos_override_lifecycle.cpp

image_transport/CMakeLists.txt

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde · 2025-04-11T19:06:40Z

Changes on image_transpor_plugins #352

authaldo · 2025-04-13T09:29:00Z

image_transport/include/image_transport/image_transport.hpp

 struct ImageTransport::Impl
 {
-  rclcpp::Node::SharedPtr node_;
+  RequiredInterfaces required_interfaces_;


~~Out of curiosity: Why did you remove the old pointer in your second iteration?~~

In my view keeping this pointer was one key benefit of your suggested implementation, as it allowed to keep the image_transport_plugins unchanged. I would really appreciate a plugin interface centered around the rclcpp::NodeInterfaces as the long run solution, however, I cannot quantify the additional overhead caused by the synchronization need for both repositories. As a maintainer you have likely more insights regarding this topic.
Depending on how large that overhead is, I would see your first iteration which kept the pointer as an acceptable solution since backwards compatibility is included. The full switch to using only rclcpp::NodeInterfaces in the plugin interfaces may then still be carried later on (potentially including multiple steps / deprecation cycles for backwards compatibility?)

EDIT:

Did the first version which kept the old pointer even work with the unmodified rolling state of the plugins for you? Even though it compiles I would expect runtime problems, since the plugins (e.g. for compressed image transport) explicitly override advertiseImpl(rclcpp::Node * node, ...).

authaldo · 2025-04-13T14:58:35Z

Changes on image_transpor_plugins #352

Just for clarity since you link this MR instead of the other repository:
The corresponding changes can be found here: ros-perception/image_transport_plugins#180

Since we are now back to the point where the two initial MRs started off (with changes both in image_common and image_transport_plugins), I would like to ask again which implications the synchronization need of both repositories has.
Can we simply merge both changes into the respective rolling branches? Anyone only using the "official" plugins shouldn't notice the change, since any release should contain the changes from both repositories. However, we might break any private plugins, but currently I do not see a way to avoid this.

ahcorde · 2025-04-15T09:42:45Z

Changes on image_transpor_plugins #352

Just for clarity since you link this MR instead of the other repository: The corresponding changes can be found here: ros-perception/image_transport_plugins#180

Since we are now back to the point where the two initial MRs started off (with changes both in image_common and image_transport_plugins), I would like to ask again which implications the synchronization need of both repositories has. Can we simply merge both changes into the respective rolling branches? Anyone only using the "official" plugins shouldn't notice the change, since any release should contain the changes from both repositories. However, we might break any private plugins, but currently I do not see a way to avoid this.

The idea is to merge these changes only in the rolling branches. The only way to notify non official plugins is to deprecate some methods or/and open an issue in the repositories.

I'm aware of (which are in rosdistro):

ffmpeg_image_transport
turbojpeg_compressed_image_transport

berndpfrommer · 2025-04-30T20:41:13Z

I maintain the ffmpeg_image_transport and foxglove_compressed_video_transport packages and encourage you to get this PR over the finish line. It's been almost 6 years now that this is an open issue.
I'll fix whatever is broken. Thanks!
PS: found this PR by accident when looking for how to write a life cycle node that uses image transport. If you want life cycle nodes to be used, this is an important one.

authaldo · 2025-05-05T08:14:58Z

I fully support the opinion of @berndpfrommer. This issue has been open for such a long time, hence, we should really move forward and implement the lifecycle node support (even though this might include breaking changes).

@ahcorde
Do you have an overview of what is missing?

ahcorde · 2025-05-08T12:44:12Z

@berndpfrommer and @authaldo

One of the big problems here, it's the advertiseImpl( signature which is implemented in the plugins. With these changes we have two signature one for the Node class and another one for the NodeInterfaces but only one can be called, depending with which contructor you use. This means we break user. We should find a better way to deprecated old API and keep the behaviour.

Suggestions are welcome:

One possible solution is deprecate PublisherPlugin and SubscriberPlugin and create new ones with the NodeInterface constuctor. But we need to create new classes with decent names.

What do you think ?

authaldo · 2025-05-08T14:38:16Z

Honestly, I am not sure whether there is even a nice way to move forward without breaking backwards compatibility (at least I haven't found one yet).

As far as my understanding goes we agree that the desired interface from image transport perspective should be a NodeInterfaces instance. Meaning that either a normal node or a lifecycle node will be split up into their individual interfaces.
Given these interfaces, we are not able to recover the "original" node class, meaning that any interface to a plugin will always be limited to a NodeInterface instance as well.

I see your point that breaking backwards compatibility will (potentially) create problems for users, however, I would like to highlight that the missing support for lifecycle nodes already burdens users to create workarounds (e.g. legacy node within lifecycle node only for image transport) or even blocks features (e.g. in the case of @berndpfrommer). And that for quite a long time.
Given that one of the plugin maintainers already offered his help, the user problems of breaking the interface might be tolerable as pull requests with corresponding fixes might be directly available for the plugins.

ahcorde · 2025-05-08T20:49:46Z

As this packages is a core package, I will bring this topic to next TGC meeting and let's see what we can do

authaldo · 2025-05-26T11:16:46Z

Are there any updates on this? I didn't find much within the official meeting protocol

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

authaldo · 2025-05-28T13:31:07Z

@ahcorde
Regarding your two linked merge requests:
Is there really a need / motivation to insert the manual conversion to the RequiredInterfaces everywhere?

If I remember correctly from my original PR, it should be possible to template the respective functions (see here. This way, everything which can be converted to the RequiredInterfaces class should be directly usable with image_transport (without any further changes in the user code).

In my view the "code duplication" of the templated function is justified given that it allows to reuse any other user code relying on image transport without changes.

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

authaldo · 2025-07-09T15:20:02Z

image_transport/include/image_transport/subscriber_filter.hpp

  SubscriberFilter(
-    rclcpp::Node * node, const std::string & base_topic,
+    rclcpp::Node * node,
+    const std::string & base_topic,
    const std::string & transport)
  {
-    subscribe(node, base_topic, transport);
+    subscribe(*node, base_topic, transport);
+  }


@ahcorde

Is there a future plan for this function? Will the rclcpp::Node accepting functions and constructors be deprecated in the future in favor of their NodeInterfaces equivalents?

If not:
Is there anything against templating the NodeType in those functions / constructors? This would also allow passing LifecycleNodes in there. From a user perspective it may be a bit confusing that we want to introduce lifecycle support but nonetheless keep the convenience overloads restricted to rclcpp::Node.

EDIT:
The question applies to all files where separate rclcpp::Node overloads are available, I've only chosen this instance as an example.

I would say I prrfer to use NodeInterfaces but can keep some methods which accept rclcpp::Node if that makes sense.

This is very nice documented issue ros2/geometry2#698 why we shouldn't use templates and why we should use rclcpp::node_interfaces::NodeInterfaces

There is a ppt too https://docs.google.com/presentation/d/1bdXOOZPhR9yAnyGNxoLhuO_bU4RW5IjnzvCtrVlpe_g/edit?slide=id.g24afec4abf4_0_0#slide=id.g24afec4abf4_0_0

Feel free to make suggestions in the code or open a PR against these one with your suggestions. Happy to review it and/or discuss these changes

Sorry if my suggestion wasn't formulated clearly, so let me have another try on this:

I am fully aware that rclcpp::node_interfaces::NodeInterfaces is the way to go (otherwise I wouldn't have introduced them within message filters). Instead, the intended target of my question are the "convenience" (wrapper) functions which are currently kept (i.e. taking rclcpp::Node*).
In my view it is not consistent to support both, normal and lifecycle nodes, but provide those "convenience" functions only for rclcpp::Node*.

In order to have a consistent interface, there are two options in my view:

Deprecate every method which relies on rclcpp::Node so that in the future only the NodeInterfaces are used.

Add support for lifecycle node pointers next to functions / c-tors which accept rclcpp::Node*.

I am slightly favoring option 2, since option 1 will potentially cause a lot of deprecation warnings. However, I would be fine with option 1 as well as my main point of criticsm is the inconsistency which would be solved by both options.

Added deprecation to ever method which relies on rclcpp::Node 6bf7d3d

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde · 2025-07-17T09:31:00Z

Pulls: #352, ros2/rviz#1472
Gist: https://gist.githubusercontent.com/ahcorde/fc00c5b8ef2e4dd0f753252ea6eaa924/raw/e2b20feef9596daeaf224bf29229a3906fbac53e/ros2.repos
BUILD args: --packages-above-and-dependencies image_transport
TEST args: --packages-above image_transport
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16515

Linux
Linux-aarch64
Linux-rhel
Windows

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde · 2025-07-17T10:08:52Z

Pulls: #352, ros2/rviz#1472
Gist: https://gist.githubusercontent.com/ahcorde/95d0c6d953c2580c825cf60358a0e30b/raw/e2b20feef9596daeaf224bf29229a3906fbac53e/ros2.repos
BUILD args: --packages-above-and-dependencies image_transport
TEST args: --packages-above image_transport
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16516

Linux
Linux-aarch64
Linux-rhel
Windows

ahcorde · 2025-07-18T08:17:03Z

@authaldo do you mind to take another look?

authaldo

@ahcorde

I only skimmed quickly through your last commit as the already mentioned issue with testing the code still persists for me. Once that is solved, I am happy to take a more detailed look at the code again.

Hence, I would again ask you (and the other who already approved the changes) to check whether the following works on your machine:

To simplify recreating the test, I switched to an official ros docker image:
docker run -it --rm ros:jazzy-perception

Additionally, I installed tmux and wget and cloned the following repositories:

git clone https://github.com/ros-perception/image_common.git --branch ahcorde/rolling/lifecycle
git clone https://github.com/ros2/message_filters.git
git clone https://github.com/authaldo/image_transport_tutorials.git
git clone https://github.com/ros-perception/image_transport_plugins

In my fork of the image_transport_tutorials the publisher node is a lifecycle node. After building and sourcing I entered tmux and split my terminal once. In the first one, I start the publisher with this image downloaded via wget:
ros2 run image_transport_tutorials my_publisher ROS2_Jazzy_Jalisco_poster.png

The second terminal is used for the subscriber. The raw subscription via
ros2 topic echo /camera/image --no-arr
works flawlessly. However, once the plugin is involved, e.g., by subscribing to the compressed image
ros2 topic echo /camera/image/compressed --no-arr the publisher node crashes instantly.

The current rolling branch of image_common together with the unmodified image_transport_tutorials publisher allows subscribing to the compressed image.

Does this only happen for me? Which plugins did you test? To the best of my knowledge the CI tests do not cover the interaction across the repositories (beyond building). Or am I wrong about that?

ahcorde · 2025-07-21T11:25:18Z

Did you try these branch in the follwoing repositories ?

https://github.com/ros-perception/image_transport_tutorials.git - ahcorde/rolling/support_lifecycle
https://github.com/ros-perception/image_transport_plugins - ahcorde/rolling/lifecycle

Sorry I didn't include these related PRs in the description, now it's updated

…fecycle

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde · 2025-07-22T10:22:01Z

@authaldo Let me know if this instructions are working for you

authaldo · 2025-07-29T19:03:46Z

@ahcorde

Sorry for the late feedback, I was unfortunately busy with other stuff! With the adapted version of the image_transport_plugins it works fine.

Honestly, I am a bit confused now:
In an earlier message you wrote that it should be usable without changes to the image_transport_plugins:

@ahcorde Just to be sure for testing: With 'keeping the behavior without breaking the API' you mean that this PR can now be used "standalone"? In other words, it doesn't require changes to the plugins and should thus be usable with the current state of the plugins (e.g. rolling branch in image_transport_plugins repository)?

I think so, you should see just the deprecation warning but the behavior of the plugins should remain

Combined with the draft state of the respective branch in the image_transport_plugins repository this made me think that the branch is stale and should be not included for the testing. However, adaptions are obviously needed (which is fine, based on my own MR #351 I always expected changes to be necessary in both repositories).

There is one last thing that bothers me:
In my initial test (with the unmodified image_transport_plugins) everything compiled fine, even though it crashed directly at runtime once a plugin was involved. Without the prior knowledge of testing this MR, my first guess regarding the crash reason would probably not have been a version mismatch.
Hence, wouldn't it be more convenient for the users if the version mismatch is directly obvious through a compiler error? (this is probably the case without the implementation of the pure virtual functions?)
I am mostly thinking about 3rd party or private plugins. If the maintainer doesn't know about these changes to image_transport, an end user might have a hard time figuring out the real cause for runtime crashes of the respective plugins.

authaldo

@ahcorde

Any thoughts on my last question regarding the compilation with an outdated version of the plugins (see previous comment)?

ahcorde · 2025-08-20T08:42:42Z

There is one last thing that bothers me:
In my initial test (with the unmodified image_transport_plugins) everything compiled fine, even though it crashed directly at runtime once a plugin was involved. Without the prior knowledge of testing this MR, my first guess regarding the crash reason would probably not have been a version mismatch.
Hence, wouldn't it be more convenient for the users if the version mismatch is directly obvious through a compiler error? (this is probably the case without the implementation of the pure virtual functions?)
I am mostly thinking about 3rd party or private plugins. If the maintainer doesn't know about these changes to image_transport, an end user might have a hard time figuring out the real cause for runtime crashes of the respective plugins.

Related with this, I will make an announcement in ROS discourse, I already sent message to mainteners of the other rosdistro image transport plugins. Do you think this is enough?

authaldo

@ahcorde
I just wanted to have it noted. Most users hopefully will anyways solely rely on the official plugins, so it shouldn't be too problematic. In the end you're the maintainer, so if it's fine for you then go ahead and merge this. We've waited long enough for lifecycle support within image_common 😄

ahcorde · 2025-08-21T11:53:51Z

Linux
Linux-aarch64
Linux-rhel
Windows

berndpfrommer · 2025-08-21T17:51:37Z

Thank you! This closes a major ROS2 feature gap. I can now implement lifecycle nodes for my camera drivers!

authaldo · 2025-08-22T07:35:20Z

Finally 👍
(although I would have enjoyed being listed as a co-author given the time I spent for that feature (including my previous PR #351 😅 ))

@ahcorde What's the state of ros-perception/image_transport_plugins#180? It is still marked as draft.
Without that being merged the current rolling branches of image_common and image_transport_plugins will happily compile together, but crash during runtime once any of the plugins is involved. This is exactly what I experienced during my testing.
Hence, I would opt for merging the plugins PR as soon as possible in order to minimize the chance of people building the mismatched versions.
Given how long people have waited for the lifecycle support I could imagine that at least some may build image_common from source in order to benefit from it earlier.

EDIT:
I think we can now also safely close #304 and #258 to keep the PR overview cleaner.

* Fix image_common NodeT deprecation warnings from ros-perception/image_common#352 - migrate to NodeInterfaces * Fix image_common rmw_qos_profile_t deprecation warnings from ros-perception/image_common#364 - migrate to rclcpp::QoS * Fix rviz update float deprecation warnings from ros2/rviz#1533 - migrate to std::chrono::duration * Fix geometry2 tf2_ros .h deprecation warnings from ros2/geometry2#805 - migrate Kilted and Rolling to .hpp * Fix geometry2 tf2_ros NodeT deprecation warnings from ros2/geometry2#714 - migrate to NodeInterfaces * Fix rclcpp spin_some deprecation warnings from ros2/rclcpp#2848 - migrate to SingleThreadedExecutor --------- Co-authored-by: Andrea <realeandrea@yahoo.it>

Support lifecycle node - NodeInterfaces

3de255d

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde self-assigned this Apr 8, 2025

This was referenced Apr 8, 2025

Draft: Lifecycle support via rclcpp::NodeInterfaces #351

Closed

Support lifecycle node #304

Closed

ahcorde requested a review from mikeferguson April 8, 2025 19:16

authaldo reviewed Apr 9, 2025

View reviewed changes

Benjamin-Tan suggested changes Apr 10, 2025

View reviewed changes

review feedback and reduce code duplicity

946582a

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde mentioned this pull request Apr 11, 2025

Support lifecycle ros-perception/image_transport_plugins#180

Merged

authaldo reviewed Apr 13, 2025

View reviewed changes

berndpfrommer mentioned this pull request May 7, 2025

Implement livecycle node ros-drivers/flir_camera_driver#237

Closed

Removed rclcpp::Node

547f3e9

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

This was referenced May 28, 2025

Support image transport lifecycle node ros-visualization/rqt_image_view#89

Merged

Support lifecycle Nodes ros-perception/image_transport_tutorials#20

Merged

This was referenced May 28, 2025

Support image_transport lifecycle ros-perception/image_pipeline#1099

Merged

Support image transport lifecycle ros2/rviz#1472

Merged

Fix pybind_image_transport

7c3840c

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde requested a review from mjcarroll June 2, 2025 11:24

authaldo reviewed Jul 9, 2025

View reviewed changes

Added deprecation warning to rclcpp::Node*

6bf7d3d

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde requested a review from authaldo July 17, 2025 09:29

make linters happy

d89185d

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

authaldo suggested changes Jul 18, 2025

View reviewed changes

ahcorde added 2 commits July 21, 2025 15:36

Merge remote-tracking branch 'origin/rolling' into ahcorde/rolling/li…

aaa464c

…fecycle

Fixed test

a8f9810

Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>

ahcorde requested a review from authaldo July 23, 2025 10:52

authaldo reviewed Aug 11, 2025

View reviewed changes

authaldo approved these changes Aug 20, 2025

View reviewed changes

ahcorde merged commit 97c03dc into rolling Aug 21, 2025
2 checks passed

ahcorde deleted the ahcorde/rolling/lifecycle branch August 21, 2025 17:34

ahcorde mentioned this pull request Sep 3, 2025

Lifecycle nodes #258

Closed

nbbrooks mentioned this pull request Sep 8, 2025

Fix deprecations in image_common and tf2_ros moveit/moveit2#3567

Merged

6 tasks

This was referenced Nov 6, 2025

[ROS2] image_transport does not support LifecycleNode #108

Closed

Unify LifecycleNode and Node ros2/rclcpp#2874

Open

Myzhar mentioned this pull request Feb 13, 2026

Add Lifecycle nodes support NVIDIA-ISAAC-ROS/isaac_ros_nitros#68

Open

peci1 mentioned this pull request Feb 24, 2026

ros2 port ctu-vras/sensor_filters#7

Draft

8 tasks

Conversation

ahcorde commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahcorde commented Apr 8, 2025

Uh oh!

authaldo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Benjamin-Tan commented Apr 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

authaldo Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ahcorde commented Apr 11, 2025

Uh oh!

authaldo Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

authaldo commented Apr 13, 2025

Uh oh!

ahcorde commented Apr 15, 2025

Uh oh!

berndpfrommer commented Apr 30, 2025

Uh oh!

authaldo commented May 5, 2025

Uh oh!

ahcorde commented May 8, 2025

Uh oh!

authaldo commented May 8, 2025

Uh oh!

ahcorde commented May 8, 2025

Uh oh!

authaldo commented May 26, 2025

Uh oh!

authaldo commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

authaldo Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahcorde commented Jul 17, 2025

Uh oh!

ahcorde commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahcorde commented Jul 18, 2025

Uh oh!

authaldo left a comment

Choose a reason for hiding this comment

Uh oh!

ahcorde commented Jul 21, 2025

Uh oh!

ahcorde commented Jul 22, 2025

Uh oh!

authaldo commented Jul 29, 2025

ahcorde commented Apr 8, 2025 •

edited

Loading

authaldo left a comment •

edited

Loading

authaldo Apr 11, 2025 •

edited

Loading

authaldo Apr 13, 2025 •

edited

Loading

authaldo commented May 28, 2025 •

edited

Loading

authaldo Jul 9, 2025 •

edited

Loading

ahcorde commented Jul 17, 2025 •

edited

Loading

ahcorde commented Aug 21, 2025 •

edited

Loading

authaldo commented Aug 22, 2025 •

edited

Loading