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

Fix playback of compressed bagfiles #417

Merged
merged 1 commit into from
May 26, 2020

Conversation

emersonknapp
Copy link
Collaborator

The SequentialCompressionReader was not filling in the data for get_all_topics_and_types since #372

Fixes #412

The newly added test fails before the change in SequentialCompressionReader, passes after

Signed-off-by: Emerson Knapp emerson.b.knapp@gmail.com

Copy link

@prajakta-gokhale prajakta-gokhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if this method needs to be part of the class or an interface in future, if it's needed in other implementations.

Copy link
Contributor

@dabonnie dabonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add method doc

@emersonknapp emersonknapp force-pushed the emersonknapp/compression-segfault branch from 6c60c0d to 61bae17 Compare May 21, 2020 00:21
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp force-pushed the emersonknapp/compression-segfault branch from 61bae17 to 231c4a9 Compare May 21, 2020 00:36
@emersonknapp
Copy link
Collaborator Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Contributor

@piraka9011 piraka9011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like rosbag2_transport tests failing on Mac OS and Windows.
Otherwise LGTM.

@emersonknapp
Copy link
Collaborator Author

  • rerun OSX: Build Status
  • rerun Win: Build Status

Just trying them, it seems like my change is unrelated - and the play tests are some of them known unstable, as per #381

@emersonknapp emersonknapp merged commit 2438e91 into master May 26, 2020
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/compression-segfault branch May 26, 2020 17:33
@emersonknapp
Copy link
Collaborator Author

The failing tests are the noted unstable tests, merging this change to unblock further stability work

pjreed pushed a commit to pjreed/rosbag2 that referenced this pull request May 28, 2020
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: P. J. Reed <preed@swri.org>
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.

Playing a bag recorded with file compression segfaults
5 participants