Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion rosbag2_cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ option(DISABLE_SANITIZERS "disables the use of gcc sanitizers" ON)
if(NOT DISABLE_SANITIZERS AND CMAKE_COMPILER_IS_GNUCXX)
include(CheckCXXSourceRuns)
set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
set(CMAKE_REQUIRED_FLAGS "${OLD_CMAKE_REQUIRED_FLAGS} -fsanitize=leak")
set(CMAKE_REQUIRED_FLAGS "${OLD_CMAKE_REQUIRED_FLAGS} -fsanitize=address,leak,undefined")
check_cxx_source_runs("int main() {}" HAVE_SANITIZERS)
set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
if(NOT HAVE_SANITIZERS)
Expand Down Expand Up @@ -133,8 +133,10 @@ ament_export_dependencies(
if(BUILD_TESTING)
find_package(ament_cmake_gmock REQUIRED)
find_package(ament_lint_auto REQUIRED)
find_package(std_msgs REQUIRED)
find_package(test_msgs REQUIRED)
find_package(rosbag2_test_msgdefs REQUIRED)
find_package(rmw_implementation_cmake REQUIRED)
ament_lint_auto_find_test_dependencies()

add_library(
Expand Down Expand Up @@ -243,6 +245,32 @@ if(BUILD_TESTING)
)
endif()

function(test_serialization_converter_for_rmw_implementation)
set(rmw_implementation_env_var RMW_IMPLEMENTATION=${rmw_implementation})
ament_add_gmock(test_serialization_converter${target_suffix}
test/rosbag2_cpp/test_serialization_converter.cpp
ENV ${rmw_implementation_env_var}
)
if(TARGET test_serialization_converter${target_suffix})
if(NOT DISABLE_SANITIZERS)
target_compile_options(test_serialization_converter${target_suffix}
PUBLIC $<$<CXX_COMPILER_ID:GNU>:-fsanitize=address,leak,undefined>
)
target_link_libraries(test_serialization_converter${target_suffix}
$<$<CXX_COMPILER_ID:GNU>:-fsanitize=address,leak,undefined>
)
endif()
target_link_libraries(test_serialization_converter${target_suffix}
${PROJECT_NAME}
rosbag2_storage::rosbag2_storage
${std_msgs_TARGETS}
)
endif()
endfunction()
# Run test_serialization_converter for each rmw implementation because default serialization
# converter uses rmw specific functions for serialization and deserialization.
call_for_each_rmw_implementation(test_serialization_converter_for_rmw_implementation)

ament_add_gmock(test_multifile_reader
test/rosbag2_cpp/test_multifile_reader.cpp)
if(TARGET test_multifile_reader)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "rosbag2_cpp/converter_interfaces/serialization_format_serializer.hpp"
#include "rosbag2_cpp/converter_interfaces/serialization_format_deserializer.hpp"
#include "rosbag2_cpp/visibility_control.hpp"

/**
* This is a convenience class for plugin developers.
Expand All @@ -31,7 +32,7 @@ namespace rosbag2_cpp
namespace converter_interfaces
{

class SerializationFormatConverter
class ROSBAG2_CPP_PUBLIC SerializationFormatConverter
: public SerializationFormatSerializer, public SerializationFormatDeserializer
{
public:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,16 @@
#include <string>

#include "rosbag2_cpp/types/introspection_message.hpp"

#include "rosbag2_storage/serialized_bag_message.hpp"

#include "rosidl_runtime_c/message_type_support_struct.h"
#include "rosbag2_cpp/visibility_control.hpp"

namespace rosbag2_cpp
{
namespace converter_interfaces
{

class SerializationFormatDeserializer
class ROSBAG2_CPP_PUBLIC SerializationFormatDeserializer
{
public:
virtual ~SerializationFormatDeserializer() = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,16 @@
#include <string>

#include "rosbag2_cpp/types/introspection_message.hpp"

#include "rosbag2_storage/serialized_bag_message.hpp"

#include "rosidl_runtime_c/message_type_support_struct.h"
#include "rosbag2_cpp/visibility_control.hpp"

namespace rosbag2_cpp
{
namespace converter_interfaces
{

class SerializationFormatSerializer
class ROSBAG2_CPP_PUBLIC SerializationFormatSerializer
{
public:
virtual ~SerializationFormatSerializer() = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@
#include <string>

#include "rosbag2_cpp/converter_interfaces/serialization_format_converter.hpp"
#include "rosbag2_cpp/visibility_control.hpp"

// This is necessary because of using stl types here. It is completely safe, because
// a) the member is not accessible from the outside
// b) there are no inline functions.
#ifdef _WIN32
# pragma warning(push)
# pragma warning(disable:4251)
#endif

namespace rosbag2_cpp
{
Expand All @@ -31,7 +40,7 @@ class RMWImplementedConverterImpl;
* searches the system for an installed RMW implementation that understands the requested format,
* loads that library if found, and uses its implementation for serialization.
*/
class RMWImplementedConverter
class ROSBAG2_CPP_PUBLIC RMWImplementedConverter
: public rosbag2_cpp::converter_interfaces::SerializationFormatConverter
{
public:
Expand All @@ -40,7 +49,7 @@ class RMWImplementedConverter
* \throws std::runtime_error if no RMW implementation was found supporting the format.
*/
explicit RMWImplementedConverter(const std::string & format);
virtual ~RMWImplementedConverter();
~RMWImplementedConverter() override;

void deserialize(
std::shared_ptr<const rosbag2_storage::SerializedBagMessage> serialized_message,
Expand Down
2 changes: 2 additions & 0 deletions rosbag2_cpp/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
<test_depend>ament_cmake_gmock</test_depend>
<test_depend>ament_lint_auto</test_depend>
<test_depend>ament_lint_common</test_depend>
<test_depend>rmw_implementation_cmake</test_depend>
<test_depend>std_msgs</test_depend>
<test_depend>test_msgs</test_depend>
<test_depend>rosbag2_test_common</test_depend>
<test_depend>rosbag2_test_msgdefs</test_depend>
Expand Down
9 changes: 5 additions & 4 deletions rosbag2_cpp/src/rosbag2_cpp/converter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,23 @@ std::shared_ptr<rosbag2_storage::SerializedBagMessage> Converter::convert(
std::shared_ptr<const rosbag2_storage::SerializedBagMessage> message)
{
auto introspection_ts = topics_and_types_.at(message->topic_name).introspection_type_support;
auto rmw_ts = topics_and_types_.at(message->topic_name).rmw_type_support;
auto allocator = rcutils_get_default_allocator();
std::shared_ptr<rosbag2_introspection_message_t> allocated_ros_message =
allocate_introspection_message(introspection_ts, &allocator);
auto output_message = std::make_shared<rosbag2_storage::SerializedBagMessage>();

// deserialize
rosbag2_cpp::introspection_message_set_topic_name(
allocated_ros_message.get(), message->topic_name.c_str());
allocated_ros_message->time_stamp = message->time_stamp;
input_converter_->deserialize(message, introspection_ts, allocated_ros_message);
input_converter_->deserialize(message, rmw_ts, allocated_ros_message);

// re-serialize
// re-serialize with the new serializer
auto output_message = std::make_shared<rosbag2_storage::SerializedBagMessage>();
output_message->serialized_data = rosbag2_storage::make_empty_serialized_message(0);
output_message->topic_name = std::string(allocated_ros_message->topic_name);
output_message->time_stamp = allocated_ros_message->time_stamp;
output_converter_->serialize(allocated_ros_message, introspection_ts, output_message);
output_converter_->serialize(allocated_ros_message, rmw_ts, output_message);
return output_message;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "rmw_implemented_serialization_format_converter.hpp"
#include "rosbag2_cpp/rmw_implemented_serialization_format_converter.hpp"

#include <memory>
#include <sstream>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#include "rosbag2_cpp/logging.hpp"
#include "rosbag2_cpp/visibility_control.hpp"

#include "./rmw_implemented_serialization_format_converter.hpp"
#include "rosbag2_cpp/rmw_implemented_serialization_format_converter.hpp"

namespace rosbag2_cpp
{
Expand Down
Loading
Loading