Skip to content
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

Snapshot based on duration #900

Open
CourchesneA opened this issue Nov 1, 2021 · 10 comments · May be fixed by #1844
Open

Snapshot based on duration #900

CourchesneA opened this issue Nov 1, 2021 · 10 comments · May be fixed by #1844
Labels
enhancement New feature or request

Comments

@CourchesneA
Copy link

Description

There was a recent addition of the snapshot feature that allows a circular buffer of fixed size. In our case, it is not possible to specify a buffer zise, but we would like to specify a duration. It would be great to be able to trigger a snapshot for X seconds in the past and Y seconds in the future.

Related Issues

#663

Completion Criteria

  • Able to trigger a snapshot based on time instead of memory

Testing Notes / Suggestions

Extra goals would be support for multiple overlapping snapshots

@CourchesneA CourchesneA added the enhancement New feature or request label Nov 1, 2021
@AntoineHX
Copy link

Are their plans to integrate this to Foxy-future ?
I believe https://github.com/gaia-platform/rosbag2_snapshot, now, has this feature. But it seems only compatible with Galactic or above.

@MichaelOrlov
Copy link
Contributor

@AntoineHX There are no plans or scheduled effort for backporting rosbag2_snapshot feature to the https://github.com/ros2/rosbag2/tree/foxy-future branch.
As always PRs are welcome.

@Stefan-working
Copy link

Are there any plans to implement this enhancement to the rosbag2?
If I am reading the doc and source of the https://github.com/gaia-platform/rosbag2_snapshot correctly this still misses the feature to also further record Y seconds into the future and rosbag2_snapshot seems to have no license.
I really would appreciate it if this feature would be part of the rosbag2 core, as we really like to use this feature for recording only specific scenarios and use cases.

@CourchesneA
Copy link
Author

@Stefan-working In the end we ended up using the GAIA's implementation (which I believe is not unmaintained). We were able to implement "record Y seconds into the future" manually with a simple sleep(Y).

@MichaelOrlov
Copy link
Contributor

@Stefan-working
Copy link

Thank you @CourchesneA and @MichaelOrlov with this license we may be able to use it and adapt it with the sleep.

@tonynajjar
Copy link

Reviving this issue, this would be quite useful to me as well!

@MichaelOrlov
Copy link
Contributor

Sorry, don't have capacity for this feature for a while. It is supposed to be structural changes and not straightforward to implement.

@tonynajjar
Copy link

Sorry, don't have capacity for this feature for a while. It is supposed to be structural changes and not straightforward to implement.

It's the issue with the most "thumbs up" though, or how is work prioritized in this repo?

Screenshot_20240326_083922_GitHub.jpg

@MichaelOrlov
Copy link
Contributor

Priority is assigned according to the importance and feasibility.
Priority is low because there exists a workaround (one can estimate data stream size and specify buffer size in bytes) and also exists a third-party solution. Another reason why the priority is low is because potential implementation is not straightforward and requires structural changes in the core of the rosbag2 on multiple layers.

I personally was planning to get back to this issue/feature somewhere in the June-July when dust after the Jazzy release will settle down.

@MichaelOrlov MichaelOrlov linked a pull request Oct 28, 2024 that will close this issue
@MichaelOrlov MichaelOrlov linked a pull request Oct 28, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants