search

ros-visualization / rqt_bag

http://wiki.ros.org/rqt_bag
28 stars 52 forks source link

[ros2] Enable Save #142

Closed Yadunund closed 1 year ago

Yadunund commented 1 year ago

Merge #127 first

Merge #141 first

This PR

Tested as seen in the video below.

https://github.com/ros-visualization/rqt_bag/assets/13482049/36f9bb7c-6ed2-473c-ac4b-0e0c2652bc86

clalancette commented 1 year ago

Merge #127 first

Presumably you mean "merge #141 first", correct?

Yadunund commented 1 year ago

Merge #127 first

Presumably you mean "merge #141 first", correct?

Ah yes 😅

Yadunund commented 1 year ago

Whoops sorry for not explaining the approach in the PR description.

You're right, this PR implements 1). Looking at the code, the existing implementation is only expecting to save a specific region of a recorded bag that been loaded into the timeline. There are some internal APIs to extract this region but the frontend UI does not provide any means to select a region. So in this PR I updated the implementation to stop the recording (which saves the bag) when save is clicked.

I think 2) would not be possible without some refactoring. Currently we can toggle the record button but what that does is pause/unpausing the writing to the same bag. But during this toggle, the plugin is still in a "recording" state; but just that it doesn't write any data to the bag. So you can record for a bit, pause, resume recording, save bag. Presently the creation of bag requires the plugin to not be in "recording" state so some effort is needed to implement this.

Happy to modify this PR as per any recommendations.

clalancette commented 1 year ago

All right, thanks for the explanation. Even though I find that a somewhat confusing semantic, it is an improvement over "doesn't work at all" :). I'll leave one other minor request inline, then we can run CI on it.

clalancette commented 1 year ago

CI:

Yadunund commented 1 year ago

@mergifyio backport iron

mergify[bot] commented 1 year ago

backport iron

✅ Backports have been created

* [#144 [ros2] Enable Save (backport #142)](https://github.com/ros-visualization/rqt_bag/pull/144) has been created for branch `iron`