Skip to content

Fix PlayOptions YAML round trips - #2453

Open
Old-Ding wants to merge 2 commits into
ros2:rollingfrom
Old-Ding:codex/fix-play-options-yaml-roundtrip
Open

Fix PlayOptions YAML round trips#2453
Old-Ding wants to merge 2 commits into
ros2:rollingfrom
Old-Ding:codex/fix-play-options-yaml-roundtrip

Conversation

@Old-Ding

@Old-Ding Old-Ding commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • decode the existing actions_to_filter and exclude_actions YAML keys
  • decode the existing loop and topic_remapping_options YAML keys
  • encode negative rclcpp::Duration values with the standard ROS sec/nanosec representation
  • add text YAML round-trip coverage for every field emitted by the PlayOptions encoder and the default negative acknowledgement timeout

Testing

  • verified the Rolling baseline has 27 encoded keys but only 23 matching decoded keys
  • verified the updated converter has identical 27-key encode/decode sets
  • verified the regression test assigns and checks a non-default value for all 27 encoded fields
  • verified the default wait_acked_timeout = -1ns encodes as sec: -1, nsec: 999999999 and round-trips
  • ament_copyright on the new test
  • UTF-8 without BOM and CRLF checks
  • added-line 100-column check
  • git diff --check

The repository-native C++ tests were not run locally because this Windows environment does not have the ROS 2 build dependencies or colcon. Repository CI covers the new round-trip tests.

Old-Ding added 2 commits July 11, 2026 14:47
The encoder writes action filters, loop, and remapping options, but the decoder ignored those keys. Decode every existing encoded key and add complete text YAML round-trip coverage for the current converter contract.

Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
Use rclcpp's standard Duration message conversion so negative nanoseconds are normalized to a non-negative nanosecond field. This keeps the default PlayOptions acknowledgement timeout round-trippable through text YAML.

Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant