image_common icon indicating copy to clipboard operation
image_common copied to clipboard

[ROS2] image_transport does not support LifecycleNode

Open Myzhar opened this issue 5 years ago • 12 comments

Hi, I'm trying to integrate the new image_transport from Crystal in my LifecycleNode, but the functions image_transport::create_camera_publisher and image_transport::create_camera_publisher only take rclcpp::Node* node as parameter.

Is there any workaround?

Myzhar avatar Jan 16 '19 09:01 Myzhar

Not currently, I believe that this would require some further work to integrate the LifecycleNode.

mjcarroll avatar Jan 17 '19 22:01 mjcarroll

I think that using a rclcpp::node_interfaces::NodeBaseInterface::SharedPtr instead of a rclcpp::Node* is the right way.

There is a similar problem with the TF Broadcaster

The rclcpp::executor::Executor uses the Base Interface: https://github.com/ros2/rclcpp/blob/master/rclcpp/include/rclcpp/executor.hpp#L124

Myzhar avatar Jan 18 '19 08:01 Myzhar

Any updates on this one?

marting87 avatar Aug 19 '19 15:08 marting87

Any progress on this? It looks like the above issue mention on this has been partially resolved as I am now able to do std::make_shared<rclcpp::AsyncParametersClient>( this->get_node_base_interface(),this->get_node_topics_interface(), this->get_node_graph_interface(), this->get_node_services_interface()); on Ubuntu 20.04 Foxy which is a bit more verbose than would be desired but gets the job done. Would something like that work here?

Michael-Equi avatar Jul 25 '20 05:07 Michael-Equi

This issue is increasingly relevant as managed lifecycle nodes are used more widely. In my opinion it makes a lot of sense for an image processing node to have lifecycle states, so that we can configure it with image parameters, and then activate / deactivate potentially expensive processing and perception steps through the lifecycle interfaces.

More related issues like the ones above will keep arising, and the workarounds are generally less efficient (such as giving up on the image transport layer and just using raw pub / sub of sensor_msg Image instead).

Thanks @Rayman for the initiative to solve this. I believe #190 is a great demonstration of the concept of using base interfaces rather than a Node instance to allow for lifecycle nodes and really any other node-like classes. It sets the precedent for the slightly larger #167 (but which ultimately follows the same concept).

Is there anything that can be done to encourage these PRs and this issue to be closed?

eeberhard avatar Aug 08 '22 14:08 eeberhard

I don't know why it takes so long to merge these PRs. It's a relatively simple change. There should be more activity in such a core ros package.

Rayman avatar Aug 22 '22 12:08 Rayman

#190 fell off my todo list for some reason. I've started it moving forward again.

#167 cannot be merged as it is still a draft PR. Until the author does the necessary work to finish the PR, it will remain that way.

gbiggs avatar Aug 22 '22 21:08 gbiggs

@gbiggs created another draft PR https://github.com/ros-perception/image_common/pull/258

ilidar avatar Nov 05 '22 17:11 ilidar

Any update on this ? we want to use lifecycle nodes in our package, but still blocked by image_transport component.

SamerKhshiboun avatar Mar 24 '23 09:03 SamerKhshiboun

Any progress on this? It looks like the above issue mention on this has been partially resolved as I am now able to do std::make_shared<rclcpp::AsyncParametersClient>( this->get_node_base_interface(),this->get_node_topics_interface(), this->get_node_graph_interface(), this->get_node_services_interface()); on Ubuntu 20.04 Foxy which is a bit more verbose than would be desired but gets the job done. Would something like that work here?

@Michael-Equi , Can you advice on how to call image_transport::create_publisher with lifecycle node ? Any conversion that I can do to the lifecycle node to get a raw node from it ?

Thanks

SamerKhshiboun avatar Mar 24 '23 09:03 SamerKhshiboun

Any update on this? Is this package still being maintained/developed?

Benjamin-Tan avatar Oct 31 '23 12:10 Benjamin-Tan

Any update on this? Is this package still being maintained/developed?

No update that I'm aware of. Generally, I think maintenance is limited to bugfixes and releases. We are always happy to take/review contributions, but I believe that the PR in question here is still in draft?

mjcarroll avatar Oct 31 '23 14:10 mjcarroll