Readers/info can accept a single bag storage file, and detect its storage id automatically #1072

emersonknapp · 2022-08-19T20:36:39Z

Closes #972

API-level allowance for bare bag files as input. Main improvement is the storage factory loop over all plugins, when storage id is not provided.

To expose this new capability consistently, factor out common options into a single place for all ros2 bag verbs that read bags.

… its storage id Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>

emersonknapp · 2022-08-19T21:45:10Z

Gist: https://gist.githubusercontent.com/emersonknapp/e68af89691e22dd924aa077fd66a298b/raw/a8c3181416c2d89e59b10df9f616ff8aee8203e3/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_storage rosbag2_cpp
TEST args: --packages-above rosbag2_storage rosbag2_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/10681

Linux
Linux-aarch64
Windows

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

emersonknapp · 2022-08-23T20:48:35Z

@MichaelOrlov any concerns here?

emersonknapp · 2022-08-23T21:03:14Z

Gist: https://gist.githubusercontent.com/emersonknapp/7d8a4e70a1192d9dc5bbcb4054ff1956/raw/b344e687fdbd8f0959e589b0a03b97a09532f812/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_storage rosbag2_cpp
TEST args: --packages-above rosbag2_storage rosbag2_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/10690

Linux
Linux-aarch64
Windows

MichaelOrlov

@emersonknapp Idea and implementation looks great!
However I have a few questions in the scope of the code review which I would like to clarify and minor nitpicks to fix.

One of the questions is: "If storage id not specified directly and URI pointing out to the db3 file directly. Will we fail to load metadata? Or what behavior will be in this case?"

ros2bag/ros2bag/verb/info.py

ros2bag/ros2bag/api/__init__.py

rosbag2_cpp/test/rosbag2_cpp/test_info.cpp

rosbag2_cpp/test/rosbag2_cpp/test_sequential_reader.cpp

rosbag2_storage/src/rosbag2_storage/impl/storage_factory_impl.hpp

… debug logging Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>

emersonknapp · 2022-08-26T18:02:49Z

OK @MichaelOrlov - i updated the tests and the storage factory logging - lmk what you think now.

MichaelOrlov

@emersonknapp Looks good now. Approving.
Please re-run CI one more time since it was some changes from last run.

emersonknapp · 2022-08-26T19:04:35Z

Gist: https://gist.githubusercontent.com/emersonknapp/2396fcd1ddd4175a8bfe93f8b6382b16/raw/b344e687fdbd8f0959e589b0a03b97a09532f812/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_storage rosbag2_cpp
TEST args: --packages-above rosbag2_storage rosbag2_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/10697

Linux
Linux-aarch64
Windows

emersonknapp · 2022-08-30T22:10:14Z

@Mergifyio backport humble

…rage id automatically (#1072) * allow Info and Reader to accept a single bag storage file, and detect its storage id Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix end to end test and reduce verbosity Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix MSBuild warning Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Reader CLIs can take single file consistently Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix info end to end test Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Cleanup sqlite3 hardcoding in tests, clean wording in storage factory debug logging Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> (cherry picked from commit 7e5bd14) # Conflicts: # ros2bag/ros2bag/verb/burst.py # ros2bag/ros2bag/verb/reindex.py # rosbag2_cpp/test/rosbag2_cpp/test_info.cpp

mergify · 2022-08-30T22:10:55Z

backport humble

✅ Backports have been created

#1077 Readers/info can accept a single bag storage file, and detect its storage id automatically (backport #1072) has been created for branch humble

…rage id automatically (#1072) * allow Info and Reader to accept a single bag storage file, and detect its storage id Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix end to end test and reduce verbosity Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix MSBuild warning Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Reader CLIs can take single file consistently Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix info end to end test Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Cleanup sqlite3 hardcoding in tests, clean wording in storage factory debug logging Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>

ros-discourse · 2022-09-20T00:46:30Z

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-2022-9-15/27394/1

…rage id automatically (#1072) (#1077) * allow Info and Reader to accept a single bag storage file, and detect its storage id Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix end to end test and reduce verbosity Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix MSBuild warning Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Reader CLIs can take single file consistently Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Fix info end to end test Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> * Cleanup sqlite3 hardcoding in tests, clean wording in storage factory debug logging Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com> Co-authored-by: Emerson Knapp <537409+emersonknapp@users.noreply.github.com>

ros-discourse · 2022-11-23T18:31:56Z

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

https://discourse.ros.org/t/psa-default-ros-2-bag-storage-format-is-changing-to-mcap-in-iron/28489/1

emersonknapp requested a review from a team as a code owner August 19, 2022 20:36

emersonknapp requested review from MichaelOrlov and hidmic and removed request for a team August 19, 2022 20:36

emersonknapp mentioned this pull request Aug 19, 2022

Reader CLI argument consistency pass (coherently enables bare-file) #1073

Closed

allow Info and Reader to accept a single bag storage file, and detect…

b81bfea

… its storage id Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>

emersonknapp force-pushed the emersonknapp/reader-allow-single-file branch from a941f17 to b81bfea Compare August 19, 2022 21:37

emersonknapp added 4 commits August 19, 2022 16:55

Fix end to end test and reduce verbosity

e6c38f9

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

Fix MSBuild warning

34478b7

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

Reader CLIs can take single file consistently

1083441

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

Fix info end to end test

48603f4

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

emersonknapp changed the title ~~allow Info and Reader to accept a single bag storage file, and detect its storage id~~ Readers/info can accept a single bag storage file, and detect its storage id automatically Aug 22, 2022

MichaelOrlov requested changes Aug 26, 2022

View reviewed changes

Cleanup sqlite3 hardcoding in tests, clean wording in storage factory…

a9b5f52

… debug logging Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>

MichaelOrlov approved these changes Aug 26, 2022

View reviewed changes

MichaelOrlov merged commit 7e5bd14 into rolling Aug 26, 2022

delete-merged-branch bot deleted the emersonknapp/reader-allow-single-file branch August 26, 2022 22:05

mergify bot mentioned this pull request Aug 30, 2022

[Humble] Readers/info can accept a single bag storage file, and detect its storage id automatically (backport #1072) #1077

Merged

MichaelOrlov mentioned this pull request Nov 15, 2022

Autodetect MCAP in rosbag2_cpp open #1158

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Readers/info can accept a single bag storage file, and detect its storage id automatically #1072

Readers/info can accept a single bag storage file, and detect its storage id automatically #1072

emersonknapp commented Aug 19, 2022 •

edited

Loading

emersonknapp commented Aug 19, 2022

emersonknapp commented Aug 23, 2022

emersonknapp commented Aug 23, 2022 •

edited

Loading

MichaelOrlov left a comment

emersonknapp commented Aug 26, 2022

MichaelOrlov left a comment

emersonknapp commented Aug 26, 2022

emersonknapp commented Aug 30, 2022

mergify bot commented Aug 30, 2022

ros-discourse commented Sep 20, 2022

ros-discourse commented Nov 23, 2022

Readers/info can accept a single bag storage file, and detect its storage id automatically #1072

Readers/info can accept a single bag storage file, and detect its storage id automatically #1072

Conversation

emersonknapp commented Aug 19, 2022 • edited Loading

emersonknapp commented Aug 19, 2022

emersonknapp commented Aug 23, 2022

emersonknapp commented Aug 23, 2022 • edited Loading

MichaelOrlov left a comment

Choose a reason for hiding this comment

emersonknapp commented Aug 26, 2022

MichaelOrlov left a comment

Choose a reason for hiding this comment

emersonknapp commented Aug 26, 2022

emersonknapp commented Aug 30, 2022

mergify bot commented Aug 30, 2022

✅ Backports have been created

ros-discourse commented Sep 20, 2022

ros-discourse commented Nov 23, 2022

emersonknapp commented Aug 19, 2022 •

edited

Loading

emersonknapp commented Aug 23, 2022 •

edited

Loading