Write QoS metadata YAML as multiline

[emersonknapp]

Description

Currently, the offered_qos_profiles that are stored in metadata.yaml are stored as a serialized yaml string within the yaml file, e.g.

offered_qos_profiles: "- history: 3\n  depth: 0\n  reliability: 1\n  durability: 2\n  deadline:\n    sec: 2147483647\n    nsec: 4294967295\n  lifespan:\n    sec: 2147483647\n    nsec: 4294967295\n  liveliness: 1\n  liveliness_lease_duration:\n    sec: 2147483647\n    nsec: 4294967295\n  avoid_ros_namespace_conventions: false"

This would be simpler if it was also serialized and formatted as YAML rather than a string

Related Issues

N/A

Completion Criteria

Metadata looks like the following instead:

offered_qos_profiles: 
  - history: 3
    depth: 0
    reliability: 1
    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

Implementation Notes / Suggestions

Possibly need to bump the metadata version for this

Testing Notes / Suggestions

Make sure that both the old string types and the new types are correctly parsed

[guichristmann]

Is this still somewhat relevant? I'd like to try tackling this issue.

I've done a quick preliminary check of the necessary code changes.

I believe we should replace the type of the field offered_qos_profiles in TopicMetadata (in rosbag2_storage) from an std::string to a std::vector<rclpp::QoS>. Then, we can add an specialization to the convert template class in rosbag2_storage/include/rosbag2_storage/yaml.hpp to take an std::vector<rclpp::QoS>.

Since we modified the type of a member in the struct TopicMetadata we would also need to reflect these changes everywhere the field offered_qos_profiles is accessed expecting an std::string, but I haven't looked at how deep this goes yet.

In terms of making it compatible with previous versions, we could bump this new metadata format to version 7 (I think the current newest is 6 it seems?). For previous versions we can add a diverging code path to parse the serialized field with a 2nd YAML::Load call (as it is currently done) and use the new convert template specialization to fit it into the std::vector<rclpp::QoS> type.

Appreciate hearing other people's thoughts on this direction or if this is misguided. I'm just starting to dip my toes in the codebase and this would be my first contribution.

Cheers.

[emersonknapp]

Main issue with this approach is that right now rosbag2_storage has no dependency on rclcpp, which we've tried to avoid in favor of making the storage, at least, a "transport-free" layer of the package - rclcpp brings in the entire transport/networking system of ROS 2. This was not all entirely successful as now rosbag2_cpp does depend on it, so while the lowest-level storage API rosbag2_storage doesn't have this dependency, every user-facing API does anyway.

That part aside, while I think the approach you are considering is not crazy, it might be overkill to accomplish the goal here. The real issue is just that there aren't any newlines in the offered_qos_profiles YAML field - not really that the data is unstructured at this point in the code (though maybe there could be benefits from adding the strong typing in TopicMetadata? I'm not sure without an example at hand)

I have a branch https://github.com/ros2/rosbag2/tree/emersonknapp/metadata-qos-yaml-format where i started on this - it just has the commit b9ea95a which as you can see is pretty small.

Since so much changed since I wrote that a year ago - I lost track of it in other busyness - it was awkward to rebase, so I just made a new branch and opened draft PR

#1197

If that passes all the existing tests, it should probably have at least one new unit test to try the new behavior specifically.

Sorry, not to try an swipe an attempt at picking up a ticket! Just, this reminded me I had (maybe) already solved this one. We'll see how that goes. Maybe you want to take over that PR and push it through to the end? Or, there are probably other goods tickets on the backlog.

[guichristmann]

Thanks for the reply! Seems like you already got it sorted out in a much simpler way than I had in mind. In that case, I'll let you take this one over and I'll look for a different issue where I can contribute 😄.

[roncapat]

@MichaelOrlov shall this one be closed too?

[MichaelOrlov]

• Closing as done in the Use enum values for offered_qos_profiles in code and string names in serialized metadata #1476