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

Move the QoS detection code to a more central place #601

Open
clalancette opened this issue Jan 14, 2021 · 3 comments
Open

Move the QoS detection code to a more central place #601

clalancette opened this issue Jan 14, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@clalancette
Copy link
Contributor

Description

rosbag2 currently has the ability to automatically detect the QoS of publishers and set itself up to successfully receive data from them. It looks like this went in via the following issue: #125 . Also, it looks like the core code that implements this is here and here.

This is a great capability that has applicability outside of rosbag2. At least the following two other use cases have come up:

  1. When using the ros1_bridge, it would be nice if the QoS could be auto-detected across the bridge.
  2. When using RViz2, it would be nice if the user didn't have to specify the QoS settings when subscribing to topics.

There may be other uses.

The idea here would be to take that Rosbag2QoS class (and any supporting infrastructure), rename it, and move it into a more central location (probably rclcpp, though we may want to consider putting it in the rcl layer so other client libraries have access to it).

Related Issues

#125

Completion Criteria

  • Move the Rosbag2QoS class to a central location
  • Move the tests (as appropriate) for Rosbag2QoS to a central location
  • Refactor rosbag2 to use the new class

Testing Notes / Suggestions

Unit tests already exist for this, so making sure those unit tests continue to run. Also making sure that rosbag2 continues to operate as expected.

@clalancette clalancette added the enhancement New feature or request label Jan 14, 2021
@emersonknapp
Copy link
Collaborator

I absolutely agree in the usefulness of this functionality (ros2 topic echo!) - and have it on my roadmap for this year. Thanks for logging this as a publicly visible issue, I forgot to do that.

I believe that rcl would be best, so that rclpy can use the functionality as well (ros2cli!) - this will require a rewrite into C, but shouldn't be too difficult to do.

I would love to get this done in time for Galactic, but I'm not able to commit to that timeline today.

@clalancette
Copy link
Contributor Author

That all sounds good, thanks @emersonknapp . This came up in the context of ros2/ros1_bridge#304 ; it isn't really required to fix that issue, but it could help. So from my end, there is no immediate need, it would just be nice to have it available.

@jacobperron
Copy link
Member

This functionality should certainly be moved to a common place. I think at least it should live in rcl so that rclpy (and other client libraries) can take advantage of it. However, I'd argue that rmw may make the most sense if we consider that non-DDS implementations may have different rules for compatibility. I've opened up a ticket proposing an RMW API: ros2/rmw#304

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

3 participants