Adapt interface and introduce better logging

rosbag2/CMakeLists.txt

@@ -17,16 +17,14 @@ endif()
 
 find_package(ament_cmake REQUIRED)
 find_package(rclcpp REQUIRED)
+find_package(rcutils REQUIRED)
 find_package(std_msgs REQUIRED)
 find_package(rosbag2_storage REQUIRED)
 
-
 add_library(librosbag
-  include/rosbag2/rosbag2.hpp
   src/rosbag2/rosbag2.cpp)
 
-ament_target_dependencies(librosbag rclcpp std_msgs rosbag2_storage)
-target_link_libraries(librosbag)
+ament_target_dependencies(librosbag rclcpp rcutils std_msgs rosbag2_storage)
 target_include_directories(librosbag
   PUBLIC
   $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
@@ -38,7 +36,15 @@ target_link_libraries(${PROJECT_NAME}_record librosbag)
 
 add_executable(${PROJECT_NAME}_play src/rosbag2/demo_play.cpp)
 target_link_libraries(${PROJECT_NAME}_play librosbag)
-# TODO(anhosi): Add install for library and headers (GH-5)
+
+install(
+  DIRECTORY include/
+  DESTINATION include)
+
+install(
+  TARGETS librosbag
+  ARCHIVE DESTINATION lib
+  LIBRARY DESTINATION lib)
 
 if(BUILD_TESTING)
   # TODO(Martin-Idel-SI): Add copyright linter or use ament_lint_auto() once available
@@ -54,14 +60,16 @@ if(BUILD_TESTING)
   find_package(ament_cmake_gtest REQUIRED)
   find_package(ament_cmake_gmock REQUIRED)
 
-  ament_add_gmock(rosbag2_write_integration_test test/rosbag2/rosbag2_write_integration_test.cpp
+  ament_add_gmock(rosbag2_write_integration_test
+    test/rosbag2/rosbag2_write_integration_test.cpp
     test/rosbag2/rosbag2_test_fixture.hpp
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   if(TARGET rosbag2_write_integration_test)
     target_link_libraries(rosbag2_write_integration_test librosbag)
   endif()
 
-  ament_add_gmock(rosbag2_read_integration_test test/rosbag2/rosbag2_read_integration_test.cpp
+  ament_add_gmock(rosbag2_read_integration_test
+    test/rosbag2/rosbag2_read_integration_test.cpp
     test/rosbag2/rosbag2_test_fixture.hpp
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   if(TARGET rosbag2_read_integration_test)

rosbag2/package.xml

@@ -10,7 +10,10 @@
   <buildtool_depend>ament_cmake</buildtool_depend>
 
   <build_depend>rosbag2_storage</build_depend>
+  <build_depend>rcutils</build_depend>
+
   <exec_depend>rosbag2_storage</exec_depend>
+  <exec_depend>rcutils</exec_depend>
 
   <test_depend>ament_lint_auto</test_depend>
   <test_depend>ament_lint_common</test_depend>

rosbag2/src/rosbag2/rosbag2.cpp

@@ -19,55 +19,54 @@
 #include <string>
 
 #include "rclcpp/rclcpp.hpp"
+#include "rcutils/logging_macros.h"
 #include "std_msgs/msg/string.hpp"
 
 #include "rosbag2_storage/storage_factory.hpp"
 
 namespace rosbag2
 {
 
+const char * ROS_PACKAGE_NAME = "rosbag2";
+
 void Rosbag2::record(
   const std::string & file_name,
   const std::string & topic_name,
   std::function<void(void)> after_write_action)
 {
   rosbag2_storage::StorageFactory factory;
-  std::unique_ptr<rosbag2_storage::WritableStorage> storage =
-    factory.get_for_writing("sqlite3", file_name);
+  auto storage =
+    factory.get_read_write_storage("sqlite3", file_name);
+  storage->create_topic();
 
   if (storage) {
     auto node = std::make_shared<rclcpp::Node>("rosbag_node");
     auto subscription = node->create_subscription<std_msgs::msg::String>(
       topic_name,
       [&storage, after_write_action](std_msgs::msg::String::ConstSharedPtr msg) {
-        std::string message = msg->data;
-        void * data = &message;
-        storage->write(data, strlen(msg->data.c_str()));
+        storage->write(msg->data);
         if (after_write_action) {
           after_write_action();
         }
       });
 
-    // TODO(anhosi): use proper logging from rcutils
-    std::cout << "Waiting for messages..." << std::endl;
+    RCUTILS_LOG_INFO_NAMED(ROS_PACKAGE_NAME, "Waiting for messages...");
     rclcpp::spin(node);
   }
 }
 
 void Rosbag2::play(const std::string & file_name, const std::string & topic_name)
 {
   rosbag2_storage::StorageFactory factory;
-  std::unique_ptr<rosbag2_storage::ReadableStorage> storage =
-    factory.get_for_reading("sqlite3", file_name);
+  auto storage = factory.get_read_only_storage("sqlite3", file_name);
 
   if (storage) {
-    char buffer[1000];
-    size_t size = 0;
     auto node = std::make_shared<rclcpp::Node>("rosbag_publisher_node");
     auto publisher = node->create_publisher<std_msgs::msg::String>(topic_name);
-    while (storage->read_next(buffer, size)) {
+    while (storage->has_next()) {
+      std::string message = storage->read_next();
       auto string_msg = std_msgs::msg::String();
-      string_msg.data = buffer;
+      string_msg.data = message;
       // without the sleep_for() many messages are lost.
       std::this_thread::sleep_for(std::chrono::milliseconds(500));
       publisher->publish(string_msg);

rosbag2/test/rosbag2/rosbag2_test_fixture.hpp

@@ -93,14 +93,13 @@ class Rosbag2TestFixture : public Test
   {
     std::vector<std::string> table_msgs;
     rosbag2_storage::StorageFactory factory;
-    std::unique_ptr<rosbag2_storage::ReadableStorage> storage =
-      factory.get_for_reading("sqlite3", db_name);
+    auto storage =
+      factory.get_read_only_storage("sqlite3", db_name);
 
     if (storage) {
-      char buffer[1000];
-      size_t size = 0;
-      while (storage->read_next(buffer, size)) {
-        table_msgs.emplace_back(buffer);
+      while (storage->has_next()) {
+        std::string message = storage->read_next();
+        table_msgs.emplace_back(message);
       }
     }
     return table_msgs;
@@ -109,12 +108,13 @@ class Rosbag2TestFixture : public Test
   void write_messages(const std::string & db_name, std::vector<std::string> messages)
   {
     rosbag2_storage::StorageFactory factory;
-    std::unique_ptr<rosbag2_storage::WritableStorage> storage =
-      factory.get_for_writing("sqlite3", db_name);
+    auto storage =
+      factory.get_read_write_storage("sqlite3", db_name);
+    storage->create_topic();
 
     if (storage) {
       for (auto msg : messages) {
-        storage->write(&msg, 0);
+        storage->write(msg);
       }
     }
   }

rosbag2/test/rosbag2/rosbag2_write_integration_test.cpp

@@ -94,6 +94,6 @@ TEST_F(RosBag2IntegrationTestFixture, published_messages_are_recorded)
 
   auto recorded_messages = get_messages(database_name_);
 
-  ASSERT_THAT(recorded_messages, SizeIs(1));
+  ASSERT_THAT(recorded_messages, Not(IsEmpty()));
   ASSERT_THAT(recorded_messages[0], Eq(message_to_publish));
 }

rosbag2_storage_default_plugins/CMakeLists.txt

@@ -16,10 +16,10 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
 endif()
 
 find_package(ament_cmake REQUIRED)
-find_package(sqlite3_vendor REQUIRED)
-find_package(rosbag2_storage REQUIRED)
 find_package(pluginlib REQUIRED)
-
+find_package(rcutils REQUIRED)
+find_package(rosbag2_storage REQUIRED)
+find_package(sqlite3_vendor REQUIRED)
 find_package(SQLite3 REQUIRED)  # provided by sqlite3_vendor
 
 add_library(${PROJECT_NAME} SHARED
@@ -28,8 +28,14 @@ add_library(${PROJECT_NAME} SHARED
   src/rosbag2_storage_default_plugins/sqlite/sqlite_storage.hpp
   src/rosbag2_storage_default_plugins/sqlite/sqlite_storage.cpp)
 
-ament_target_dependencies(${PROJECT_NAME} rclcpp std_msgs SQLite3 rosbag2_storage pluginlib)
-target_link_libraries(${PROJECT_NAME})
+ament_target_dependencies(${PROJECT_NAME}
+  rosbag2_storage
+  rclcpp
+  rcutils
+  std_msgs
+  SQLite3
+  pluginlib)
+
 target_include_directories(${PROJECT_NAME}
   PUBLIC
   $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
@@ -60,16 +66,32 @@ if(BUILD_TESTING)
 
   ament_add_gmock(sqlite_storage_test
     test/rosbag2_storage_default_plugins/sqlite/sqlite_storage_test.cpp
+    src/rosbag2_storage_default_plugins/sqlite/sqlite_storage.cpp
+    src/rosbag2_storage_default_plugins/sqlite/sqlite_wrapper.cpp
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   if(TARGET sqlite_storage_test)
-    target_link_libraries(sqlite_storage_test ${PROJECT_NAME})
+    ament_target_dependencies(sqlite_storage_test
+      rosbag2_storage
+      rclcpp
+      rcutils
+      std_msgs
+      SQLite3
+      pluginlib)
   endif()
 
   ament_add_gmock(sqlite_storage_integration_test
     test/rosbag2_storage_default_plugins/sqlite/sqlite_storage_integration_test.cpp
+    src/rosbag2_storage_default_plugins/sqlite/sqlite_storage.cpp
+    src/rosbag2_storage_default_plugins/sqlite/sqlite_wrapper.cpp
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
   if(TARGET sqlite_storage_integration_test)
-    target_link_libraries(sqlite_storage_integration_test ${PROJECT_NAME})
+    ament_target_dependencies(sqlite_storage_integration_test
+      rosbag2_storage
+      rclcpp
+      rcutils
+      std_msgs
+      SQLite3
+      pluginlib)
   endif()
 endif()
 

rosbag2_storage_default_plugins/package.xml

@@ -1,5 +1,4 @@
 <?xml version="1.0"?>
-<?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rosbag2_storage_default_plugins</name>
   <version>0.0.0</version>
@@ -12,11 +11,13 @@
   <build_depend>sqlite3_vendor</build_depend>
   <build_depend>rclcpp</build_depend>
   <build_depend>std_msgs</build_depend>
+  <build_depend>rcutils</build_depend>
   <build_depend>rosbag2_storage</build_depend>
 
   <exec_depend>sqlite3_vendor</exec_depend>
   <exec_depend>rosbag2_storage</exec_depend>
   <exec_depend>rclcpp</exec_depend>
+  <exec_depend>rcutils</exec_depend>
   <exec_depend>std_msgs</exec_depend>
 
   <test_depend>ament_lint_auto</test_depend>

rosbag2_storage_default_plugins/plugin_description.xml

@@ -2,16 +2,8 @@
   <class
     name="sqlite3"
     type="rosbag2_storage_plugins::SqliteStorage"
-    base_class_type="rosbag2_storage::WritableStorage"
+    base_class_type="rosbag2_storage::ReadWriteStorage"
   >
     <description>Plugin to write to SQLite3 databases</description>
   </class>
-
-  <class
-    name="sqlite3"
-    type="rosbag2_storage_plugins::SqliteStorage"
-    base_class_type="rosbag2_storage::ReadableStorage"
-  >
-    <description>Plugin to read from SQLite3 databases</description>
-  </class>
 </library>