[iron] Add ROS_DISTRO metadata record to mcap file when opening for writing

[emersonknapp]

Related to #1239
This is an API/ABI-stable patch to add some of the functionality from #1241 to Iron

Adds ROS_DISTRO metadata record to mcap storage file when opened for writing. ros2 bag info will not know about this information, but its presence in the file will be readable by other mcap-reading tools

When complete - will backport to Humble

[MichaelOrlov]

@emersonknapp In the future I would like to add functionality for saving full serialized metadata and it would be more reasonable to add it at the same place as ROS_DISTRO.
Wouldn't you mind considering moving functionality by saving ROS_DISTRO in the update_metada(metadata) method as it was made originally on Rolling?

And simple unit tests for getting saved ROS_DISTRO as in SQLite3 backend on Iron with get_ros_distro() API would be much appreciated.

[emersonknapp]

If we do update_metadata - that means we can create multiple metadata records in the output mcap. I think SequentialWriter calls update_metadata twice in normal operation. Maybe that's fine - if so do we just consider the latest Metadata record named "rosbag2" to be the correct values?

[emersonknapp]

Update - by using the flag I only write this particular metadata value once - so it's fine. I added a unit test but since MCAPStorage headers are not exported I just used mcap library directly to inspect the bag for the test.

[emersonknapp]

Pulls: #1371
Gist: https://gist.githubusercontent.com/emersonknapp/4bde126f5e1dfaf0c5fb9699d7d2ffe4/raw/00461a95d0a7f269e7fdc3d234551779b3fea7c5/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_storage_mcap
TEST args: --packages-above rosbag2_storage_mcap
ROS Distro: iron
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/12168

• Linux
• Linux-aarch64
• Windows

[MichaelOrlov]

@emersonknapp As regards

I think SequentialWriter calls update_metadata twice in normal operation. Maybe that's fine - if so do we just consider the latest Metadata record named "rosbag2" to be the correct values?

Yes, this is correct I imply we do similar things as on Rolling. i.e. Look out for the first record of the Metadata record named "rosbag2" and take ROS_DISTRO value if it is non-empty. If it is empty try to lookout for the next Metadata record.

[emersonknapp]

Rebuilds build #12168

• Linux
• Linux-aarch64
• Windows

[emersonknapp]

I expect that the Windows error is a totally unrelated flaky test (which has had attempts to make it reliable in #1012 and #1290 - but I don't think either of them actually fixed it). Rerunning to check, and not block this PR on that issue

Windows (just build-up-to rosbag2_cpp and test that package)

[MichaelOrlov]

@emersonknapp As regards to the test failure in the rosbag2_cpp.TimeControllerClockTest.unpaused_sleep_returns_true on Windows CI.
Take a look on my PR #1367 and more interesting a failed rpr job https://build.ros2.org/job/Rpr__rosbag2__ubuntu_jammy_amd64/849/testReport/junit/rosbag2_cpp/TimeControllerClockTest/unpaused_sleep_returns_true/ with those changes.

It seems the root cause of the issue is that rclcpp::ok() became false somewhere in the middle of the test.
But how it could be?
cc: @clalancette

[emersonknapp]

Note #1384 fixing the test - I'll hold off on merging this one until that is backported to Iron so we can have good happy green CI

[emersonknapp]

OK - just waiting on #1389 to merge, then will rebase and CI

[emersonknapp]

Pulls: #1371
Gist: https://gist.githubusercontent.com/emersonknapp/bf5714f5aed1a9461b42b0c74dff1116/raw/00461a95d0a7f269e7fdc3d234551779b3fea7c5/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_storage_mcap
TEST args: --packages-above rosbag2_storage_mcap
ROS Distro: iron
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/12213

• Linux
• Linux-aarch64
• Windows

[emersonknapp]

@Mergifyio backport humble

[mergify]

backport humble

✅ Backports have been created

• #1393 [iron] Add ROS_DISTRO metadata record to mcap file when opening for writing (backport #1371) has been created for branch humble but encountered conflicts