Use QoS for subscription, recording, and playback

[Karsten1987]

Feature Request

• Detect QoS for topics in order to subscribe to all topics correctly
• Record QoS of topics in the bag
• Playback bags with recorded QoS
• Option to override the recorded QoS with a different profile on playback (e.g. recorded a BEST_EFFORT, but want to play back as RELIABLE)

Original issue report:

as the title says. When publishing topics with unreliable qos, the bag does not record any data on this topic as it expects a reliable connection - meaning all subscribers are spawned as reliable.

Work tracking

Tracking in-progress to-do list here

QoS storage in metadata

• QoS equality operators Add equality operators for QoS profile rclcpp#1032
• QoS YAML serialization Add QoS to metadata (re-do #330) #335
• QoS field add to metadata Add QoS to metadata (re-do #330) #335
• Query and store qos for a given topic at subscription time Query offered QoS profiles for a topic and store in metadata #333

Testing robustness

• Run tests on all rmw implementations
  • Fix small ament bug gmock - Don't pass FALSE value to ament_add_test when SKIP_TEST is not provided ament/ament_cmake#236
  • Run the rosbag2_transport tests on all rmw impls Test all RMW implementations for rosbag2_transport #349

Record with correct QoS

• Allow the GenericPublisher and GenericSubscription to receive and use a QoS profile Allow GenericPublisher / GenericSubscription to take a QoS profile (re-do #337) #340
• Utilize queried qos to create subscription (if all offered profiles the same)
  • Initial implementation Subscribe to topics using the common offered QoS #343
  • disable tests because not working on cyclone Disable adaptive qos subscription for now  #348
  • More holistic implementation Intelligently subscribe to topics according to their QoS profiles #355

Playback with correct QoS

• Read saved QoS profiles from bag metadata into TopicMetadata Read serialized qos profiles out of the metadata #359
• Expose YAML metadata to the Player Expose BaseReaderInterface's BagMetadata  #377
• Provide adaptive QoS logic for publishing Move qos utilities to their own compilation unit #379
• Use adaptive QoS to create publisher for playback Adaptive playback qos based on recorded metadata #364

Explicit profile overrides

• Let user provide override QoS file to CLI for recording WIP: Set default QoS Profile - Record #341
• Let user provide override QoS file to CLI for playback Override QoS Profiles in CLI - Playback #356
• Enable explicit per-topic qos profile override for recording Add QoS Profile override to CLI #347
• Enable explicit per-topic qos profile override for playback QoS Profile Overrides - Player #353
• ros2bag refactoring Refactor utility functions in ros2bag #358

[dirk-thomas]

This sounds rather severe. Should this be added to the Dashing patch release 1 project?

[Jconn]

I've got some sensor data that I want to record while testing robot localization, and I'm running into this.

My workaround for is to replace rcl_subscription_get_default_options() in the generic_subscription.cpp constructor with sensor options.

I would think the correct fix for this is mirroring the QoS options of the topic you're recording. Are those options accessible?

[paulbovbel]

This seems like part of a general problem in rosbag2 due to QoS mismatches. Is there an intent to eventually replicate a mechanism like ROS1's bagging of latched, where the bag records a topic's QoS settings and then replicates them on playback?

[mkhansenbot]

I'm running into this issue when trying to record the /scan topic. Any ETA on a fix?

[Karsten1987]

Sorry y'all for having this issue open for so long unattended. I just haven't gotten my head around to allocate some time for it.

I see three possibilities to proceed here:

1. we extend the command line interface to take a --unreliable <topic1> <topic2> ... option which explicitly lists topics with are not sent reliably. A similar approach should then be taken with --latched .... However, that can only cope with predefined QoS profiles and doesn't really allow to adapt to custom settings.
2. as an extension to option 1, we could read a user specified .yaml file or such which provides a map between topic and its QoS settings. That would allow for being more flexible to adapt to custom designed settings.
3. an actual fix would be to have the qos settings auto-discovered. There is works on the way for it, but I don't have a precise ETA on when that feature would be really implemented: Expose QOS settings for a topic rmw#151

I am a bit puzzled whether to get started with option 1 or 2 for then just to remove that feature later on once option 3 is completed.
Opinions?

[Jconn]

It looks like the umbrella of PRs related to ros2/ros2cli#385 are implementing QoS discovery. I'm okay waiting for these to come in and then implementing option 3.

I think there is probably a 'correct' answer for other QoS settings besides reliability, e.g. I want my rosbag subscriber history to be large enough that I don't drop messages out the back. Which means there isn't a compelling reason to make depth a per-topic parameter, so I don't see a need to implement option 2 for other QoS parameters.

[mkhansenbot]

I'm happy with option 1 unless option 3 will be ready for merge soon. I kind of doubt that however as 5 PRs have to merge. I didn't go through them though, so not sure how much work is needed there.

In any case I have a work-around (I think) for my issue not using rosbag.

[mkhansenbot]

I just noticed that there is already some command line support in ros2 topic echo for adding a qos-profile etc:

mkhansen@mkhansen-desk:~/ros2_dev/navigation2_ws/src/navigation2/nav2_system_tests/src/updown$ ros2 topic echo
usage: ros2 topic echo [-h]
                       [--qos-profile {system_default,sensor_data,services_default,parameters,parameter_events,action_status_default}]
                       [--qos-reliability {system_default,reliable,best_effort}]
                       [--qos-durability {system_default,transient_local,volatile}]
                       [--csv] [--full-length]
                       [--truncate-length TRUNCATE_LENGTH] [--no-arr]
                       [--no-str]
                       topic_name [message_type]
ros2 topic echo: error: the following arguments are required: topic_name

I don't know if that would be easier to add support for in rosbag2 than what was suggested in option 1 above, just thought it might help.

[emersonknapp]

I think the auto-discovery work (as per the PRs on ros2/rmw#151) will provide the most meaningful solution to this issue, since we don't want to have to know all the QoS in order to ros2 bag record -a. But, a shorter term solution may be desired

[emersonknapp]

@Karsten1987 I need to serialize rclcpp::QoS or rmw_qos_profile_t (either is fine) to YAML for storing as metadata (regardless of whether I put it in the yaml file or into the database - as we discussed a binary dump wouldn't be future-proof). For my prototype I'm implementing that conversion within rosbag2_transport because it already has a dependency on rclcpp - but the ser/deser logic seems more appropriate for rosbag2_storage. storage currently has no dependency on rclcpp or rmw, do you think it is ok to add this dependency in order to keep serialization logic all in one place?

[Karsten1987]

would rosbag2_cpp be the right place for it? That's where all the message (de-)serialization is happening as well.

This is certainly up for discussion and of no high priority to get things going, but I could also see that implementation of std::string to_string(rclcpp::QoS) being part of rclcpp. That could make it easier to update in case any fields are added to the QoS profile.

[emersonknapp]

implementation of std::string to_string(rclcpp::QoS) being part of rclcpp

and from_string. Would rclcpp feel the same responsibility for backwards compatibility? It's not generally concerned with long-lived storage that may be played on different ROS versions, which an important consideration for rosbag. Having the serialization close to the data definition is a point in favor of this though, to make it easier to keep in sync.

It seems like it's best to implement as a yaml-cpp convert struct so that we can seamlessly include the QoS in whatever YAML. The preexisting convert structs we have defined live in rosbag2_storage/metadata_io.cpp which is why my instinct was to put the implementation there, to preserve types as long as possible. Converting the QoS to YAML string beforehand and using those strings as fields in the YAML leads to less readable formatting, but maybe that's not a dealbreaker, see this example:

rosbag2_bagfile_information:
  version: 3
  storage_identifier: sqlite3
  relative_file_paths:
    - bag4/bag4_0.db3
  duration:
    nanoseconds: 8999916874
  starting_time:
    nanoseconds_since_epoch: 1584577824549133950
  message_count: 10
  topics_with_message_count:
    - topic_metadata:
        name: /chat
        type: std_msgs/msg/String
        serialization_format: cdr
        offered_qos_profiles:
          - "history: 3\ndepth: 0\nreliability: 2\ndurability: 2\ndeadline:\n  sec: 2147483647\n  nsec: 4294967295\nlifespan:\n  sec: 2147483647\n  nsec: 4294967295\nliveliness: 1\nliveliness_lease_duration:\n  sec: 2147483647\n  nsec: 4294967295\navoid_ros_namespace_conventions: false"
          - "history: 3\ndepth: 0\nreliability: 1\ndurability: 1\ndeadline:\n  sec: 2147483647\n  nsec: 4294967295\nlifespan:\n  sec: 2147483647\n  nsec: 4294967295\nliveliness: 1\nliveliness_lease_duration:\n  sec: 2147483647\n  nsec: 4294967295\navoid_ros_namespace_conventions: false"
      message_count: 10
  compression_format: ""

The string can be read node["offered_qos_profiles"].as<std::string>() and then parse that string as YAML when a QoS is needed.

A nicer solution might look like this:

rosbag2_bagfile_information:
  version: 3
  storage_identifier: sqlite3
  relative_file_paths:
    - tbag4/tbag4_0.db3
  duration:
    nanoseconds: 8999916874
  starting_time:
    nanoseconds_since_epoch: 1584577824549133950
  message_count: 10
  topics_with_message_count:
    - topic_metadata:
        name: /chat
        type: std_msgs/msg/String
        serialization_format: cdr
        offered_qos_profiles:
          - history: 3
            depth: 0
            reliability: 2
            durability: 2
            deadline:
              sec: 2147483647
              nsec: 4294967295
            lifespan:  
              sec: 2147483647
              nsec: 4294967295
            liveliness: 1
            liveliness_lease_duration:
              sec: 2147483647
              nsec: 4294967295
            avoid_ros_namespace_conventions: false
          - history: 3
            depth: 0
            reliability: 1
            durability: 1
            deadline:
              sec: 2147483647
              nsec: 4294967295
            lifespan:  
              sec: 2147483647
              nsec: 4294967295
            liveliness: 1
            liveliness_lease_duration:
              sec: 2147483647
              nsec: 4294967295
            avoid_ros_namespace_conventions: false
      message_count: 10
  compression_format: ""

[emersonknapp]

EDIT: Put the work tracking in the issue description for more visibility