Readers/info can accept a single bag storage file, and detect its storage id automatically #1072
Conversation
emersonknapp
requested review from
MichaelOrlov and
hidmic
and removed request for
a team
… its storage id Signed-off-by: Emerson Knapp <[email protected]>
emersonknapp
force-pushed
the
emersonknapp/reader-allow-single-file
branch
from
a941f17 to
b81bfea
Compare
|
Gist: https://gist.githubusercontent.com/emersonknapp/e68af89691e22dd924aa077fd66a298b/raw/a8c3181416c2d89e59b10df9f616ff8aee8203e3/ros2.repos
|
Signed-off-by: Emerson Knapp <[email protected]>
Signed-off-by: Emerson Knapp <[email protected]>
Signed-off-by: Emerson Knapp <[email protected]>
Signed-off-by: Emerson Knapp <[email protected]>
emersonknapp
changed the title
|
@MichaelOrlov any concerns here? |
|
Gist: https://gist.githubusercontent.com/emersonknapp/7d8a4e70a1192d9dc5bbcb4054ff1956/raw/b344e687fdbd8f0959e589b0a03b97a09532f812/ros2.repos
|
There was a problem hiding this comment.
@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?"
rosbag2_storage/src/rosbag2_storage/impl/storage_factory_impl.hpp
Outdated
Show resolved
Hide resolved
rosbag2_storage/src/rosbag2_storage/impl/storage_factory_impl.hpp
Outdated
Show resolved
Hide resolved
… debug logging Signed-off-by: Emerson Knapp <[email protected]>
|
OK @MichaelOrlov - i updated the tests and the storage factory logging - lmk what you think now. |
There was a problem hiding this comment.
@emersonknapp Looks good now. Approving.
Please re-run CI one more time since it was some changes from last run.
|
Gist: https://gist.githubusercontent.com/emersonknapp/2396fcd1ddd4175a8bfe93f8b6382b16/raw/b344e687fdbd8f0959e589b0a03b97a09532f812/ros2.repos
|
delete-merged-branch
bot
deleted the
emersonknapp/reader-allow-single-file
branch
|
@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 <[email protected]> * Fix end to end test and reduce verbosity Signed-off-by: Emerson Knapp <[email protected]> * Fix MSBuild warning Signed-off-by: Emerson Knapp <[email protected]> * Reader CLIs can take single file consistently Signed-off-by: Emerson Knapp <[email protected]> * Fix info end to end test Signed-off-by: Emerson Knapp <[email protected]> * Cleanup sqlite3 hardcoding in tests, clean wording in storage factory debug logging Signed-off-by: Emerson Knapp <[email protected]> Signed-off-by: Emerson Knapp <[email protected]> (cherry picked from commit 7e5bd14) # Conflicts: # ros2bag/ros2bag/verb/burst.py # ros2bag/ros2bag/verb/reindex.py # rosbag2_cpp/test/rosbag2_cpp/test_info.cpp
✅ Backports have been created
|
…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 <[email protected]> * Fix end to end test and reduce verbosity Signed-off-by: Emerson Knapp <[email protected]> * Fix MSBuild warning Signed-off-by: Emerson Knapp <[email protected]> * Reader CLIs can take single file consistently Signed-off-by: Emerson Knapp <[email protected]> * Fix info end to end test Signed-off-by: Emerson Knapp <[email protected]> * Cleanup sqlite3 hardcoding in tests, clean wording in storage factory debug logging Signed-off-by: Emerson Knapp <[email protected]> Signed-off-by: Emerson Knapp <[email protected]>
…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 <[email protected]> * Fix end to end test and reduce verbosity Signed-off-by: Emerson Knapp <[email protected]> * Fix MSBuild warning Signed-off-by: Emerson Knapp <[email protected]> * Reader CLIs can take single file consistently Signed-off-by: Emerson Knapp <[email protected]> * Fix info end to end test Signed-off-by: Emerson Knapp <[email protected]> * Cleanup sqlite3 hardcoding in tests, clean wording in storage factory debug logging Signed-off-by: Emerson Knapp <[email protected]> Signed-off-by: Emerson Knapp <[email protected]>
|
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 <[email protected]> * Fix end to end test and reduce verbosity Signed-off-by: Emerson Knapp <[email protected]> * Fix MSBuild warning Signed-off-by: Emerson Knapp <[email protected]> * Reader CLIs can take single file consistently Signed-off-by: Emerson Knapp <[email protected]> * Fix info end to end test Signed-off-by: Emerson Knapp <[email protected]> * Cleanup sqlite3 hardcoding in tests, clean wording in storage factory debug logging Signed-off-by: Emerson Knapp <[email protected]> Signed-off-by: Emerson Knapp <[email protected]> Signed-off-by: Emerson Knapp <[email protected]> Co-authored-by: Emerson Knapp <[email protected]>
|
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 |
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 bagverbs that read bags.