ros2 / rosbag2

Apache License 2.0
285 stars 251 forks source link

[iron] Bugfix for rosbag2_cpp serialization converter (backport #1814) #1823

Closed mergify[bot] closed 1 month ago

mergify[bot] commented 1 month ago
mergify[bot] commented 1 month ago

Cherry-pick of 6e82f52f3917c365ce60f9ffd8f5248e25c0fe55 has failed:

On branch mergify/bp/iron/pr-1814
Your branch is up to date with 'origin/iron'.

You are currently cherry-picking commit 6e82f52f.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
    modified:   rosbag2_cpp/CMakeLists.txt
    modified:   rosbag2_cpp/include/rosbag2_cpp/converter_interfaces/serialization_format_converter.hpp
    modified:   rosbag2_cpp/include/rosbag2_cpp/converter_interfaces/serialization_format_deserializer.hpp
    modified:   rosbag2_cpp/include/rosbag2_cpp/converter_interfaces/serialization_format_serializer.hpp
    renamed:    rosbag2_cpp/src/rosbag2_cpp/rmw_implemented_serialization_format_converter.hpp -> rosbag2_cpp/include/rosbag2_cpp/rmw_implemented_serialization_format_converter.hpp
    modified:   rosbag2_cpp/package.xml
    modified:   rosbag2_cpp/src/rosbag2_cpp/rmw_implemented_serialization_format_converter.cpp
    modified:   rosbag2_cpp/src/rosbag2_cpp/serialization_format_converter_factory_impl.hpp
    new file:   rosbag2_cpp/test/rosbag2_cpp/test_serialization_converter.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
    both modified:   rosbag2_cpp/src/rosbag2_cpp/converter.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

MichaelOrlov commented 1 month ago

The changes in the PR are ABI/API compatible because we moved RMWImplementedConverter class declaration from the src folder to the include folder and made it publicaly available in the dll. This is equivalent to adding a new class to the dll. According to the https://acodersjourney.com/20-abi-breaking-changes/ - this is an ABI compatible changes.

MichaelOrlov commented 1 month ago

Pulls: ros2/rosbag2#1823 Gist: https://gist.githubusercontent.com/MichaelOrlov/26ced40d72ff85483c4f0e7fc9427dab/raw/d255e6babae5f6a9027a5beb3b1d2415e4d15413/ros2.repos BUILD args: --packages-above-and-dependencies rosbag2_cpp TEST args: --packages-above rosbag2_cpp ROS Distro: iron Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14616

MichaelOrlov commented 1 month ago

Pulls: ros2/rosbag2#1823 Gist: https://gist.githubusercontent.com/MichaelOrlov/889d3a9b53433b3f1f0d4d6dd29ef20c/raw/d255e6babae5f6a9027a5beb3b1d2415e4d15413/ros2.repos BUILD args: --packages-above-and-dependencies rosbag2_cpp TEST args: --packages-above rosbag2_cpp ROS Distro: iron Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14617

MichaelOrlov commented 1 month ago

Pulls: ros2/rosbag2#1823 Gist: https://gist.githubusercontent.com/MichaelOrlov/19b3219cfeebd91be9631b8c46933f5a/raw/d255e6babae5f6a9027a5beb3b1d2415e4d15413/ros2.repos BUILD args: --packages-above-and-dependencies rosbag2_cpp TEST args: --packages-above rosbag2_cpp ROS Distro: iron Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14619

MichaelOrlov commented 1 month ago

Warnings on RHEL build no CONNEXTDDS_DIR nor NDDSHOME specified is a known issue and unrelated to the changes from this PR.

MichaelOrlov commented 1 month ago

https://github.com/Mergifyio backport humble

mergify[bot] commented 1 month ago

backport humble

✅ Backports have been created

* [#1824 [iron] Bugfix for rosbag2_cpp serialization converter (backport #1814) (backport #1823)](https://github.com/ros2/rosbag2/pull/1824) has been created for branch `humble` but encountered conflicts