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

Reader and writer can use default storage by not specifying #1167

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

emersonknapp
Copy link
Collaborator

@emersonknapp emersonknapp commented Nov 18, 2022

Don't hardcode anything for reader, as it can now autodetect.

Allow users to not specify a storage id for writer, in which case it will fall back to the default.

Closes #1158

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp requested a review from a team as a code owner November 18, 2022 02:02
@emersonknapp emersonknapp requested review from MichaelOrlov and jhdcs and removed request for a team November 18, 2022 02:03
Copy link
Contributor

@MichaelOrlov MichaelOrlov left a comment

Choose a reason for hiding this comment

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

LGTM.

@MichaelOrlov
Copy link
Contributor

Gist: https://gist.githubusercontent.com/MichaelOrlov/b998440130e586b54d4cce57a4c7b4e1/raw/95d5a9f996440de5bd1ab712a08fbde6aff9962b/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_tests rosbag2_transport ros2bag
TEST args: --packages-above rosbag2_cpp rosbag2_tests rosbag2_transport ros2bag
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11151

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

@MichaelOrlov
Copy link
Contributor

Re-run Windows build, since failure mimick_vendor and unrelated to the changes from this PR.

  • Windows Build Status

@clalancette
Copy link
Contributor

FYI, I kicked off the Windows build again (we were having some issues with CMake 3.25):

  • Windows Build Status

@emersonknapp emersonknapp merged commit 5c4dafe into rolling Nov 21, 2022
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/writer-default-storage branch November 21, 2022 22:19
@doisyg
Copy link

doisyg commented Nov 21, 2022

Nice, possible to backport to humble ?

@MichaelOrlov
Copy link
Contributor

https://github.com/Mergifyio backport humble

mergify bot pushed a commit that referenced this pull request Nov 22, 2022
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
(cherry picked from commit 5c4dafe)

# Conflicts:
#	rosbag2_cpp/src/rosbag2_cpp/writer.cpp
@mergify
Copy link

mergify bot commented Nov 22, 2022

backport humble

✅ Backports have been created

MichaelOrlov added a commit that referenced this pull request Nov 22, 2022
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
MichaelOrlov added a commit that referenced this pull request Nov 22, 2022
- Replaced `rosbag2_storage::get_default_storage_id()` with hardcoded
kDefaultStorageID = "sqlite3" ssince we don't have
`get_default_storage_id()` on `humble`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
MichaelOrlov added a commit that referenced this pull request Nov 23, 2022
…(backport #1167) (#1174)

* Reader and writer can use default storage by not specifying (#1167)

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
(cherry picked from commit 5c4dafe)

# Conflicts:
#	rosbag2_cpp/src/rosbag2_cpp/writer.cpp

* Fix merge conflicts after backporting pr #1167 from rolling

- Replaced `rosbag2_storage::get_default_storage_id()` with hardcoded
kDefaultStorageID = "sqlite3" ssince we don't have
`get_default_storage_id()` on `humble`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
Co-authored-by: Emerson Knapp <537409+emersonknapp@users.noreply.github.com>
Co-authored-by: Michael Orlov <michael.orlov@apex.ai>
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.

Autodetect MCAP in rosbag2_cpp open
5 participants