[humble] Backport close() to rosbag2_cpp::writer to humble

[bernatgaston]

Ported the close functionality from rolling to humble

[emersonknapp]

The code itself is really straightforward but I don't see a way that we can really backport this change. See, within a stable ROS distribution (Humble), we guarantee both API and ABI stability for libraries in the base. While this change does not break any existing APIs, it very much does change the ABI, requiring recompilation of all downstream projects using it to guarantee correct behavior.

The best way I can think of in Humble to get the desired behavior is to just use the WriterImpl directly. rosbag2_cpp::Writer is a convenience wrapper class that provides a couple useful helper functions but really provides no complex new behavior of its own. It's very unfortunate that Writer::writer_impl_ is private, because otherwise a trivial subclass could give you the new method.

[MichaelOrlov]

@emersonknapp Actually adding a new non-virtual method is not a breaking API ABI change according to the https://www.acodersjourney.com/20-abi-breaking-changes/

Although we might need to add it to the end of the existing methods.

[MichaelOrlov]

@bernatgaston Please fix DCO and uncrustify warnings.

[bernatgaston]

Hi, thanks.
I moved the code to the end of the existing methods as requested previously

Although we might need to add it to the end of the existing methods.

Also I fixed the DCA and other errors/warnings

[MichaelOrlov]

ABI/API compatibility report

API compatibility report between librosbag2_cpp.so (X) and librosbag2_cpp.so (Y) objects on x86_64

Binary
Compatibility Source
Compatibility

Test Info

---

Module Name	libTest
Version #1	X
Version #2	Y
Arch	x86_64
GCC Version	11.3.0
Subject	Binary Compatibility

Test Results

---

Total Header Files	148
Total Source Files	17
Total Objects	1
Total Symbols / Types	273 / 132
Compatibility	100%

Problem Summary

---

	Severity	Count
Added Symbols	-	1
Removed Symbols	High	0
Problems with Data Types	High	0
Medium	0
Low	0
Problems with Symbols	High	0
Medium	0
Low	0
Problems with Constants	Low	0

Added Symbols  1 

---

writer.hpp, librosbag2_cpp.so
namespace rosbag2_cpp
Writer::close ( )

_ZN11rosbag2_cpp6Writer5closeEv

to the top

Header Files  148 

---

__mbstate_t.h
aligned_buffer.h
alloc_traits.h
allocated_ptr.h
allocator.h
asserts.hpp
atomic
atomic_base.h
atomic_wide_counter.h
atomic_word.h
bag_events.hpp
bag_metadata.hpp
base_reader_interface.hpp
base_writer_interface.hpp
basic_string.h
basic_string.tcc
bitset
c++config.h
cache_buffer_interface.hpp
cache_consumer.hpp
char_traits.h
chrono
circular_message_cache.hpp
class_desc.hpp
class_loader.hpp
class_loader_base.hpp
class_loader_core.hpp
class_loader_imp.hpp
clock.hpp
concurrence.h
condition_variable
converter.hpp
converter_options.hpp
ctype_base.h
deque.tcc
emitterstyle.h
enable_special_members.h
exceptions.hpp
filesystem_helper.hpp
functional
functional_hash.h
gthr-default.h
hashtable.h
hashtable_policy.h
info.hpp
initializer_list
introspection_message.hpp
ios_base.h
list.tcc
locale_classes.h
locale_facets.h
mark.h
memory.h
message_cache.hpp
message_cache_buffer.hpp
message_cache_circular_buffer.hpp
message_cache_interface.hpp
message_type_support_struct.h
metadata_io.hpp
move.h
multi_library_class_loader.hpp
new_allocator.h
node.h
node_data.h
node_ref.h
player_clock.hpp
postypes.h
pthreadtypes.h
ptr.h
ptr_traits.h
ratio
reader.hpp
refwrap.h
regex.h
regex.tcc
regex_automaton.h
regex_automaton.tcc
regex_compiler.h
regex_compiler.tcc
regex_constants.h
regex_error.h
regex_executor.h
regex_executor.tcc
regex_scanner.h
regex_scanner.tcc
reindexer.hpp
ret_types.h
rmw_implemented_serialization_format_converter.hpp
sequential_reader.hpp
sequential_writer.hpp
serialization_format_converter.hpp
serialization_format_converter_factory.hpp
serialization_format_converter_factory_impl.hpp
serialization_format_converter_factory_interface.hpp
serialization_format_deserializer.hpp
serialization_format_serializer.hpp
serialized_bag_message.hpp
serialized_message.h
shared_ptr.h
shared_ptr_base.h
split.hpp
sstream
sstream.tcc
std_function.h
std_mutex.h
std_thread.h
stddef.h
stdexcept
stdint-intn.h
stdint-uintn.h
stdint.h
stl_algo.h
stl_construct.h
stl_deque.h
stl_function.h
stl_iterator.h
stl_iterator_base_types.h
stl_list.h
stl_map.h
stl_pair.h
stl_set.h
stl_stack.h
stl_tree.h
stl_vector.h
storage_factory_interface.hpp
storage_filter.hpp
storage_options.hpp
streambuf
streambuf.tcc
string_conversions.h
stringfwd.h
struct_mutex.h
thread-shared-types.h
time.h
time_controller_clock.hpp
topic_metadata.hpp
tuple
type.h
type_traits
types.h
typesupport_helpers.hpp
uint8_array.h
unique_ptr.h
unordered_map.h
unordered_set.h
utility
vector.tcc
writer.hpp

to the top

Source Files  17 

---

cache_consumer.cpp
circular_message_cache.cpp
converter.cpp
info.cpp
introspection_message.cpp
message_cache.cpp
message_cache_buffer.cpp
message_cache_circular_buffer.cpp
reader.cpp
reindexer.cpp
rmw_implemented_serialization_format_converter.cpp
sequential_reader.cpp
sequential_writer.cpp
serialization_format_converter_factory.cpp
time_controller_clock.cpp
typesupport_helpers.cpp
writer.cpp

to the top

Objects  1 

---

librosbag2_cpp.so

to the top

Test Info

---

Module Name	libTest
Version #1	X
Version #2	Y
Arch	x86_64
Subject	Source Compatibility

Test Results

---

Total Header Files	148
Total Source Files	17
Total Objects	1
Total Symbols / Types	856 / 252
Compatibility	100%

Problem Summary

---

	Severity	Count
Added Symbols	-	1
Removed Symbols	High	0
Problems with Data Types	High	0
Medium	0
Low	0
Problems with Symbols	High	0
Medium	0
Low	0
Problems with Constants	Low	0

Added Symbols  1 

---

writer.hpp
namespace rosbag2_cpp
Writer::close ( )

_ZN11rosbag2_cpp6Writer5closeEv

to the top

Header Files  148 

---

__mbstate_t.h
aligned_buffer.h
alloc_traits.h
allocated_ptr.h
allocator.h
asserts.hpp
atomic
atomic_base.h
atomic_wide_counter.h
atomic_word.h
bag_events.hpp
bag_metadata.hpp
base_reader_interface.hpp
base_writer_interface.hpp
basic_string.h
basic_string.tcc
bitset
c++config.h
cache_buffer_interface.hpp
cache_consumer.hpp
char_traits.h
chrono
circular_message_cache.hpp
class_desc.hpp
class_loader.hpp
class_loader_base.hpp
class_loader_core.hpp
class_loader_imp.hpp
clock.hpp
concurrence.h
condition_variable
converter.hpp
converter_options.hpp
ctype_base.h
deque.tcc
emitterstyle.h
enable_special_members.h
exceptions.hpp
filesystem_helper.hpp
functional
functional_hash.h
gthr-default.h
hashtable.h
hashtable_policy.h
info.hpp
initializer_list
introspection_message.hpp
ios_base.h
list.tcc
locale_classes.h
locale_facets.h
mark.h
memory.h
message_cache.hpp
message_cache_buffer.hpp
message_cache_circular_buffer.hpp
message_cache_interface.hpp
message_type_support_struct.h
metadata_io.hpp
move.h
multi_library_class_loader.hpp
new_allocator.h
node.h
node_data.h
node_ref.h
player_clock.hpp
postypes.h
pthreadtypes.h
ptr.h
ptr_traits.h
ratio
reader.hpp
refwrap.h
regex.h
regex.tcc
regex_automaton.h
regex_automaton.tcc
regex_compiler.h
regex_compiler.tcc
regex_constants.h
regex_error.h
regex_executor.h
regex_executor.tcc
regex_scanner.h
regex_scanner.tcc
reindexer.hpp
ret_types.h
rmw_implemented_serialization_format_converter.hpp
sequential_reader.hpp
sequential_writer.hpp
serialization_format_converter.hpp
serialization_format_converter_factory.hpp
serialization_format_converter_factory_impl.hpp
serialization_format_converter_factory_interface.hpp
serialization_format_deserializer.hpp
serialization_format_serializer.hpp
serialized_bag_message.hpp
serialized_message.h
shared_ptr.h
shared_ptr_base.h
split.hpp
sstream
sstream.tcc
std_function.h
std_mutex.h
std_thread.h
stddef.h
stdexcept
stdint-intn.h
stdint-uintn.h
stdint.h
stl_algo.h
stl_construct.h
stl_deque.h
stl_function.h
stl_iterator.h
stl_iterator_base_types.h
stl_list.h
stl_map.h
stl_pair.h
stl_set.h
stl_stack.h
stl_tree.h
stl_vector.h
storage_factory_interface.hpp
storage_filter.hpp
storage_options.hpp
streambuf
streambuf.tcc
string_conversions.h
stringfwd.h
struct_mutex.h
thread-shared-types.h
time.h
time_controller_clock.hpp
topic_metadata.hpp
tuple
type.h
type_traits
types.h
typesupport_helpers.hpp
uint8_array.h
unique_ptr.h
unordered_map.h
unordered_set.h
utility
vector.tcc
writer.hpp

to the top

Source Files  17 

---

cache_consumer.cpp
circular_message_cache.cpp
converter.cpp
info.cpp
introspection_message.cpp
message_cache.cpp
message_cache_buffer.cpp
message_cache_circular_buffer.cpp
reader.cpp
reindexer.cpp
rmw_implemented_serialization_format_converter.cpp
sequential_reader.cpp
sequential_writer.cpp
serialization_format_converter_factory.cpp
time_controller_clock.cpp
typesupport_helpers.cpp
writer.cpp

to the top

Objects  1 

---

librosbag2_cpp.so

to the top

---

_Generated by ABI Compliance Checker 2.3  _

[MichaelOrlov]

LGTM

[MichaelOrlov]

Gist: https://gist.githubusercontent.com/MichaelOrlov/331214daeefb3e46d9456335e295117d/raw/2a163a2ed0bb7d765da8944331210e3652461a74/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp
TEST args: --packages-above rosbag2_cpp
ROS Distro: humble
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/12148

• Linux
• Linux-aarch64
• Windows

[emersonknapp]

Actually adding a new non-virtual method is not a breaking API ABI change according to the https://www.acodersjourney.com/20-abi-breaking-changes/

Thanks for the info! It looks like I've been overly conservative with API changes, maybe because the ABI-checker is so unreliable. It looks like the ABI checker is not enabled for anything in Humble or Iron - that's unfortunate because I don't trust us to be 100% reliable at maintaining that guarantee as human reviewers

[MichaelOrlov]

@emersonknapp I agree that we need to add ABI-Checker to the PR job on Humble and Iron. But last time on Foxy it was causing errors that ABI checker was not able to pull in dependencies for the mcap-src which is downloading in via CMake file.

As regards to this particular PR I have spent some hefty time and ran ABI Checker manually on my local machine see my report here #1363 (comment)

[emersonknapp]

I mean that there are no packages at all that use it in Humble, maybe it was disabled entirely. Perhaps it would be worth trying to make a GitHub Action in the action-ros-ci/action-ros-lint suite to try and run the ABI checker.