[jazzy] Bugfix for rosbag2_cpp serialization converter (backport #1814) #1822
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}
* 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)
mergify
bot
requested review from
gbiggs and
james-rms
and removed request for
a team
MichaelOrlov
changed the title
MichaelOrlov
requested review from
MichaelOrlov
and removed request for
gbiggs
MichaelOrlov
approved these changes
Sep 26, 2024
There was a problem hiding this comment.
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 publicly 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.
|
Pulls: #1822
|
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).