Skip to content

Commit

Permalink
Fix outdated names and implementation details in design proposal doc
Browse files Browse the repository at this point in the history
Signed-off-by: Cameron Miller <[email protected]>
  • Loading branch information
Cameron Miller committed Aug 17, 2021
1 parent 272485c commit 088bb72
Showing 1 changed file with 9 additions and 7 deletions.
16 changes: 9 additions & 7 deletions docs/design/rosbag2_snapshot_mode.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ $ ros2 bag record --max-cache-size 100000 --snapshot-mode [topics [topics ...]]

Triggering a snapshot via CLI:
```
$ ros2 service call ~/snapshot rosbag2_interfaces/TakeSnapshot
$ ros2 service call /rosbag2_recorder/snapshot rosbag2_interfaces/TakeSnapshot
```

Triggering a snapshot with a keyboard shortcut:
Expand All @@ -29,22 +29,24 @@ Triggering a snapshot with a keyboard shortcut:

* Add a `--snapshot-mode` flag to the `record` verb in `ros2bag` that will be passed via `StorageOptions`.

* Create a new class `CircularBufferMessageCache` which maintains two circular buffers sized according to the existing `—max-cache-size` parameter. During snapshot mode, this class will keep adding messages to the same circular buffer until triggered to switch to the secondary buffer by the `Recorder` class.
* Create a new class `CircularMessageCache` which maintains two circular buffers, individally implemented in a class `MessageCacheCircularBuffer` and sized according to the existing `—max-cache-size` parameter. During snapshot mode, `CircularMessageCache` will keep adding messages to the same circular buffer until triggered to switch to the secondary buffer by the `Recorder` class.

* Create a service `~/snapshot` in the `rosbag2_recorder` node that will listen for snapshot requests. Upon receiving a service call, it will trigger a buffer flip in `CircularBufferMessageCache` and call the new `take_snapeshot` function in `SequentialWriter` (detailed below) to write the snapshot messages to storage.
* Create a service `~/snapshot` in the `rosbag2_recorder` node that will listen for snapshot requests. Upon receiving a service call, it will trigger a buffer flip in `CircularMessageCache` and call the new `take_snapeshot` function in `SequentialWriter` (detailed below) to write the snapshot messages to storage.

* Modify `SequentialWriter` to use `CircularBufferMessageCache` when snapshot mode is enabled instead of `MessageCache` and `CacheConsumer`.
* Modify `SequentialWriter` to use `CircularMessageCache` when snapshot mode is enabled instead of `MessageCache` and `CacheConsumer`.

* Add a new `take_snapshot` function to `SequentialWriter` that will be invoked by the `rosbag2_recorder` node's snapshot service to write the snapshot data to the rosbag, close the rosbag, and open a new rosbag for a future snapshot.
* Add a new `take_snapshot` function to `SequentialWriter` that will be invoked by the `rosbag2_recorder` node's snapshot service to write the snapshot data to the rosbag.

* Modify `BaseWriterInterface` to include a pure virutal function `take_snapshot` to ensure that all future writers are compatible with snapshot mode.

* Implement the `~/snapshot` service interface as `TakeSnapshot.srv` in `rosbag2_interfaces`. It won’t require any arguments (similar to `Resume.srv`).


## Implementation Breakdown
The snapshot feature implementation could be broken down into the following PRs:

* Implement `CircularBufferMessageCache` and add corresponding tests.
* Integrate `CircularBufferMessageCache` into `SequentialWriter` and update its tests.
* Implement `CircularMessageCache` and `MessageCacheCircularBuffer`. Then add corresponding tests.
* Integrate `CircularMessageCache` into `SequentialWriter` and update its tests.
* Create the `~/snapshot` service in the `Recorder` class, add corresponding tests, and create the `TakeSnapshot.srv` service interface.
* Add `--snapshot-mode` to the `record` verb and update `StorageOptions`
* Add keyboard shortcut for triggering a snapshot in snapshot mode

0 comments on commit 088bb72

Please sign in to comment.