Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ROS2] image_transport does not support LifecycleNode #108

Open
Myzhar opened this issue Jan 16, 2019 · 14 comments · May be fixed by #304
Open

[ROS2] image_transport does not support LifecycleNode #108

Myzhar opened this issue Jan 16, 2019 · 14 comments · May be fixed by #304
Labels
enhancement ros2 Issues or Pull Requests targeting ROS2

Comments

@Myzhar
Copy link

Myzhar commented Jan 16, 2019

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?

@mjcarroll mjcarroll added enhancement ros2 Issues or Pull Requests targeting ROS2 labels Jan 17, 2019
@mjcarroll
Copy link
Contributor

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

@Myzhar
Copy link
Author

Myzhar commented Jan 18, 2019

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

@marting87
Copy link

Any updates on this one?

@Michael-Equi
Copy link

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?

@eeberhard
Copy link

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?

@Rayman
Copy link
Contributor

Rayman commented Aug 22, 2022

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.

@gbiggs
Copy link
Contributor

gbiggs commented Aug 22, 2022

#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.

@ilidar
Copy link

ilidar commented Nov 5, 2022

@gbiggs created another draft PR #258

@SamerKhshiboun
Copy link

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

@SamerKhshiboun
Copy link

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

@Benjamin-Tan
Copy link

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

@mjcarroll
Copy link
Contributor

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?

@LightRadiance
Copy link

Any update on this?

@tgreier
Copy link

tgreier commented Oct 9, 2024

Looking for an update here as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ros2 Issues or Pull Requests targeting ROS2
Projects
None yet
Development

Successfully merging a pull request may close this issue.