From d3362a48ba70cf5e8aade07ada9672644dd1ae1f Mon Sep 17 00:00:00 2001 From: Michael Orlov Date: Mon, 23 Sep 2024 06:37:30 -0700 Subject: [PATCH 1/2] Allow unknown types in bag rewrite (#1812) Signed-off-by: Michael Orlov (cherry picked from commit cd7bd63696604973e23c739afa6387556f3e7781) # Conflicts: # rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp --- .../local_message_definition_source.cpp | 14 ++++++++++++++ .../test_local_message_definition_source.cpp | 15 +++++++++++++++ .../src/rosbag2_transport/bag_rewrite.cpp | 2 +- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp b/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp index 74e04f9f01..e2d913ca32 100644 --- a/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp +++ b/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp @@ -24,6 +24,7 @@ #include #include +#include #include "rosbag2_cpp/logging.hpp" @@ -165,8 +166,21 @@ const LocalMessageDefinitionSource::MessageSpec & LocalMessageDefinitionSource:: throw TypenameNotUnderstoodError(topic_type); } std::string package = match[1]; +<<<<<<< HEAD std::string share_dir = ament_index_cpp::get_package_share_directory(package); std::ifstream file{share_dir + "/msg/" + match[2].str() + +======= + std::string share_dir; + try { + share_dir = ament_index_cpp::get_package_share_directory(package); + } catch (const ament_index_cpp::PackageNotFoundError & e) { + ROSBAG2_CPP_LOG_WARN("'%s'", e.what()); + throw DefinitionNotFoundError(definition_identifier.topic_type()); + } + std::string dir = definition_identifier.format() == Format::MSG || + definition_identifier.format() == Format::IDL ? "/msg/" : "/srv/"; + std::ifstream file{share_dir + dir + match[2].str() + +>>>>>>> cd7bd636 (Allow unknown types in bag rewrite (#1812)) extension_for_format(definition_identifier.format())}; if (!file.good()) { throw DefinitionNotFoundError(definition_identifier.topic_type()); diff --git a/rosbag2_cpp/test/rosbag2_cpp/test_local_message_definition_source.cpp b/rosbag2_cpp/test/rosbag2_cpp/test_local_message_definition_source.cpp index 7e7d09ae1d..6e1ab63790 100644 --- a/rosbag2_cpp/test/rosbag2_cpp/test_local_message_definition_source.cpp +++ b/rosbag2_cpp/test/rosbag2_cpp/test_local_message_definition_source.cpp @@ -136,3 +136,18 @@ TEST(test_local_message_definition_source, no_crash_on_bad_name) }); ASSERT_EQ(result.encoding, "unknown"); } + +TEST(test_local_message_definition_source, throw_definition_not_found_for_unknown_msg) +{ + LocalMessageDefinitionSource source; + ASSERT_THROW( + { + source.get_full_text("rosbag2_test_msgdefs/msg/UnknownMessage"); + }, rosbag2_cpp::DefinitionNotFoundError); + + // Throw DefinitionNotFoundError for not found message definition package name + ASSERT_THROW( + { + source.get_full_text("not_found_msgdefs_pkg/msg/UnknownMessage"); + }, rosbag2_cpp::DefinitionNotFoundError); +} diff --git a/rosbag2_transport/src/rosbag2_transport/bag_rewrite.cpp b/rosbag2_transport/src/rosbag2_transport/bag_rewrite.cpp index ebf47f1b87..6c9fa19c5a 100644 --- a/rosbag2_transport/src/rosbag2_transport/bag_rewrite.cpp +++ b/rosbag2_transport/src/rosbag2_transport/bag_rewrite.cpp @@ -111,7 +111,7 @@ setup_topic_filtering( } for (const auto & [writer, record_options] : output_bags) { - rosbag2_transport::TopicFilter topic_filter{record_options}; + rosbag2_transport::TopicFilter topic_filter{record_options, nullptr, true}; auto filtered_topics_and_types = topic_filter.filter_topics(input_topics); // Done filtering - set up writer From b13b33fdcac16790d46d26c52c31a48e39b074b6 Mon Sep 17 00:00:00 2001 From: Michael Orlov Date: Mon, 23 Sep 2024 19:52:47 -0700 Subject: [PATCH 2/2] Address merge conflicts during backporting Signed-off-by: Michael Orlov --- .../local_message_definition_source.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp b/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp index e2d913ca32..85e66b1e46 100644 --- a/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp +++ b/rosbag2_cpp/src/rosbag2_cpp/message_definitions/local_message_definition_source.cpp @@ -166,10 +166,6 @@ const LocalMessageDefinitionSource::MessageSpec & LocalMessageDefinitionSource:: throw TypenameNotUnderstoodError(topic_type); } std::string package = match[1]; -<<<<<<< HEAD - std::string share_dir = ament_index_cpp::get_package_share_directory(package); - std::ifstream file{share_dir + "/msg/" + match[2].str() + -======= std::string share_dir; try { share_dir = ament_index_cpp::get_package_share_directory(package); @@ -177,10 +173,7 @@ const LocalMessageDefinitionSource::MessageSpec & LocalMessageDefinitionSource:: ROSBAG2_CPP_LOG_WARN("'%s'", e.what()); throw DefinitionNotFoundError(definition_identifier.topic_type()); } - std::string dir = definition_identifier.format() == Format::MSG || - definition_identifier.format() == Format::IDL ? "/msg/" : "/srv/"; - std::ifstream file{share_dir + dir + match[2].str() + ->>>>>>> cd7bd636 (Allow unknown types in bag rewrite (#1812)) + std::ifstream file{share_dir + "/msg/" + match[2].str() + extension_for_format(definition_identifier.format())}; if (!file.good()) { throw DefinitionNotFoundError(definition_identifier.topic_type());