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

Mixed-QoS settings used for ros2 bag play #629

Open
mjeronimo opened this issue Feb 1, 2021 · 5 comments
Open

Mixed-QoS settings used for ros2 bag play #629

mjeronimo opened this issue Feb 1, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@mjeronimo
Copy link
Contributor

Documenting the following issue to get feedback and understand if it is worth addressing or if the case is so infrequent that a change like this is not worth considering...

Description

When recording, ros2 bag record saves QoS information for each publisher on a specific topic to the database (under 'offered_qos_profiles'). Each message received on that topic is also saved. However, there is not an association between the messages and the QoS settings used to originally publish the messages. Currently, when playing back from a previously-recorded database, ros2 bag play creates a publisher for each topic (even if there were originally multiple publishers for that topic). When creating the publisher, it needs to provide a QoS profile. If the topic had a single publisher, or if a topic had multiple publishers but they all used the same QoS profile, there is not a problem figuring out the QoS profile to use. However, if the original publishers used different QoS profiles, ros2 bag play needs to adapt the recorded QoS profiles ("offered_qos_profiles") into one it can use for the publisher. In this case, the rosbag2 code uses a default QoS setting:

Rosbag2QoS Rosbag2QoS::adapt_offer_to_recorded_offers(
  const std::string & topic_name, const std::vector<Rosbag2QoS> & profiles)
{
  if (profiles.empty()) {
    return Rosbag2QoS{};
  }
  if (all_profiles_effectively_same(profiles)) {
    auto result = profiles[0];
    return result.default_history();
  }
  ROSBAG2_TRANSPORT_LOG_WARN_STREAM(
    "Not all original publishers on topic " << topic_name << " offered the same QoS profiles. "
      "Rosbag2 cannot yet choose an adapted profile to offer for this mixed case. "
      "Falling back to the rosbag2_transport default publisher offer.");
  return Rosbag2QoS{};
}

The result is that "ros2 bag play" is not quite faithful, from a QoS perspective to the original publishing.

Implementation Notes / Suggestions

To implement this case, we would need to know the QoS settings used for each published message. We currently store the QoS profiles, but they are not associated with individual messages, but are only associated to a topic (which could possibly have multiple publishers with different QoS). It may be that this is a rare situation and publishers of the same topic should be using the same QoS settings.

@mjeronimo mjeronimo added the enhancement New feature or request label Feb 1, 2021
@emersonknapp emersonknapp changed the title QoS settings used for ros2 bag play Mixed-QoS settings used for ros2 bag play Feb 3, 2021
@emersonknapp
Copy link
Collaborator

emersonknapp commented Feb 3, 2021

This is definitely something we considered when building it (and clearly just backed off from making a final decision). The problem here is that because ROS 2 is built up as an anonymized pub/sub system - we don't know where an individual message is coming from, and therefore don't know how to associate a specific message with a specific QoS.

There are a few cases to consider:

  1. By querying all the QoS Profiles, you could create multiple subscriptions, each with one of the unique qos profiles. Any message coming in on the suscription could then be tagged with the associated QoS profile

    • you will (most likely) receive duplicate messages, because some subscriptions will likely be compatible with more than one of the publishers, given that most qos profiles are in a "strictness order" for compatibility - the least strict subscription is likely to receive a superset of the messages from a more strict subscription
  2. By querying the exact origin ID of each message, we could then associate that message with an output publisher profile

    • Right now we have no way of knowing, through the RMW API, what participant a given message is coming from
    • EDIT: I did some looking at the DDS spec - the presence of SampleInfo.publication_handle that comes with incoming samples to the DataReader indicates that it's possible to recreate this information properly. Given that the goal of rosbag is to record and playback data as faithfully as possible, it's worth considering exposing a new RMW API for the purpose - maybe not intended to be used by most parts of ROS2 as a "low level API".
  3. We could construct a QoS profile for the single replay publisher, that would be "compatible with all original publishers"

    • This requires offering the "strictest common denominator" of mixed QoS profiles. This means that all original subscriptions on the given topic would be able to receive messages from the playback. BUT, this means that some original subscriptions that had a more strict requirement originally, would match with "all publishers on the topic" (the single playback publisher) rather than only a subset like it would have in the original system

Clearly option 1 is unacceptable (probably fully duplicatilng some topics' messages). Option 3 may be a decent compromise but could have subtly confusing side effects for users. Option 2 is the only truly correct way, but it also will require new RMW APIs to expose information that is not presently available.

EDIT 2: Leaving above for context - I did not realize that that RMW API rmw_take_with_info exists. This appears to make this problem a lot easier - we could tag messages per origin publisher and play them back on a Publisher with the appropriate QoS. Now that I'm aware of this API the problem seems less daunting. I think it will still require a good deal of code and testing, best broken up into several PRs, but it seems more doable assuming this API is properly implemented for at least the 3 top-tier RMW implementations

EDIT 3: I really should do all my research before posting conclusions - the GenericSubscription that rosbag2_transport uses to receive data already has access to the rclcpp exposure of the rmw_take_with_info API - in handle_message https://github.com/ros2/rosbag2/blob/master/rosbag2_transport/src/rosbag2_transport/generic_subscription.hpp#L62. I've done a quick experiment and am able to get the publisher GIDs of messages. It may be worth writing up all these findings into a design doc to inform development on this feature, once it comes to top of backlog. I have tested it out quickly at #633 and it seems easy enough to get this info, the only part we have to worry about is what to do with it

@jacobperron
Copy link
Member

Thanks @mjeronimo for opening the issue and @emersonknapp for laying out the options. I agree that option 2 seems the most appropriate.

I suggest we leave this ticket open for visibility, in case someone really wants to tackle it. Possibly label it as "backlog", as it doesn't sound like something we're planning to address.

@emersonknapp
Copy link
Collaborator

Not sure if comment edits notify watchers, so I am commenting to note that I edited the above with a lot more promising context.

@mjeronimo
Copy link
Contributor Author

@emersonknapp Thanks for the helpful info. That'll give someone a good start on this.

@MichaelOrlov
Copy link
Contributor

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