Decoding RecordOptions from YAML conflicts with chrono::milliseconds

[JoLichtenfeld]

Description

Currently, for parsing a yaml node to RecordOptions, the key "topic_polling_interval" of type std::chrono::milliseconds throws an error. Only the key "milliseconds" can be parsed to std::chrono::milliseconds.

Expected Behavior

I try to parse a yaml node node to rosbag2_transport::RecordOptions using this approach:
ROSBAG2_TRANSPORT_PUBLIC YAML::convert<rosbag2_transport::RecordOptions>::decode(node, record_options);
The yaml node looks like this:

...
topics: []
topic_polling_interval: 100
topic_types: []
...

Actual Behavior

This line called from here (record_options.cpp) throws the following error:

terminate called after throwing an instance of 'YAML::BadSubscript'
what(): yaml-cpp: error at line 29, column 25: operator[] call on a scalar (key: "milliseconds")

The method expects the key "milliseconds". If I change the yaml node to

...
topics: []
topic_polling_interval: 
  milliseconds: 100
topic_types: []
...

it works. But I don't see a reason why the additional key should be necessary.

System

• OS: Ubuntu 24.04
• ROS 2 Distro: jazzy

[MichaelOrlov]

@r7vme could you please make a preliminary analysis for this issue? It could be a side effect from recently merged fix for duration timepoint conversion.

[r7vme]

@MichaelOrlov I think related code was added in this PR #1419 . Do you know why we need node["milliseconds"] instead of just using the node?

  static Node encode(const std::chrono::milliseconds & duration)
  {
    Node node;
    node["milliseconds"] = duration.count();
    return node;
  }

can be

  static Node encode(const std::chrono::milliseconds & duration)
  {
    return Node(duration.count());
  }

[MichaelOrlov]

Likely because the similar issue as in the #1744.
I've fixed converter in one place for timestamps but maybe we need more. Need to dig deeper or at least try to reproduce on latest rolling.

[MichaelOrlov]

Ahh, no. Please disregard my previous comment.
Actually it looks like a WAD. Works As Designed. Topics interval explicitly in milliseconds to be clear that it is in milliseconds. And I believe it was made on purpose this way to avoid confusion in what measurement it is.

[JoLichtenfeld]

Alright, if it was intended this way then it's fine.

[MichaelOrlov]

• Closing as WAD (Works As Designed)