-
Notifications
You must be signed in to change notification settings - Fork 248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recording - limited buffer with triggerable snapshot #663
Comments
Really cool feature 👍 |
Would like to have more details of the feature on this ticket itself. Putting to bottom of backlog for now. |
Hey, it feels like i can give a hand with implementing this feature. Would it be more suitable to open PR and new discussion there or continue designing solution in this thread? |
As with most features of any significant weight, I'd prefer a design proposal first before looking at an implementation (feels slower - but slow is smooth and smooth is fast). You could open up a PR with a new document in the folder https://github.com/ros2/rosbag2/tree/master/docs/design - then we could tie this feature request ticket to that design. There's room for flexibility, of course, if you think it's faster to throw up a sample implementation than to write a small doc - but generally I find it easier to review the doc, that way any disagreement over definitions or behaviors can be hashed out before a lot of time is lost on making it work well enough for a proof-of-concept. Luckily, we're getting to a place where it should be a lot easier to implement something like this. The In a design, I would be looking for specifics on expected user inputs / disk outputs. It doesn't have to focus too heavily on the exact implementation details, but pseudocode API or diagrams with an idea of how they tie to existing structures would help make the review go smoothly. Feel free to join the Tooling WG meetings if you want to have open discussion. |
@d-walsh Sorry for resurrecting this zombie thread but please see https://github.com/gaia-platform/rosbag2_snapshot. |
I'll also note that this feature now exists in the core - which you can observe in the above merged PRs. I'd love to see any diff between that feature and your package contributed to the core, so that you don't have to maintain a custom wrapper. |
@emersonknapp I am apparently blind but thanks for pointing this out! Is there documentation for usage? |
You know what - that made me realize there isn't! I'll look into adding some now. You can see the option in A more comprehensive documentation page is definitely needed but that's a little ways out, I think. |
Thanks! I'll give it a look and see how it compares. |
I have tested the snapshot feature that was merged into the main branch and it looks like it is working great. However it lacks the option to provide the duration of the snapshot instead of the buffer size, which seems to be supported by @JWhitleyWork's implementation. We will test that implementation in the following days, it is also great that it is in a separate package, which removes the need to build and run a rolling distribution to get this feature |
@CourchesneA Thanks for the info! The timestamps were not correct on my node but that is being fixed with gaia-platform/rosbag2_snapshot#5 (please feel free to test/approve if you have time). We will also be adding the ability to trigger snapshots which run into the future (stop time > now()) very soon. Let us know if you have any issues! |
Description
The snapshot capability was added to the ROS1 version of rosbag and it would be great to support this use case with ROS2. https://github.com/ros/rosbag_snapshot
Feature high level:
This allows for capturing data around events of interest without having to store the data for an entire recording duration, saving on disk usage for resource limited use case.
Implementation Suggestions
Testing Suggestions
The text was updated successfully, but these errors were encountered: