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.

Sign up for GitHub

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

Jump to bottom

Add per-message ZSTD compression #418

Merged
merged 12 commits into from
Jul 13, 2020
Merged

Add per-message ZSTD compression #418

merged 12 commits into from
Jul 13, 2020

Conversation

pjreed
Copy link
Contributor

@pjreed pjreed commented May 22, 2020 •
edited
Loading

Fixes #414.

This implements the per-messages compression and decompression
functions for the ZSTD compressor and also adds unit tests
for them.

Distro A, OPSEC #2893

Signed-off-by: P. J. Reed preed@swri.org

@pjreed
Add per-message ZSTD compression
9f93b32 
This implements the per-messages compression and decompression
functions for the ZSTD compressor and also adds unit tests
for them.

Distro A, OPSEC #2893

Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Fix cpplint issues
60e0105 
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Re-use helper method from rosbag2_storage
17a6b61 
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Fill in topics & types in SequentialCompressionReader
404aab5 
This was causing playback of bags with per-message encryption to fail;
the player was not creating publishers because the topics had not been
filled in.

Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Remove unnecessary whitespace changes
a4d45dd 
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Fix lint issues
5288fc5 
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Revert "Don't allow user to specify unimplemented compression mode 'm…
a255b20 
…essage' (ros2#415)"

This reverts commit 75c8126.

Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Add note on rcutils_uint8_array_resize usage
e91de52 
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Fix uncrustify issue
881883e 
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Copy link
Contributor Author

pjreed commented Jun 2, 2020

This is ready to go now. The build_and_test PR check lists two failed unit tests in rosbag2_transport, but I don't believe those are related to my changes.

@pjreed pjreed marked this pull request as ready for review June 2, 2020 19:07
@Karsten1987 Karsten1987 added this to In progress in Galactic via automation Jun 3, 2020
emersonknapp
emersonknapp reviewed Jul 10, 2020
View reviewed changes
Copy link
Collaborator

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

This looks good to me other than the duplicated function. Sorry for the slow review

piraka9011
piraka9011 approved these changes Jul 10, 2020
View reviewed changes
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.

Given that a follow up PR for consolidating utilities is incoming and pending green CI, this would be fine by me,

Galactic automation moved this from In progress to Reviewer approved Jul 10, 2020
@piraka9011
Copy link
Contributor

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

@pjreed
Move test value into constant
c51e090 
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed @piraka9011
Update rosbag2_compression/test/rosbag2_compression/test_zstd_compres…
ab2ff8a 
…sor.cpp

Co-authored-by: Anas Abou Allaban <aabouallaban@protonmail.com>
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Fix spacing again
a86fbc7 
Signed-off-by: P. J. Reed <preed@swri.org>
@pjreed
Copy link
Contributor Author

pjreed commented Jul 13, 2020

#459 should handle consolidating those utility functions.

@pjreed
Copy link
Contributor Author

pjreed commented Jul 13, 2020

Ok, this should be all ready now.

@emersonknapp emersonknapp mentioned this pull request Jul 13, 2020
Merged
@piraka9011
Copy link
Contributor

piraka9011 commented Jul 13, 2020 •
edited
Loading

One more round of CI

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

@piraka9011 piraka9011 merged commit 9856f5b into ros2:master Jul 13, 2020
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2020-07-16/15468/1

@emersonknapp emersonknapp mentioned this pull request Aug 17, 2020
Merged
@clalancette clalancette moved this from Reviewer approved to Done in Galactic Jan 12, 2021
emersonknapp pushed a commit that referenced this pull request Feb 2, 2021
@pjreed
Add per-message ZSTD compression (#418)
16a6ff4 
* Add per-message ZSTD compression

This implements the per-messages compression and decompression
functions for the ZSTD compressor and also adds unit tests
for them.

Distro A, OPSEC #2893

Signed-off-by: P. J. Reed <preed@swri.org>
emersonknapp pushed a commit that referenced this pull request Feb 17, 2021
@pjreed
Add per-message ZSTD compression (#418)
1d39a5e 
* Add per-message ZSTD compression

This implements the per-messages compression and decompression
functions for the ZSTD compressor and also adds unit tests
for them.

Distro A, OPSEC #2893

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
Reviewers

@emersonknapp emersonknapp emersonknapp left review comments

@piraka9011 piraka9011 piraka9011 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Galactic
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Compress by message
4 participants
@pjreed @piraka9011 @ros-discourse @emersonknapp