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

Expose bag_rewrite as ros2 bag convert #921

Merged
merged 4 commits into from
Dec 8, 2021
Merged

Expose bag_rewrite as ros2 bag convert #921

merged 4 commits into from
Dec 8, 2021

Conversation

emersonknapp
Copy link
Collaborator

@emersonknapp emersonknapp commented Nov 24, 2021 •
edited
Loading

Closes #831
Closes #163
Closes #668

Depends on #920

Exposes the bag_rewrite API to rosbag2_py and ros2bag for usage on the commandline.

@emersonknapp emersonknapp mentioned this pull request Nov 24, 2021
Merged
@emersonknapp emersonknapp force-pushed the emersonknapp/bag-rewriter branch 7 times, most recently from 277d612 to 7e2f01b Compare November 30, 2021 19:27
@emersonknapp emersonknapp force-pushed the emersonknapp/bag-rewriter branch 2 times, most recently from 2ce7cc9 to e9c33e7 Compare December 2, 2021 20:11
@emersonknapp emersonknapp marked this pull request as ready for review December 2, 2021 20:33
@emersonknapp emersonknapp requested a review from a team as a code owner December 2, 2021 20:33
@emersonknapp emersonknapp requested review from gbiggs and lihui815 and removed request for a team December 2, 2021 20:33
@emersonknapp emersonknapp changed the title Expose bag_rewrite to Python/CLI Expose bag_rewrite as ros2 bag convert Dec 2, 2021
@delete-merged-branch delete-merged-branch bot deleted the branch master December 3, 2021 00:06
@emersonknapp emersonknapp changed the base branch from emersonknapp/bag-rewriter to master December 3, 2021 00:10
@emersonknapp emersonknapp self-assigned this Dec 3, 2021
@emersonknapp
Copy link
Collaborator Author

Gist: https://gist.githubusercontent.com/emersonknapp/6523851ab0085fd489e8bccccd1b6ee2/raw/0252be16f2dd6ee5a6183d48dff623001c91e6bc/ros2.repos
BUILD args: --packages-up-to ros2bag rosbag2_py rosbag2_tests rosbag2
TEST args: --packages-select ros2bag rosbag2_py rosbag2_tests rosbag2
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/9440

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

@emersonknapp
Bag rewriter CLI and README
6ee730f 
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp
Better to use uri= constructor
48b2b24 
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp
Copy link
Collaborator Author

emersonknapp commented Dec 3, 2021 •
edited
Loading

Note to reviewers: should we call this verb convert, or is rewrite better? It felt, internal to the implementation that this is "rewriting", and that "conversion" was confused as a term.

  • "Convert" already applies a lot to serialization formats in the codebase
  • There was some discussion thinking of "converting" between ros distros - which this feature can do to some extent, but probably not if message definitions have changed
  • In ros1, there is a slightly similar feature called "filter", but that doesn't feel like it really captures this feature properly

@emersonknapp emersonknapp marked this pull request as draft December 3, 2021 21:03
lihui815
lihui815 approved these changes Dec 3, 2021
View reviewed changes
Copy link
Contributor

@lihui815 lihui815 left a comment

Choose a reason for hiding this comment

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

contingent on more comprehensive testing

@emersonknapp
add rosbag2_py test_convert basic testing
15e3f3c 
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp marked this pull request as ready for review December 7, 2021 23:04
@emersonknapp
Copy link
Collaborator Author

Gist: https://gist.githubusercontent.com/emersonknapp/c1cf8632ab27d7407bfa31d67e76e93c/raw/0252be16f2dd6ee5a6183d48dff623001c91e6bc/ros2.repos
BUILD args: --packages-up-to ros2bag rosbag2_py rosbag2_tests rosbag2
TEST args: --packages-select ros2bag rosbag2_py rosbag2_tests rosbag2
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/9464

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

@emersonknapp
Fix regex for windows
d6817a3 
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp
Copy link
Collaborator Author

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

@emersonknapp emersonknapp merged commit d6ffd91 into master Dec 8, 2021
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/bag-rewriter-cli branch December 8, 2021 01:55
@wep21 wep21 mentioned this pull request Dec 8, 2021
Closed
@Blast545 Blast545 mentioned this pull request Dec 8, 2021
Closed
@clalancette clalancette mentioned this pull request Apr 29, 2022
Closed
87 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lihui815 lihui815 lihui815 approved these changes

@gbiggs gbiggs Awaiting requested review from gbiggs gbiggs is a code owner automatically assigned from ros2/tooling-reviewers

Assignees

@emersonknapp emersonknapp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Generic Bag(s)->Bag(s) Rewriter Join or stitch together multiple rosbags ros2bag convert verb
2 participants
@emersonknapp @lihui815