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

Export generic_subscription.hpp/cpp and generic_publisher.hpp/cpp for external use #378

Closed
TomyTomm opened this issue Apr 15, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@TomyTomm
Copy link

Description

I am working on creating a ROS cloud extension which will allow the customer to stream video and raw data to an external peer, and I would like it to allow for any ROS message types which the user may input. These files are currently located in the rosbag2_transport package.

@TomyTomm TomyTomm added the enhancement New feature or request label Apr 15, 2020
@Karsten1987
Copy link
Collaborator

Karsten1987 commented Apr 15, 2020

I had this discussion with @emersonknapp in an earlier PR already. I personally think that this generic subscription is actually better suited within rclcpp than rosbag2. I don't think one has to be depend on rosbag2_transport for using this functionality.

I am okay with making them public for Foxy though if that helps. @emersonknapp any thoughts? We can also make a push for rclcpp directly to see if that still lands in Foxy?
I guess technically, we could even do our own package for it, not sure it's worth it though.

@emersonknapp
Copy link
Collaborator

emersonknapp commented Apr 15, 2020

I think we're probably out of luck for including in rclcpp. The api freeze extension only applied to pre-planned work that slipped. I'd say for Foxy we expose from rosbag2 somewhere, and try to include it more centrally in G-turtle.

@emersonknapp
Copy link
Collaborator

We could spin out a separate "generic transport" package within the rosbag2 repo, to make the distinction clear and the dependency small. The port into rclcpp later would be easy then

@emersonknapp
Copy link
Collaborator

But, yeah, it may not be worth the extra complexity/effort right now.

@Martin-Idel
Copy link

Just FYI: Here is the ticket in rclcpp which asks for that feature ros2/rclcpp#544 - although the description is a bit too focused on the implementation.

The original plan was to make the split in rosbag such that one could just move the files and change some namespacing and such, but for various reasons we couldn't create it directly in rclcpp.

@emersonknapp
Copy link
Collaborator

ros2/rclcpp#1077
ros2/rclcpp#1076

PRs in rclcpp to add this functionality there. This ticket wouldn't be closed until we moved the dependency over and removed the classes from rosbag2

@emersonknapp
Copy link
Collaborator

ros2/rclcpp#1452 is replacing the above PRs

@Karsten1987
Copy link
Collaborator

closing with ros2/rclcpp#1452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants