From db896393f8d086f44b4c75e184faa7ec4a4ed064 Mon Sep 17 00:00:00 2001
From: Emerson Knapp <537409+emersonknapp@users.noreply.github.com>
Date: Thu, 28 Oct 2021 17:04:06 -0700
Subject: [PATCH] Fix converter plugin choices for record (#897)

* Fix converter plugin choices. 1 - provide Converter class as an option for record. 2 - fix naming for CLI choices due to _converter suffix logic

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
---
 ros2bag/ros2bag/verb/record.py                            | 6 ++++++
 .../serialization_format_converter.hpp                    | 5 +++++
 .../serialization_format_converter_factory_impl.hpp       | 6 +++---
 rosbag2_py/src/rosbag2_py/_writer.cpp                     | 8 ++++++--
 rosbag2_py/test/test_sequential_writer.py                 | 5 ++++-
 5 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/ros2bag/ros2bag/verb/record.py b/ros2bag/ros2bag/verb/record.py
index 628879d0e4..115230b2e1 100644
--- a/ros2bag/ros2bag/verb/record.py
+++ b/ros2bag/ros2bag/verb/record.py
@@ -39,6 +39,12 @@ def add_arguments(self, parser, cli_name):  # noqa: D102
 
         compression_format_choices = get_registered_compressors()
         serialization_choices = get_registered_serializers()
+        converter_suffix = '_converter'
+        serialization_choices = {
+            f[:-len(converter_suffix)]
+            for f in serialization_choices
+            if f.endswith(converter_suffix)
+        }
 
         parser.add_argument(
             '-a', '--all', action='store_true',
diff --git a/rosbag2_cpp/include/rosbag2_cpp/converter_interfaces/serialization_format_converter.hpp b/rosbag2_cpp/include/rosbag2_cpp/converter_interfaces/serialization_format_converter.hpp
index 4d4c0c596b..151c491702 100644
--- a/rosbag2_cpp/include/rosbag2_cpp/converter_interfaces/serialization_format_converter.hpp
+++ b/rosbag2_cpp/include/rosbag2_cpp/converter_interfaces/serialization_format_converter.hpp
@@ -35,6 +35,11 @@ class SerializationFormatConverter
   : public SerializationFormatSerializer, public SerializationFormatDeserializer
 {
 public:
+  static std::string get_package_name()
+  {
+    return "rosbag2_cpp";
+  }
+
   static std::string get_base_class_name()
   {
     return "rosbag2_cpp::converter_interfaces::SerializationFormatConverter";
diff --git a/rosbag2_cpp/src/rosbag2_cpp/serialization_format_converter_factory_impl.hpp b/rosbag2_cpp/src/rosbag2_cpp/serialization_format_converter_factory_impl.hpp
index b08bf18984..31901cec77 100644
--- a/rosbag2_cpp/src/rosbag2_cpp/serialization_format_converter_factory_impl.hpp
+++ b/rosbag2_cpp/src/rosbag2_cpp/serialization_format_converter_factory_impl.hpp
@@ -128,9 +128,9 @@ class SerializationFormatConverterFactoryImpl
       }
     }
 
-    ROSBAG2_CPP_LOG_INFO(
-      "No plugin found providing serialization format. "
-      "Falling back to checking RMW implementations.");
+    ROSBAG2_CPP_LOG_INFO_STREAM(
+      "No plugin found providing serialization format '" << format << "'. " <<
+        "Falling back to checking RMW implementations.");
     try {
       return std::make_unique<RMWImplementedConverter>(format);
     } catch (const std::runtime_error & ex) {
diff --git a/rosbag2_py/src/rosbag2_py/_writer.cpp b/rosbag2_py/src/rosbag2_py/_writer.cpp
index 650b9b1ab6..2683f6002e 100644
--- a/rosbag2_py/src/rosbag2_py/_writer.cpp
+++ b/rosbag2_py/src/rosbag2_py/_writer.cpp
@@ -94,8 +94,12 @@ std::unordered_set<std::string> get_registered_compressors()
 
 std::unordered_set<std::string> get_registered_serializers()
 {
-  return rosbag2_cpp::plugins::get_class_plugins
-         <rosbag2_cpp::converter_interfaces::SerializationFormatSerializer>();
+  auto serializers = rosbag2_cpp::plugins::get_class_plugins<
+    rosbag2_cpp::converter_interfaces::SerializationFormatSerializer>();
+  auto converters = rosbag2_cpp::plugins::get_class_plugins<
+    rosbag2_cpp::converter_interfaces::SerializationFormatConverter>();
+  serializers.insert(converters.begin(), converters.end());
+  return serializers;
 }
 
 }  // namespace rosbag2_py
diff --git a/rosbag2_py/test/test_sequential_writer.py b/rosbag2_py/test/test_sequential_writer.py
index 3e05d64948..3ad7fcc167 100644
--- a/rosbag2_py/test/test_sequential_writer.py
+++ b/rosbag2_py/test/test_sequential_writer.py
@@ -118,4 +118,7 @@ def test_serialization_plugin_list():
     :return:
     """
     serialization_formats = rosbag2_py.get_registered_serializers()
-    assert 's_converter' in serialization_formats
+    assert 's_converter' in serialization_formats, \
+        'get_registered_serializers should return SerializationFormatSerializer plugins'
+    assert 'a_converter' in serialization_formats, \
+        'get_registered_serializers should also return SerializationFormatConverter plugins'