ros2GH-23 Move methods to node for better interface

rosbag2/CMakeLists.txt

@@ -110,14 +110,6 @@ if(BUILD_TESTING)
     )
   endif()
 
-  ament_add_gmock(rosbag2_integration_test
-    test/rosbag2/rosbag2_integration_test.cpp
-    src/rosbag2/typesupport_helpers.cpp
-    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
-  if(TARGET rosbag2_integration_test)
-    target_link_libraries(rosbag2_integration_test librosbag2)
-  endif()
-
   ament_add_gmock(rosbag2_rosbag_node_test
     test/rosbag2/rosbag2_rosbag_node_test.cpp
     test/rosbag2/test_memory_management.cpp

rosbag2/include/rosbag2/rosbag2.hpp

@@ -62,15 +62,6 @@ class Rosbag2
   ROSBAG2_PUBLIC
   void play(const std::string & file_name);
 
-  /// Exposed for testing
-  ROSBAG2_PUBLIC
-  std::map<std::string, std::string> get_topics_with_types(
-    const std::vector<std::string> & topic_names, std::shared_ptr<rclcpp::Node> node);
-
-  /// Exposed for testing
-  ROSBAG2_PUBLIC
-  std::map<std::string, std::string> get_all_topics_with_types(std::shared_ptr<rclcpp::Node> node);
-
 private:
   void prepare_publishers(
     std::shared_ptr<Rosbag2Node> node,
@@ -83,9 +74,6 @@ class Rosbag2
     std::shared_ptr<Rosbag2Node> & node,
     const std::string & topic_name, const std::string & topic_type) const;
 
-  std::map<std::string, std::string> sanitize_topics_and_types(
-    std::map<std::string, std::vector<std::string>> topics_and_types);
-
   std::vector<std::shared_ptr<GenericSubscription>> subscriptions_;
   std::map<std::string, std::shared_ptr<GenericPublisher>> publishers_;
 };

rosbag2/src/rosbag2/rosbag2.cpp

@@ -14,12 +14,10 @@
 
 #include "rosbag2/rosbag2.hpp"
 
-#include <algorithm>
 #include <chrono>
 #include <map>
 #include <memory>
 #include <string>
-#include <utility>
 #include <vector>
 
 #include "rcl/graph.h"
@@ -58,8 +56,8 @@ void Rosbag2::record(
   auto node = std::make_shared<Rosbag2Node>("rosbag2");
 
   auto topics_and_types = topic_names.empty() ?
-    get_all_topics_with_types(node) :
-    get_topics_with_types(topic_names, node);
+    node->get_all_topics_with_types() :
+    node->get_topics_with_types(topic_names);
 
   if (topics_and_types.empty()) {
     throw std::runtime_error("No topics found. Abort");
@@ -152,67 +150,4 @@ void Rosbag2::prepare_publishers(
   }
 }
 
-std::map<std::string, std::string> Rosbag2::get_topics_with_types(
-  const std::vector<std::string> & topic_names, std::shared_ptr<rclcpp::Node> node)
-{
-  std::vector<std::string> sanitized_topic_names;
-  std::transform(topic_names.begin(), topic_names.end(), std::back_inserter(sanitized_topic_names),
-    [](std::string topic_name) {
-      return topic_name[0] != '/' ? "/" + topic_name : topic_name;
-    });
-
-  // TODO(Martin-Idel-SI): This is a short sleep to allow the node some time to discover the topic
-  // This should be replaced by an auto-discovery system in the future
-  std::this_thread::sleep_for(std::chrono::milliseconds(100));
-  auto topics_and_types = node->get_topic_names_and_types();
-
-  std::map<std::string, std::vector<std::string>> filtered_topics_and_types;
-  std::remove_copy_if(topics_and_types.begin(), topics_and_types.end(),
-    std::inserter(filtered_topics_and_types, filtered_topics_and_types.end()),
-    [sanitized_topic_names](auto element) {
-      return std::find(sanitized_topic_names.begin(), sanitized_topic_names.end(), element.first) ==
-      sanitized_topic_names.end();
-    });
-
-  return sanitize_topics_and_types(filtered_topics_and_types);
-}
-
-std::map<std::string, std::string>
-Rosbag2::get_all_topics_with_types(std::shared_ptr<rclcpp::Node> node)
-{
-  // TODO(Martin-Idel-SI): This is a short sleep to allow the node some time to discover the topic
-  // This should be replaced by an auto-discovery system in the future
-  std::this_thread::sleep_for(std::chrono::milliseconds(100));
-  return sanitize_topics_and_types(node->get_topic_names_and_types());
-}
-
-std::map<std::string, std::string> Rosbag2::sanitize_topics_and_types(
-  std::map<std::string, std::vector<std::string>> topics_and_types)
-{
-  std::map<std::string, std::vector<std::string>> filtered_topics_and_types;
-  std::remove_copy_if(topics_and_types.begin(),
-    topics_and_types.end(),
-    std::inserter(filtered_topics_and_types, filtered_topics_and_types.end()),
-    [](auto element) {
-      if (element.second.size() > 1) {
-        RCUTILS_LOG_ERROR_NAMED(
-          ROS_PACKAGE_NAME,
-          "Topic '%s' has several types associated. Only topics with one type are supported.",
-          element.first.c_str());
-        return true;
-      }
-      return false;
-    });
-
-  std::map<std::string, std::string> topics_and_types_to_record;
-  std::transform(
-    filtered_topics_and_types.begin(),
-    filtered_topics_and_types.end(),
-    std::inserter(topics_and_types_to_record, topics_and_types_to_record.end()),
-    [](auto element) {
-      return std::make_pair(element.first, element.second[0]);
-    });
-  return topics_and_types_to_record;
-}
-
 }  // namespace rosbag2

rosbag2/src/rosbag2/rosbag2_node.cpp

@@ -14,8 +14,12 @@
 
 #include "rosbag2_node.hpp"
 
+#include <algorithm>
+#include <map>
 #include <memory>
 #include <string>
+#include <vector>
+#include <utility>
 
 #include "typesupport_helpers.hpp"
 
@@ -57,5 +61,67 @@ std::shared_ptr<GenericSubscription> Rosbag2Node::create_generic_subscription(
 
   return subscription;
 }
+std::map<std::string, std::string> Rosbag2Node::get_topics_with_types(
+  const std::vector<std::string> & topic_names)
+{
+  std::vector<std::string> sanitized_topic_names;
+  std::transform(topic_names.begin(), topic_names.end(), std::back_inserter(sanitized_topic_names),
+    [](std::string topic_name) {
+      return topic_name[0] != '/' ? "/" + topic_name : topic_name;
+    });
+
+  // TODO(Martin-Idel-SI): This is a short sleep to allow the node some time to discover the topic
+  // This should be replaced by an auto-discovery system in the future
+  std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  auto topics_and_types = this->get_topic_names_and_types();
+
+  std::map<std::string, std::vector<std::string>> filtered_topics_and_types;
+  std::remove_copy_if(topics_and_types.begin(), topics_and_types.end(),
+    std::inserter(filtered_topics_and_types, filtered_topics_and_types.end()),
+    [sanitized_topic_names](auto element) {
+      return std::find(sanitized_topic_names.begin(), sanitized_topic_names.end(), element.first) ==
+      sanitized_topic_names.end();
+    });
+
+  return sanitize_topics_and_types(filtered_topics_and_types);
+}
+
+std::map<std::string, std::string>
+Rosbag2Node::get_all_topics_with_types()
+{
+  // TODO(Martin-Idel-SI): This is a short sleep to allow the node some time to discover the topic
+  // This should be replaced by an auto-discovery system in the future
+  std::this_thread::sleep_for(std::chrono::milliseconds(100));
+  return sanitize_topics_and_types(this->get_topic_names_and_types());
+}
+
+std::map<std::string, std::string> Rosbag2Node::sanitize_topics_and_types(
+  std::map<std::string, std::vector<std::string>> topics_and_types)
+{
+  std::map<std::string, std::vector<std::string>> filtered_topics_and_types;
+  std::remove_copy_if(topics_and_types.begin(),
+    topics_and_types.end(),
+    std::inserter(filtered_topics_and_types, filtered_topics_and_types.end()),
+    [](auto element) {
+      if (element.second.size() > 1) {
+        RCUTILS_LOG_ERROR_NAMED(
+          "rosbag2",
+          "Topic '%s' has several types associated. Only topics with one type are supported.",
+          element.first.c_str());
+        return true;
+      }
+      return false;
+    });
+
+  std::map<std::string, std::string> topics_and_types_to_record;
+  std::transform(
+    filtered_topics_and_types.begin(),
+    filtered_topics_and_types.end(),
+    std::inserter(topics_and_types_to_record, topics_and_types_to_record.end()),
+    [](auto element) {
+      return std::make_pair(element.first, element.second[0]);
+    });
+  return topics_and_types_to_record;
+}
 
 }  // namespace rosbag2

rosbag2/src/rosbag2/rosbag2_node.hpp

@@ -15,8 +15,10 @@
 #ifndef ROSBAG2__ROSBAG2_NODE_HPP_
 #define ROSBAG2__ROSBAG2_NODE_HPP_
 
+#include <map>
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "rclcpp/node.hpp"
 #include "rcutils/types.h"
@@ -40,6 +42,14 @@ class Rosbag2Node : public rclcpp::Node
     const std::string & topic,
     const std::string & type,
     std::function<void(std::shared_ptr<rcutils_char_array_t>)> callback);
+
+  std::map<std::string, std::string>
+  get_topics_with_types(const std::vector<std::string> & topic_names);
+
+  std::map<std::string, std::string> get_all_topics_with_types();
+
+  std::map<std::string, std::string> sanitize_topics_and_types(
+    std::map<std::string, std::vector<std::string>> topics_and_types);
 };
 
 }  // namespace rosbag2