Default rmw implemented serialization converter fails to deserialize messages with rmw_fastrtps

MichaelOrlov commented 1 month ago

Description

When implementing a custom serialization converter plugin and trying to use it during recording, the Rosbag2 recorder fails to deserialize messages from the CDR serialization format.

Expected Behavior

It should be possible to use the Rosbag2 recorder with serialization converter plugins, and the default rmw implemented serialization converter shall not fail during serialization or deserialization operations.

Actual Behavior

ros2 bag record -f s --all fails with error messages like

[INFO] [1726883791.924227547] [rosbag2_recorder]: Press SPACE for pausing/resuming
[INFO] [1726883791.924463727] [rosbag2_cpp]: No plugin found providing serialization format 'cdr'. Falling back to checking RMW implementations.
[WARN] [1726883791.926102035] [rosbag2_cpp]: Environment variable ROS_DISTRO not set, can't store value in bag metadata.
[INFO] [1726883791.929565146] [rosbag2_recorder]: Listening for topics...
[INFO] [1726883791.929595375] [rosbag2_recorder]: Event publisher thread: Starting
[INFO] [1726883791.934202847] [rosbag2_recorder]: Subscribed to topic '/rosout'
[INFO] [1726883791.935778981] [rosbag2_recorder]: Subscribed to topic '/events/write_split'
[INFO] [1726883791.935840261] [rosbag2_recorder]: Recording...
[ERROR] [1726883791.936079666] [rosbag2_cpp]: Failed to deserialize message for conversion.
[ERROR] [1726883791.936148588] [rosbag2_cpp]: Failed to deserialize message for conversion.
[ERROR] [1726883791.936163193] [rosbag2_cpp]: Failed to deserialize message for conversion.
[ERROR] [1726883791.936173710] [rosbag2_cpp]: Failed to deserialize message for conversion.
[ERROR] [1726883791.936185274] [rosbag2_cpp]: Failed to deserialize message for conversion.
[ERROR] [1726883791.936198334] [rosbag2_cpp]: Failed to deserialize message for conversion.

To Reproduce

Issue reproduces with converter_test_plugin from the rosbag2_cpp unit tests https://github.com/ros2/rosbag2/blob/rolling/rosbag2_cpp/test/rosbag2_cpp/converter_test_plugin.xml
Build code up to the rosbag2 with the command colcon build --packages-up-to rosbag2
source ./install/setup.bash
Run the command ros2 bag record -f s --all. Where -f s CLI option to use s_converter from the converter_test_plugin
Logs print error '[ERROR] [1726883791.936079666] [rosbag2_cpp]: Failed to deserialize message for conversion.'

System (please complete the following information)

OS: Ubuntu 22.04
ROS 2 Distro: Iron, Rolling
Install Method: Build from sources
Version: Rolling 0.29.0

Additional context

This issue reproduces only with the default rmw_fastrtps.

MichaelOrlov commented 1 month ago

RCA (Root cause Analysis)

In the Rosbag2 recorder if rmw_serialization_format from the RecordOptions is different from the currently using rmw_get_serialization_format() we are trying to create a serialization converter https://github.com/ros2/rosbag2/blob/fd8dd26f840c1634534607364a5dc71d41cf6265/rosbag2_transport/src/rosbag2_transport/recorder.cpp#L270-L272
In the SerializationFormatConverterFactoryImpl we are trying to find and load serialization plugins for deserializing from the rmw_get_serialization_format() (in our case CDR) and serializing to the rmw_serialization_format specified in the the RecordOptions.
When we specify -f s option the SerializationFormatConverter will find converter_test_plugin from the rosbag2_cpp unit tests fro the serialization https://github.com/ros2/rosbag2/blob/fd8dd26f840c1634534607364a5dc71d41cf6265/rosbag2_cpp/test/rosbag2_cpp/converter_test_plugin.xml#L10-L15. However, for CDR there are no specific plugin registered in the system and SerializationFormatConverter will fallback to usage RMWImplementedConverter https://github.com/ros2/rosbag2/blob/fd8dd26f840c1634534607364a5dc71d41cf6265/rosbag2_cpp/src/rosbag2_cpp/serialization_format_converter_factory_impl.hpp#L131-L138. Which is using rmw_deserialize(..) function.
Converter we are trying to call input_converter_->deserialize(message, introspection_ts, allocated_ros_message); https://github.com/ros2/rosbag2/blob/fd8dd26f840c1634534607364a5dc71d41cf6265/rosbag2_cpp/src/rosbag2_cpp/converter.cpp#L75-L79 with introspection type support.
The issue is that rmw_fastrtps rmw_deserialize only support RMW_FASTRTPS_CPP_TYPESUPPORT_C and RMW_FASTRTPS_CPP_TYPESUPPORT_CPP and doesn;t support introspection type support https://github.com/ros2/rmw_fastrtps/blob/fc66051e598c49d0f01665e33ee63d3ae83b1e2d/rmw_fastrtps_cpp/src/rmw_serialize.cpp#L64-L79 However, other rmw dds implementations like rmw_cyclonedds and rmw_connextdds support both introspection and CPP type supports.

Proposal for the fix

The proposed fix will be in usage of the rosidl_typesupport_cpp in the rosbag2_cpp::converter::convert(..) method for calling serialization and deserialization functions. https://github.com/ros2/rosbag2/blob/fd8dd26f840c1634534607364a5dc71d41cf6265/rosbag2_cpp/src/rosbag2_cpp/converter.cpp#L75-L87 Please note that we already have rosidl_typesupport_cpp stored in the local member variable of the converter class. https://github.com/ros2/rosbag2/blob/fd8dd26f840c1634534607364a5dc71d41cf6265/rosbag2_cpp/src/rosbag2_cpp/converter.cpp#L96-L98

MichaelOrlov commented 1 month ago

Closing as fixed in the #1823

ros2 / rosbag2