[Epic] Rosbag2 playback time control and simulation time

[emersonknapp]

Description

Tracking issue for the group of features (planned for Galactic) around control of the flow of time during rosbag2 playback. See "Completion Criteria" for full feature list

Design

• Initial Design: Design for rosbag2 handling of clock/simtime #675
• Extension with time conversion logic Add time conversion logic diagrams to time control design #700
• Extension with play_next Add play_next() API to design document #708

Feature List / Completion Criteria

Design at https://github.com/ros2/rosbag2/blob/master/docs/design/rosbag2_playback_time.md is fully implemented including all of the following features (names tagged where items are known to be work-in-progress):

• PlayerClock integration into Player (@emersonknapp)
  • (bugfix/unblock) Fix incorrect filter test: Fix and clarify logic in test_play filter test #690
  • Initial PlayerClock implementation: PlayerClock initial implementation - Player functionally unchanged #689
  • Thread safety annotation followup Enable thread safety analysis for rosbag2_cpp and add annotations in TimeControllerClock #710
• pause/resume time during playback Pause/resume ros2 bag play with keyboard controls #55 (@emersonknapp)
  • impl: Pause/resume PlayerClock #704
  • fix behavior Fix pause snapshot behavior and add regression test #730
  • add error checking Enforce non-null now_fn in TimeControllerClock #731
• change rate during playback (@emersonknapp )
  • impl: Add set_rate to PlayerClock #727
• jump/seek to arbitrary time during playback (@MichaelOrlov) Add jump/seek API for the Player class #821
  • naive clock jump implementation for simpler setup Naive clock jump implementation - allows for clock reuse and simplified Player setup #754 (@emersonknapp)
  • jump callbacks Add callbacks for PlayerClock::jump(time_point) API #775 (@MichaelOrlov)
  • Add jump/seek API for the Player class #821
  • storage seek API Seek/Jump API for Reader/Storage  #824 (@lihui815)
    • added seek interface #836
    • Use Reader's seek() method for seeking/jumping in Player #873
• "Play next message" while paused (@MichaelOrlov) Add play_next() API to the player class #762
• pause/rate/play-next controllable by keyboard handler
  • Pause/resume ros2 bag play with keyboard controls #55 (@MichaelOrlov)
  • Design document for unified keyboard handler Design document for keyboard handler #768
  • Unified keyboard handler Unified keyboard handler #735 (@MichaelOrlov)
  • expose Rate to keyboard
• pause/rate/play-next/jump controllable by external caller via Service (@emersonknapp )
  • Add service definitions Add rosbag2_interfaces package with playback service definitions #728
  • Expose pause/resume services on the Player Expose pause/resume related services on the Player #729
  • Expose playback rate services on the Player Expose GetRate/SetRate services for playback #753
  • Expose play_next() API via service Expose play_next() API via service #767 (@MichaelOrlov)
  • Expose jump service on the Player Add a Seek service #874
• can publish to /clock during playback play - publish /clock topic #99 (@emersonknapp)
  • Migrate play to rosbag2_py to make it easier to add new playback options: Add rosbag2_py::Player::play to replace rosbag2_transport_python version #693
  • Clock Publisher: /clock publisher in Player #695
• can subscribe to /clock during playback (use-sim-time) "use-ros-time" - Drive rosbag2 playback via /clock time source #694 (@emersonknapp )
  • [WIP] SimTimeClock #752
  • Implement Clock::sleep_for and Clock::sleep_until rclcpp#1730
  • Add Clock::sleep_until method rclcpp#1748

Code cleanup / tech debt / rearchitecture

• Migrate record to rosbag2_py rosbag2_py pybind wrapper for "record" - remove rosbag2_transport_py #702
• Test flakiness fix Fix bad_function_call by replacing rclcpp::spin_some with SingleThreadedExecutor #705
• Fix rosbag2_py after rclpy moved rmw_qos_profile_t to be a py::class Fix rosbag2_py transport test for py capsule #707
• use rclcpp's GenericPub/Sub, remove from rosbag2 (followup to Add rclcpp_generic, a publisher and subscription for serialized messages rclcpp#1452) (@Karsten1987) use rclcpp::Node for generic pub/sub #714
• Split Rosbag2Transport into separate Player and Recorder classes Split Rosbag2Transport into Player and Recorder classes - first pass to enable further progress #721
• Player and Recorder as Nodes
  • Remove rosbag2_transport class #716
  • turn player into a node #723
  • turn recorder into a node #724
  • player owns the reader #725
  • cleanup cmakelists #726

[ros-discourse]

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/tooling-wg-breakout-session-rosbag2-playback-time-control/19711/1

[Karsten1987]

Once ros2/rclcpp#1452 is in, I'd take the refactoring of removing the rosbag2_node and replace it with the new rclcpp one. Does this sound ok or do I disturb other people's workflow with this?
I feel that the task list above might lead to quite a few conflicts as they massively overlap.

[emersonknapp]

I think that would be fine - I don't see it interfering very much with most of the existing line items. Most of the changes now should be very focused, and not be near the direct usage of GenericPublisher. I added it to the "Code cleanup" section

[Karsten1987]

Ok, I was just afraid that I would overlap with @MichaelOrlov's last task: "Split Rosbag2Transport into separate Player and Recorder classes"

[emersonknapp]

Yes, that one could overlap, it would probably be good to stage those serially.

[emersonknapp]

But - maybe it won't be too bad! Perhaps you can stage it as "replace rosbag2::generic with rclcpp::generic" - which definitely will have little to no conflict - then do "remove the rosbag2node" as a second stage

[MichaelOrlov]

@Karsten1987 Yeah I think it would overlap.
Please go ahead first. I can try to tackle another tasks for a while. Such as jump, play_next and keyboard handling.

[Kaju-Bubanja]

I don't see this awesome feature in the list of new features here: https://docs.ros.org/en/foxy/Releases/Release-Galactic-Geochelone.html#timeline-before-the-release Is it still planned to be released for galactic?

[emersonknapp]

Yes - everything that is currently checked off will be in Galactic (and maybe a couple more small things). None of the rosbag2 changes are listed on that page.

[clalancette]

None of the rosbag2 changes are listed on that page.

I hope you mean "yet" 😄 . It would be awesome to have a high-level overview of the (massive) changes to rosbag2 listed on that page.

[emersonknapp]

I hope you mean "yet" smile . It would be awesome to have a high-level overview of the (massive) changes to rosbag2 listed on that page.

Yes, yet. Should we open a PR against the page source for it?

[clalancette]

Yes, yet. Should we open a PR against the page source for it?

Yes, whenever you are ready, please feel free to open a PR and I'm happy to review it.

[emersonknapp]

Closing this issue, we don't need it anymore. It's a huge amount of work that we did! The only thing really remaining is use-sim-tim, which can be tracked via its own issue #694