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

Document the offered_qos_profiles argument of TopicMetadata #1420

Closed
asymingt opened this issue Jul 13, 2023 · 3 comments
Closed

Document the offered_qos_profiles argument of TopicMetadata #1420

asymingt opened this issue Jul 13, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@asymingt
Copy link
Contributor

asymingt commented Jul 13, 2023

Description

Right now it's not clear how to use the offered_qos_profiles argument of TopicMetadata in the rosbag2_py API. By looking at the source code I have worked out that it's stringified YAML but I can't work out the exact syntax. Looking at the QoS overrides syntax, I would assume that it's something like this:

import rosbag2_py

offered_qos_profiles = """
- reliability : reliable
  durability: volatile
  history: keep_last
  depth: 100
"""

topic_metadata = rosbag2_py.TopicMetadata(
    name="/clock", 
    type="rosgraph_msgs/msg/Clock",
    offered_qos_profiles=offered_qos_profiles,
    serialization_format="cdr")

Or possibly something like this:

import yaml
import rosbag2_py
from rclpy.qos import QoSProfile, QoSReliabilityPolicy, QoSDurabilityPolicy, QoSHistoryPolicy

offered_qos_profiles = yaml.dump([{
   "reliability" : int(QoSReliabilityPolicy.RELIABLE),
    "durability": int(QoSDurabilityPolicy.VOLATILE),
    "history": int(QoSHistoryPolicy.KEEP_LAST),
    "depth": 100}], default_flow_style=True)

topic_metadata = rosbag2_py.TopicMetadata(
    name="/clock", 
    type="rosgraph_msgs/msg/Clock",
    offered_qos_profiles=offered_qos_profiles,
    serialization_format="cdr")

In both cases there is no runtime error and the bag superficially looks good:

$ ros2 bag info preprocess.bag

Files:             preprocess.bag_0.mcap
Bag size:          2.4 MiB
Storage id:        mcap
Duration:          4290.0s
Start:             Oct 12 2015 12:00:00.0 (1444651200.0)
End:               Oct 12 2015 13:11:30.0 (1444655490.0)
Messages:          42901
Topic information: Topic: /clock | Type: rosgraph_msgs/msg/Clock | Count: 42901 | Serialization Format: cdr

The trouble is when you try and play it, then you get this error:

$ ros2 bag bag play preprocess.bag
[INFO] [1689265071.129672762] [rosbag2_player]: Set rate to 1
invalid node; this may result from using a map iterator as a sequence iterator, or vice-versa

I have confirmed that removing offered_qos_profiles=offered_qos_profiles from the TopicMetadata constructor removed warning at the cost of losing QoS information.

Related Issues

None

Completion Criteria

Need to have: A section in the root README that provides a short guide on how to get the QoS syntax correct to ensure that the offered profiles are parseable at runtime.

Nice to have: at bag generation time, issue a warning or error if the profiles are not readable when you call writer.create_topic(topic_metadata).

Implementation Notes / Suggestions

I'd be happy to implement this feature if you show me one example of how to get this working. I still am unable to the the offered QoS profiles stringified YAML syntax correct.

Testing Notes / Suggestions

A test showing that an unparseable offered_qos_profiles value results in a warning or error being thrown when writer.create_topic(topic_metadata) is called.

@asymingt asymingt added the enhancement New feature or request label Jul 13, 2023
@asymingt
Copy link
Contributor Author

asymingt commented Jul 13, 2023

I was able to work out the serialization scheme by poking around an existing working bag, and I found this:

>>> bag_file = os.path.join(get_package_prefix('my_test'), 'bags', 'example.bag')
>>> storage_options, converter_options = get_rosbag_options(bag_file, 'sqlite3')
>>> reader = rosbag2_py.SequentialReader()
>>> reader.open(storage_options, converter_options)
[INFO] [1689268155.189086900] [rosbag2_storage]: Opened database for READ_ONLY.
>>> tm = reader.get_all_topics_and_types()
>>> tm[0]
>>> tm[0].offered_qos_profiles
'- history: 3\n  depth: 0\n  reliability: 1\n  durability: 2\n  deadline:\n    sec: 9223372036\n    nsec: 854775807\n  lifespan:\n    sec: 9223372036\n    nsec: 854775807\n  liveliness: 1\n  liveliness_lease_duration:\n    sec: 9223372036\n    nsec: 854775807\n  avoid_ros_namespace_conventions: false'

Looking deeper, it seems like something of this form should work:

qos_profile = QoSProfile(depth=10,
    reliability=QoSReliabilityPolicy.RELIABLE,
    durability=QoSDurabilityPolicy.TRANSIENT_LOCAL)
qos_profile_dict = yaml.dump([qos_profile.get_c_qos_profile().to_dict()])

... but it doesn't quite handle the Duration types correctly and you get this:

TypeError: cannot pickle 'rclpy._rclpy_pybind11.rcl_duration_t' object

@emersonknapp
Copy link
Collaborator

Offered QoS profiles is, agreed, a pretty awkward part of this data structure. It was implemented that way as a workaround for some circular dependencies, long ago. It could use some rework - I even opened #1197 but there just hasn't been time to finish it.

The source for how the YAML is serialized/deserialized is in https://github.com/ros2/rosbag2/blob/rolling/rosbag2_transport/src/rosbag2_transport/qos.cpp - hopefully that can give some more insight into the structure. Any PR to the README would be welcome for review.

@MichaelOrlov
Copy link
Contributor

@asymingt We recently made structural changes and cleanup in the scope of the #1476 and now we are using std::vector<rclcpp::QoS> offered_qos_profiles; in the TopicMetadata instead of the previous serialized version of qos_profiles.
Now it is pretty clear and obvious what parameters should correspond to what value in this data structure since now it is a rclcpp::QoS structure and pretty much corresponds to the dedicated section in the readme file Overriding QoS Profiles

Feel free to reopen if you think more changes are needed.

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