Merge remote-tracking branch 'origin/rolling' into ahcorde/rolling/ex…

…clude_by_topic_type

.github/workflows/test.yml

@@ -88,6 +88,17 @@ jobs:
         source /opt/ros/rolling/setup.sh && colcon test --mixin linters-skip --packages-select ${rosbag2_packages} --packages-skip rosbag2_performance_benchmarking --event-handlers console_cohesion+ --return-code-on-test-failure --ctest-args "-L xfail" --pytest-args "-m xfail"
       working-directory: ${{ steps.action-ros-ci.outputs.ros-workspace-directory-name }}
       shell: bash
+    - name: Is regeneration of Python stubs required?
+      run: |
+        rosbag2_path=$(colcon list -p --packages-select rosbag2)/..
+        sudo pip uninstall -y mypy
+        sudo apt update && sudo apt -y install mypy=0.942-1ubuntu1
+        source install/setup.sh
+        stubgen -p rosbag2_py -o ${rosbag2_path}/rosbag2_py
+        cd ${rosbag2_path}
+        git diff --exit-code
+      working-directory: ${{ steps.action-ros-ci.outputs.ros-workspace-directory-name }}
+      shell: bash
     - uses: actions/upload-artifact@v1
       with:
         name: colcon-logs

DEVELOPING.md

@@ -11,6 +11,7 @@ colcon build --packages-up-to rosbag2
 
 Note: building Rosbag2 from source, overlaid on a debian installation of `ros-$ROS_DISTRO-rosbag2` has undefined behavior (but should work in most cases, just beware the build may find headers from the binaries instead of the local workspace.)
 
+Note: make sure to [regenerate stub files](rosbag2_py/README.md), when making changes to pybind11-related files in `rosbag2_py`.
 
 ## Executing tests
 

README.md

@@ -91,6 +91,8 @@ Currently, the only `compression-format` available is `zstd`. Both the mode and
 
 It is recommended to use this feature with the splitting options.
 
+**Note**: Some storage plugins may have their own compression methods, which are separate from the rosbag2 compression specified by the CLI options `--compression-mode` and `--compression-format`. Notably, the MCAP file format offered by the `rosbag2_storage_mcap` storage plugin supports compression in a way that produces files that are still indexable (whereas using the rosbag2 compression will not). To utilize storage plugin specific compression or other options, see [Recording with a storage configuration](#Recording-with-a-storage-configuration).
+
 #### Recording with a storage configuration
 
 Storage configuration can be specified in a YAML file passed through the `--storage-config-file` option.

rosbag2_compression/src/rosbag2_compression/sequential_compression_reader.cpp

@@ -14,19 +14,21 @@
 
 #include "rosbag2_compression/sequential_compression_reader.hpp"
 
+#include <filesystem>
 #include <memory>
 #include <stdexcept>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "rcpputils/asserts.hpp"
-#include "rcpputils/filesystem_helper.hpp"
 
 #include "rosbag2_compression/compression_options.hpp"
 
 #include "logging.hpp"
 
+namespace fs = std::filesystem;
+
 namespace rosbag2_compression
 {
 SequentialCompressionReader::SequentialCompressionReader(
@@ -67,17 +69,17 @@ void SequentialCompressionReader::preprocess_current_file()
      * Because we have no way to check whether the bag was written correctly,
      * check for the existence of the prefixed file as a fallback.
      */
-    const rcpputils::fs::path base{base_folder_};
-    const rcpputils::fs::path relative{get_current_file()};
+    const fs::path base{base_folder_};
+    const fs::path relative{get_current_file()};
     const auto resolved = base / relative;
-    if (!resolved.exists()) {
+    if (!fs::exists(resolved)) {
       const auto base_stripped = relative.filename();
       const auto resolved_stripped = base / base_stripped;
       ROSBAG2_COMPRESSION_LOG_DEBUG_STREAM(
         "Unable to find specified bagfile " << resolved.string() <<
           ". Falling back to checking for " << resolved_stripped.string());
       rcpputils::require_true(
-        resolved_stripped.exists(),
+        fs::exists(resolved_stripped),
         "Unable to resolve relative file path either as a V3 or V4 relative path");
       *current_file_iterator_ = resolved_stripped.string();
     }

rosbag2_compression/src/rosbag2_compression/sequential_compression_writer.cpp

@@ -17,16 +17,15 @@
 #include <algorithm>
 #include <chrono>
 #include <cstring>
+#include <filesystem>
 #include <functional>
 #include <memory>
 #include <stdexcept>
 #include <string>
+#include <system_error>
 #include <utility>
 
 #include "rcpputils/asserts.hpp"
-#include "rcpputils/filesystem_helper.hpp"
-
-#include "rcutils/filesystem.h"
 
 #include "rosbag2_cpp/info.hpp"
 
@@ -41,6 +40,8 @@
 #include <sys/resource.h>
 #endif
 
+namespace fs = std::filesystem;
+
 namespace rosbag2_compression
 {
 
@@ -294,14 +295,14 @@ void SequentialCompressionWriter::compress_file(
   BaseCompressorInterface & compressor,
   const std::string & file_relative_to_bag)
 {
-  using rcpputils::fs::path;
-
-  const auto file_relative_to_pwd = path(base_folder_) / file_relative_to_bag;
+  const auto file_relative_to_pwd = fs::path(base_folder_) / file_relative_to_bag;
   ROSBAG2_COMPRESSION_LOG_INFO_STREAM("Compressing file: " << file_relative_to_pwd.string());
 
-  if (file_relative_to_pwd.exists() && file_relative_to_pwd.file_size() > 0u) {
+  if (fs::exists(file_relative_to_pwd) &&
+    fs::file_size(file_relative_to_pwd) > 0u)
+  {
     const auto compressed_uri = compressor.compress_uri(file_relative_to_pwd.string());
-    const auto relative_compressed_uri = path(compressed_uri).filename();
+    const auto relative_compressed_uri = fs::path(compressed_uri).filename();
     {
       // After we've compressed the file, replace the name in the file list with the new name.
       // Must search for the entry because other threads may have changed the order of the vector
@@ -320,10 +321,11 @@ void SequentialCompressionWriter::compress_file(
       }
     }
 
-    if (!rcpputils::fs::remove(file_relative_to_pwd)) {
+    if (std::error_code ec;!fs::remove(file_relative_to_pwd, ec)) {
       ROSBAG2_COMPRESSION_LOG_ERROR_STREAM(
         "Failed to remove original pre-compressed bag file: \"" <<
-          file_relative_to_pwd.string() << "\". This should never happen - but execution " <<
+          file_relative_to_pwd.string() << "\"." << ec.message() <<
+          "This should never happen - but execution " <<
           "will not be halted because the compressed output was successfully created.");
     }
   } else {

rosbag2_compression/test/rosbag2_compression/fake_decompressor.cpp

@@ -12,19 +12,20 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <filesystem>
 #include <string>
 
 #include "pluginlib/class_list_macros.hpp"
 
-#include "rcpputils/filesystem_helper.hpp"
-
 #include "fake_decompressor.hpp"
 
+namespace fs = std::filesystem;
+
 std::string FakeDecompressor::decompress_uri(const std::string & uri)
 {
-  auto uri_path = rcpputils::fs::path{uri};
-  const auto decompressed_path = rcpputils::fs::remove_extension(uri_path);
-  return decompressed_path.string();
+  auto uri_path = fs::path{uri};
+  const auto decompressed_path = uri_path.replace_extension();
+  return decompressed_path.generic_string();
 }
 
 void FakeDecompressor::decompress_serialized_bag_message(

rosbag2_compression/test/rosbag2_compression/test_sequential_compression_reader.cpp

@@ -14,14 +14,14 @@
 
 #include <gmock/gmock.h>
 
+#include <filesystem>
 #include <fstream>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "rcpputils/asserts.hpp"
-#include "rcpputils/filesystem_helper.hpp"
 
 #include "rosbag2_compression/sequential_compression_reader.hpp"
 
@@ -37,6 +37,8 @@
 
 using namespace testing;  // NOLINT
 
+namespace fs = std::filesystem;
+
 static constexpr const char * DefaultTestCompressor = "fake_comp";
 
 class SequentialCompressionReaderTest : public Test
@@ -48,10 +50,10 @@ class SequentialCompressionReaderTest : public Test
     converter_factory_{std::make_shared<StrictMock<MockConverterFactory>>()},
     metadata_io_{std::make_unique<NiceMock<MockMetadataIo>>()},
     storage_serialization_format_{"rmw1_format"},
-    tmp_dir_{rcpputils::fs::temp_directory_path() / bag_name_},
+    tmp_dir_{fs::temp_directory_path() / bag_name_},
     converter_options_{"", storage_serialization_format_}
   {
-    rcpputils::fs::remove_all(tmp_dir_);
+    fs::remove_all(tmp_dir_);
     storage_options_.uri = tmp_dir_.string();
     topic_with_type_ = rosbag2_storage::TopicMetadata{
       0U, "topic", "test_msgs/BasicTypes", storage_serialization_format_, {}, ""};
@@ -92,7 +94,7 @@ class SequentialCompressionReaderTest : public Test
   void initialize_dummy_storage_files()
   {
     // Initialize some dummy files so that they can be found
-    rcpputils::fs::create_directories(tmp_dir_);
+    fs::create_directories(tmp_dir_);
     for (auto relative : metadata_.relative_file_paths) {
       std::ofstream output((tmp_dir_ / relative).string());
       output << "Fake storage data" << std::endl;
@@ -104,9 +106,9 @@ class SequentialCompressionReaderTest : public Test
     auto decompressor = std::make_unique<NiceMock<MockDecompressor>>();
     ON_CALL(*decompressor, decompress_uri).WillByDefault(
       [](auto uri) {
-        auto path = rcpputils::fs::path(uri);
-        EXPECT_TRUE(path.exists());
-        return rcpputils::fs::remove_extension(path).string();
+        auto path = fs::path(uri);
+        EXPECT_TRUE(fs::exists(path));
+        return path.replace_extension().generic_string();
       });
     auto compression_factory = std::make_unique<NiceMock<MockCompressionFactory>>();
     ON_CALL(*compression_factory, create_decompressor(_))
@@ -126,7 +128,7 @@ class SequentialCompressionReaderTest : public Test
   std::string storage_serialization_format_;
   rosbag2_storage::TopicMetadata topic_with_type_;
   const std::string bag_name_ = "SequentialCompressionReaderTest";
-  rcpputils::fs::path tmp_dir_;
+  fs::path tmp_dir_;
   rosbag2_storage::StorageOptions storage_options_;
   rosbag2_storage::BagMetadata metadata_;
   rosbag2_cpp::ConverterOptions converter_options_;
@@ -254,7 +256,7 @@ TEST_F(SequentialCompressionReaderTest, throws_on_incorrect_filenames)
 {
   for (auto & relative_file_path : metadata_.relative_file_paths) {
     relative_file_path = (
-      rcpputils::fs::path(bag_name_) / (relative_file_path + ".something")).string();
+      fs::path(bag_name_) / (relative_file_path + ".something")).string();
   }
   auto reader = create_reader();
   EXPECT_THROW(reader->open(storage_options_, converter_options_), std::invalid_argument);
@@ -264,7 +266,7 @@ TEST_F(SequentialCompressionReaderTest, can_find_prefixed_filenames)
 {
   // By prefixing the bag name, this imitates the V3 filename logic
   for (auto & relative_file_path : metadata_.relative_file_paths) {
-    relative_file_path = (rcpputils::fs::path(bag_name_) / relative_file_path).string();
+    relative_file_path = (fs::path(bag_name_) / relative_file_path).string();
   }
   auto reader = create_reader();
 
@@ -278,7 +280,7 @@ TEST_F(SequentialCompressionReaderTest, can_find_prefixed_filenames_in_renamed_b
   // was recorded using V3 logic, then the directory was moved to be a new name - this is the
   // use case the V4 relative path logic was intended to fix
   for (auto & relative_file_path : metadata_.relative_file_paths) {
-    relative_file_path = (rcpputils::fs::path("OtherBagName") / relative_file_path).string();
+    relative_file_path = (fs::path("OtherBagName") / relative_file_path).string();
   }
   auto reader = create_reader();
 

rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp

@@ -14,14 +14,14 @@
 
 #include <gmock/gmock.h>
 
+#include <filesystem>
 #include <fstream>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "rcpputils/asserts.hpp"
-#include "rcpputils/filesystem_helper.hpp"
 
 #include "rosbag2_compression/compression_options.hpp"
 #include "rosbag2_compression/sequential_compression_writer.hpp"
@@ -47,6 +47,8 @@
 
 using namespace testing;  // NOLINT
 
+namespace fs = std::filesystem;
+
 static constexpr const char * DefaultTestCompressor = "fake_comp";
 
 class SequentialCompressionWriterTest : public TestWithParam<uint64_t>
@@ -57,12 +59,12 @@ class SequentialCompressionWriterTest : public TestWithParam<uint64_t>
     storage_{std::make_shared<NiceMock<MockStorage>>()},
     converter_factory_{std::make_shared<StrictMock<MockConverterFactory>>()},
     metadata_io_{std::make_unique<NiceMock<MockMetadataIo>>()},
-    tmp_dir_{rcpputils::fs::temp_directory_path() / "SequentialCompressionWriterTest"},
+    tmp_dir_{fs::temp_directory_path() / "SequentialCompressionWriterTest"},
     tmp_dir_storage_options_{},
     serialization_format_{"rmw_format"}
   {
     tmp_dir_storage_options_.uri = tmp_dir_.string();
-    rcpputils::fs::remove_all(tmp_dir_);
+    fs::remove_all(tmp_dir_);
     ON_CALL(*storage_factory_, open_read_write(_)).WillByDefault(Return(storage_));
     EXPECT_CALL(*storage_factory_, open_read_write(_)).Times(AtLeast(0));
     // intercept the metadata write so we can analyze it.
@@ -79,7 +81,7 @@ class SequentialCompressionWriterTest : public TestWithParam<uint64_t>
 
   ~SequentialCompressionWriterTest() override
   {
-    rcpputils::fs::remove_all(tmp_dir_);
+    fs::remove_all(tmp_dir_);
   }
 
   void initializeFakeFileStorage()
@@ -139,7 +141,7 @@ class SequentialCompressionWriterTest : public TestWithParam<uint64_t>
   std::shared_ptr<StrictMock<MockConverterFactory>> converter_factory_;
   std::unique_ptr<MockMetadataIo> metadata_io_;
 
-  rcpputils::fs::path tmp_dir_;
+  fs::path tmp_dir_;
   rosbag2_storage::StorageOptions tmp_dir_storage_options_;
   rosbag2_storage::BagMetadata intercepted_write_metadata_;
   std::vector<rosbag2_storage::BagMetadata> v_intercepted_update_metadata_;
@@ -212,7 +214,7 @@ TEST_F(SequentialCompressionWriterTest, open_succeeds_on_supported_compression_f
     kDefaultCompressionQueueThreadsPriority};
   initializeWriter(compression_options);
 
-  auto tmp_dir = rcpputils::fs::temp_directory_path() / "path_not_empty";
+  auto tmp_dir = fs::temp_directory_path() / "path_not_empty";
   auto storage_options = rosbag2_storage::StorageOptions();
   storage_options.uri = tmp_dir.string();
 

rosbag2_compression_zstd/CMakeLists.txt

@@ -23,7 +23,6 @@ endif()
 
 find_package(ament_cmake REQUIRED)
 find_package(pluginlib REQUIRED)
-find_package(rcpputils REQUIRED)
 find_package(rosbag2_compression REQUIRED)
 find_package(zstd_vendor REQUIRED)
 find_package(zstd REQUIRED)
@@ -37,7 +36,6 @@ target_include_directories(${PROJECT_NAME}
   $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
   $<INSTALL_INTERFACE:include/${PROJECT_NAME}>)
 target_link_libraries(${PROJECT_NAME}
-  rcpputils::rcpputils
   rosbag2_compression::rosbag2_compression
   zstd::zstd
 )
@@ -63,7 +61,7 @@ ament_export_libraries(${PROJECT_NAME})
 ament_export_targets(export_${PROJECT_NAME})
 
 # order matters here, first vendor, then zstd
-ament_export_dependencies(rcpputils rosbag2_compression zstd_vendor zstd)
+ament_export_dependencies(rosbag2_compression zstd_vendor zstd)
 
 
 if(BUILD_TESTING)

rosbag2_compression_zstd/package.xml

@@ -15,7 +15,6 @@
   <buildtool_depend>ament_cmake</buildtool_depend>
 
   <depend>pluginlib</depend>
-  <depend>rcpputils</depend>
   <depend>rcutils</depend>
   <depend>rosbag2_compression</depend>
   <depend>zstd_vendor</depend>

rosbag2_compression_zstd/src/rosbag2_compression_zstd/compression_utils.hpp

@@ -23,8 +23,7 @@
 #include <vector>
 
 #include "logging.hpp"
-
-#include "rcpputils/filesystem_helper.hpp"
+#include "rcutils/types/rcutils_ret.h"
 
 namespace rosbag2_compression_zstd
 {

rosbag2_compression_zstd/src/rosbag2_compression_zstd/zstd_compressor.cpp

@@ -19,8 +19,6 @@
 #include <string>
 #include <vector>
 
-#include "rcpputils/filesystem_helper.hpp"
-
 #include "compression_utils.hpp"
 #include "rosbag2_compression_zstd/zstd_compressor.hpp"
 #include "rosbag2_storage/ros_helper.hpp"