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

[wip] Bugfix for bag_split event callbacks called to early with file compression #1643

Draft
wants to merge 3 commits into
base: rolling
Choose a base branch
from

Conversation

MichaelOrlov
Copy link
Contributor

@MichaelOrlov MichaelOrlov commented May 8, 2024

  • Bugfix for bag_split event callbacks calling to early when using file compression option for rosbag2_cpp::Writer. i.e. callbacks calling when the bag file closes. However, the expected behavior is that callbacks are called when compression is finished and the original bag file is deleted.
  • Done with fix. However, missing regression tests.

@MichaelOrlov MichaelOrlov changed the title Bugfix for bag_split event callbacks not called with file compression [wip] Bugfix for bag_split event callbacks not called with file compression May 8, 2024
@MichaelOrlov MichaelOrlov changed the title [wip] Bugfix for bag_split event callbacks not called with file compression [wip] Bugfix for bag_split event callbacks called to early with file compression May 8, 2024
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
- It is a non-virtual method and doesn't call from the base class.

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov force-pushed the morlov/bugfix-for-bag_split_callbacks-with-compression branch from 843ca1d to 1d6249c Compare May 9, 2024 21:18
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Copy link
Contributor Author

@r7vme
Copy link
Contributor

r7vme commented May 22, 2024

@MichaelOrlov do we have a reproducer for the problem that is fixed here?

@MichaelOrlov
Copy link
Contributor Author

@r7vme Nop, that is the point. We need a regression test coverage to make sure that fix works as expected and that no one will brake it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants