Fixes writer not being able to open again after closing

[yschulz]

Fixes #1598 After discussion in #1590 I verified the issue in and reapplied the changes and comments to rolling. To reiterate, the storage_factory needs to persist beyond the call to close a current bag. On the other hand, all containers that were built during the recording have to be cleared before opening another bag.

[yschulz]

I keep it as a draft until @MichaelOrlov and @fujitatomoya can verify what all can be attached to a writers lifetime and what needs to be reset. I am especially not sure about the compressed writer, since I haven't spend enough time looking at that one. Thank you guys for your comments and review, I guess we can close the other PR?

[fujitatomoya]

@yschulz so this replaces https://github.com/ros2/rosbag2/pull/1590, right? in that case, you can close https://github.com/ros2/rosbag2/pull/1590, and let us know this one is ready to review.

[yschulz]

@fujitatomoya done, and yes please :)

I had a closer look at the compression writer and as to my impression, there is no need to clear anything regarding the compression functionality. However, I saw that the closing of all the storage functionality can be directly delegated to the base class (as it is for opening and writing anyway). Also, spotted an unused member, maybe a relict from before the threaded approach, you may know more.

[yschulz]

Thanks for the review and addressing the changes directly.

[MichaelOrlov]

@yschulz Sorry for taking a long time to review. It was a rush time before Jazzy code freeze. And changes per this PR were non-trivial for review. Was need to verify that we will not break anything.

[MichaelOrlov]

Gist: https://gist.githubusercontent.com/MichaelOrlov/2677c3394586488cd9a777215205eff9/raw/4e54566b952d8218af61493f02a1e01394e567bf/ros2.repos BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_compression rosbag2_tests TEST args: --packages-above rosbag2_cpp rosbag2_compression rosbag2_tests ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/13815

• Linux
• Linux-aarch64
• Windows

[fujitatomoya]

@MichaelOrlov thanks for checking this, i will take a look at this today.

[MichaelOrlov]

Re-run Windows CI It seems the previous failure is unrelated to the changes from this PR.

C:\ci\ws\src\yschulz\rosbag2\rosbag2_storage\include\rosbag2_storage/yaml.hpp(28,10): fatal error C1083: Cannot open include file: 'yaml-cpp/yaml.h': No such file or directory (compiling source file C:\ci\ws\src\yschulz\rosbag2\rosbag2_storage\src\rosbag2_storage\metadata_io.cpp) [C:\ci\ws\build\rosbag2_storage\rosbag2_storage.vcxproj]

• Windows

[MichaelOrlov]

@clalancette Out of curiosity if this is a known issue

C:\ci\ws\src\yschulz\rosbag2\rosbag2_storage\include\rosbag2_storage/yaml.hpp(28,10): fatal error C1083: Cannot open include file: 'yaml-cpp/yaml.h': No such file or directory (compiling source file C:\ci\ws\src\yschulz\rosbag2\rosbag2_storage\src\rosbag2_storage\metadata_io.cpp) [C:\ci\ws\build\rosbag2_storage\rosbag2_storage.vcxproj]

Or we missing something like find_package(yaml-cpp REQUIRED) in CMake file?

[clalancette]

@clalancette Out of curiosity if this is a known issue

No, not a known issue. This succeeded on the nightlies: https://ci.ros2.org/view/nightly/job/nightly_win_rel/3037/ .

My best guess is that this PR needs to be rebased onto the latest rosbag2 code to get #1605

[MichaelOrlov]

Re-run Windows CI after rebasing on the latest Roling

• Windows

[MichaelOrlov]

Ok. CI is green after rebase. Merging then.

[MichaelOrlov]

https://github.com/Mergifyio backport jazzy

[mergify[bot]]

backport jazzy

✅ Backports have been created

• #1639 Bugfix for writer not being able to open again after closing (backport #1599) has been created for branch jazzy