[humble] Bugfix for rosbag2_cpp serialization converter (backport #1814) (backport #1823) #1824
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![mergify[bot]](https://avatars.githubusercontent.com/in/10562?s=60&v=4){kind=small}
#1823) * Bugfix for rosbag2_cpp serialization converter (#1814) * Bugfix for rosbag2 serialization converter - Use rmw specific type support for rmw_serilize{deserialize} function calls. Note: It is ok for CycloneDDS to use introspection type support for rmw_serilize{deserialize} functions. However, for FastRTPS it must be rmw specific type support. e.g. rosidl_typesupport_cpp. Fix works for both CycloneDDS and FastRTPS rmw. Signed-off-by: Michael Orlov <[email protected]> * Add test coverage for default rmv serialization format converter Signed-off-by: Michael Orlov <[email protected]> * Run test_serialization_converter for each rmw implementation - Rationale: To make sure that the default serialization converter can serialize and deserialize messages with all supported rmw implementations. Since it uses rmw specific functions for serialization and deserialization inside. Signed-off-by: Michael Orlov <[email protected]> * Address uncrustify formating warnings Signed-off-by: Michael Orlov <[email protected]> * Enable sanitizer by default Signed-off-by: Michael Orlov <[email protected]> * Address Windows build warnings Signed-off-by: Michael Orlov <[email protected]> * Revert "Enable sanitizer by default" This reverts commit 7241963. Signed-off-by: Michael Orlov <[email protected]> --------- Signed-off-by: Michael Orlov <[email protected]> (cherry picked from commit 6e82f52) # Conflicts: # rosbag2_cpp/src/rosbag2_cpp/converter.cpp * Address merge conflicts Signed-off-by: Michael Orlov <[email protected]> * Fix uncrustfy warning due to the different versions of uncrustify Signed-off-by: Michael Orlov <[email protected]> * Replace ament_add_gmock_test with ament_add_gmock - Rationale: The ament_add_gmock_executable(..) and ament_add_gmock_test(..) macros are not available on Iron distro. Signed-off-by: Michael Orlov <[email protected]> * Add missing dependencies from the std_msgs Signed-off-by: Michael Orlov <[email protected]> * Adjust writer_->create_topic(..) call in tests - removed TopicId field initialization since it is absent on Iron Signed-off-by: Michael Orlov <[email protected]> --------- Signed-off-by: Michael Orlov <[email protected]> Co-authored-by: Michael Orlov <[email protected]> (cherry picked from commit a269cd4) # Conflicts: # rosbag2_cpp/CMakeLists.txt
mergify
bot
requested review from
MichaelOrlov and
hidmic
and removed request for
a team
|
Cherry-pick of a269cd4 has failed: 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
changed the title
Signed-off-by: Michael Orlov <[email protected]>
Signed-off-by: Michael Orlov <[email protected]>
MichaelOrlov
approved these changes
Sep 27, 2024
|
Pulls: #1824
|
- Use rcpputils::fs instead of the std::filesystem in tests. Since RHEL8 doesn't have adequate support for the std::filesystem. Signed-off-by: Michael Orlov <[email protected]>
|
Re-run CI after an attempt to fix RHEL8 linker errors by switching to the
|
|
Warnings in RHEL 8 build with messages |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will address default rmw-implemented serialization converter failures when deserializing messages from the CDR format with rmw_fastrtps.
This PR addresses the issue described in the Default rmw implemented serialization converter fails to deserialize messages with rmw_fastrtps #1815
RCA and fix proposal please see in the Default rmw implemented serialization converter fails to deserialize messages with rmw_fastrtps #1815 (comment)
The fix uses rmw-specific type support for rmw_serilize{deserialize} function calls instead of the introspection type support.
Note: It is ok for CycloneDDS and ConnextDDS to use introspection type support for rmw_serilize{deserialize} functions. However, for FastRTPS it must be rmw specific type support. e.g.
rosidl_typesupport_cpp.The fix should work for CycloneDDS, ConnextDDS and FastRTPS RMWs.
This is an automatic backport of pull request #1814 done by [Mergify](https://mergify.com).
This is an automatic backport of pull request #1823 done by [Mergify](https://mergify.com).