Update rosbag2 filename format to index+name+timestamp

rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp

@@ -17,6 +17,8 @@
 #include <filesystem>
 #include <fstream>
 #include <memory>
+#include <regex>
+#include <sstream>
 #include <string>
 #include <utility>
 #include <vector>
@@ -27,6 +29,7 @@
 #include "rosbag2_compression/sequential_compression_writer.hpp"
 
 #include "rosbag2_cpp/writer.hpp"
+#include "rosbag2_cpp/writers/sequential_writer.hpp"
 
 #include "rosbag2_storage/ros_helper.hpp"
 #include "rosbag2_storage/storage_options.hpp"
@@ -304,12 +307,29 @@ TEST_F(SequentialCompressionWriterTest, writer_creates_correct_metadata_relative
 
   EXPECT_EQ(intercepted_write_metadata_.relative_file_paths.size(), 3u);
 
+  // New filename format: {counter}_{bag_base_dir}_{timestamp}.{compressor}
+  // Timestamp is generated at runtime, so validate using regex
+  // Use static to avoid recompiling regex on each test run
+  static const std::string file_pattern_str =
+    R"(\d+_)" + std::string("test_bag") + R"(_)" +
+    std::string(rosbag2_cpp::writers::TIMESTAMP_PATTERN) +
+    R"(\.)" + std::string(DefaultTestCompressor);
+  static const std::regex file_pattern(file_pattern_str);
+
   int counter = 0;
   for (const auto & path : intercepted_write_metadata_.relative_file_paths) {
-    std::stringstream ss;
-    ss << bag_base_dir_ << "_" << counter << "." << DefaultTestCompressor;
+    // Verify that filename matches expected format
+    EXPECT_TRUE(std::regex_match(path, file_pattern)) <<
+      "Path '" << path << "' does not match expected pattern for file " << counter;
+
+    // Verify that counter is at the correct position
+    std::stringstream expected_prefix;
+    expected_prefix << counter << "_" << bag_base_dir_ << "_";
+    EXPECT_TRUE(path.find(expected_prefix.str()) == 0) <<
+      "Path '" << path << "' does not start with expected prefix '" <<
+      expected_prefix.str() << "'";
+
     counter++;
-    EXPECT_EQ(ss.str(), path);
   }
 }
 
@@ -546,14 +566,44 @@ TEST_P(SequentialCompressionWriterTest, split_event_calls_callback_with_msg_comp
 
   ASSERT_GE(opened_files.size(), num_splits + 1);
   ASSERT_GE(closed_files.size(), num_splits + 1);
+
+  // New filename format: {counter}_{bag_base_dir}_{timestamp}
+  // Timestamp is generated at runtime, so validate using regex
+  static const std::string file_pattern_str =
+    R"(\d+_)" + std::string("test_bag") + R"(_)" +
+    std::string(rosbag2_cpp::writers::TIMESTAMP_PATTERN);
+  static const std::regex file_pattern(file_pattern_str);
+
   for (size_t i = 0; i < num_splits + 1; i++) {
-    auto expected_closed =
-      fs::path(tmp_dir_storage_options_.uri) / (bag_base_dir_ + "_" + std::to_string(i));
-    auto expected_opened = (i == num_splits) ?
+    // Verify closed file format
+    fs::path closed_path(closed_files[i]);
+    std::string closed_filename = closed_path.filename().generic_string();
+    EXPECT_TRUE(std::regex_match(closed_filename, file_pattern)) <<
+      "Closed file '" << closed_filename << "' does not match expected pattern for file " << i;
+
+    // Verify counter is at the correct position
+    std::stringstream expected_prefix;
+    expected_prefix << i << "_" << bag_base_dir_ << "_";
+    EXPECT_TRUE(closed_filename.find(expected_prefix.str()) == 0) <<
+      "Closed file '" << closed_filename << "' does not start with expected prefix '" <<
+      expected_prefix.str() << "'";
+
+    // Verify opened file format
+    if (i == num_splits) {
       // The last opened file shall be empty string when we do "writer->close();"
-      "" : fs::path(tmp_dir_storage_options_.uri) / (bag_base_dir_ + "_" + std::to_string(i + 1));
-    EXPECT_EQ(closed_files[i], expected_closed.generic_string()) << "i = " << i;
-    EXPECT_EQ(opened_files[i], expected_opened.generic_string()) << "i = " << i;
+      EXPECT_EQ(opened_files[i], "") << "i = " << i;
+    } else {
+      fs::path opened_path(opened_files[i]);
+      std::string opened_filename = opened_path.filename().generic_string();
+      EXPECT_TRUE(std::regex_match(opened_filename, file_pattern)) <<
+        "Opened file '" << opened_filename << "' does not match expected pattern for file " << i;
+
+      std::stringstream expected_opened_prefix;
+      expected_opened_prefix << (i + 1) << "_" << bag_base_dir_ << "_";
+      EXPECT_TRUE(opened_filename.find(expected_opened_prefix.str()) == 0) <<
+        "Opened file '" << opened_filename << "' does not start with expected prefix '" <<
+        expected_opened_prefix.str() << "'";
+    }
   }
 }
 
@@ -616,15 +666,49 @@ TEST_P(SequentialCompressionWriterTest, split_event_calls_callback_with_file_com
 
   ASSERT_GE(opened_files.size(), num_splits + 1);
   ASSERT_GE(closed_files.size(), num_splits + 1);
+
+  // New filename format: {counter}_{bag_base_dir}_{timestamp}.{compressor}
+  // Timestamp is generated at runtime, so validate using regex
+  static const std::string closed_file_pattern_str =
+    R"(\d+_)" + std::string("test_bag") + R"(_)" +
+    std::string(rosbag2_cpp::writers::TIMESTAMP_PATTERN) +
+    R"(\.)" + std::string(DefaultTestCompressor);
+  static const std::regex closed_file_pattern(closed_file_pattern_str);
+  static const std::string opened_file_pattern_str =
+    R"(\d+_)" + std::string("test_bag") + R"(_)" +
+    std::string(rosbag2_cpp::writers::TIMESTAMP_PATTERN);
+  static const std::regex opened_file_pattern(opened_file_pattern_str);
+
   for (size_t i = 0; i < num_splits + 1; i++) {
-    auto expected_closed =
-      fs::path(tmp_dir_storage_options_.uri) / (bag_base_dir_ + "_" + std::to_string(i) +
-      "." + DefaultTestCompressor);
-    auto expected_opened = (i == num_splits) ?
+    // Verify closed file format
+    fs::path closed_path(closed_files[i]);
+    std::string closed_filename = closed_path.filename().generic_string();
+    EXPECT_TRUE(std::regex_match(closed_filename, closed_file_pattern)) <<
+      "Closed file '" << closed_filename << "' does not match expected pattern for file " << i;
+
+    // Verify counter is at the correct position
+    std::stringstream expected_prefix;
+    expected_prefix << i << "_" << bag_base_dir_ << "_";
+    EXPECT_TRUE(closed_filename.find(expected_prefix.str()) == 0) <<
+      "Closed file '" << closed_filename << "' does not start with expected prefix '" <<
+      expected_prefix.str() << "'";
+
+    // Verify opened file format
+    if (i == num_splits) {
       // The last opened file shall be empty string when we do "writer->close();"
-      "" : fs::path(tmp_dir_storage_options_.uri) / (bag_base_dir_ + "_" + std::to_string(i + 1));
-    EXPECT_EQ(closed_files[i], expected_closed.generic_string()) << "i = " << i;
-    EXPECT_EQ(opened_files[i], expected_opened.generic_string()) << "i = " << i;
+      EXPECT_EQ(opened_files[i], "") << "i = " << i;
+    } else {
+      fs::path opened_path(opened_files[i]);
+      std::string opened_filename = opened_path.filename().generic_string();
+      EXPECT_TRUE(std::regex_match(opened_filename, opened_file_pattern)) <<
+        "Opened file '" << opened_filename << "' does not match expected pattern for file " << i;
+
+      std::stringstream expected_opened_prefix;
+      expected_opened_prefix << (i + 1) << "_" << bag_base_dir_ << "_";
+      EXPECT_TRUE(opened_filename.find(expected_opened_prefix.str()) == 0) <<
+        "Opened file '" << opened_filename << "' does not start with expected prefix '" <<
+        expected_opened_prefix.str() << "'";
+    }
   }
 }
 
@@ -690,15 +774,48 @@ TEST_F(SequentialCompressionWriterTest, snapshot_writes_to_new_file_with_file_co
   ASSERT_EQ(opened_files.size(), 2);
   ASSERT_EQ(closed_files.size(), 2);
 
+  // New filename format: {counter}_{bag_base_dir}_{timestamp}.{compressor}
+  // Timestamp is generated at runtime, so validate using regex
+  static const std::string closed_file_pattern_str =
+    R"(\d+_)" + std::string("test_bag") + R"(_)" +
+    std::string(rosbag2_cpp::writers::TIMESTAMP_PATTERN) +
+    R"(\.)" + std::string(DefaultTestCompressor);
+  static const std::regex closed_file_pattern(closed_file_pattern_str);
+  static const std::string opened_file_pattern_str =
+    R"(\d+_)" + std::string("test_bag") + R"(_)" +
+    std::string(rosbag2_cpp::writers::TIMESTAMP_PATTERN);
+  static const std::regex opened_file_pattern(opened_file_pattern_str);
+
   for (size_t i = 0; i < 2; i++) {
-    auto expected_closed = fs::path(tmp_dir_storage_options_.uri) /
-      (bag_base_dir_ + "_" + std::to_string(i) + "." + DefaultTestCompressor);
-    auto expected_opened = (i == 1) ?
+    // Verify closed file format
+    fs::path closed_path(closed_files[i]);
+    std::string closed_filename = closed_path.filename().generic_string();
+    EXPECT_TRUE(std::regex_match(closed_filename, closed_file_pattern)) <<
+      "Closed file '" << closed_filename << "' does not match expected pattern for file " << i;
+
+    // Verify counter is at the correct position
+    std::stringstream expected_prefix;
+    expected_prefix << i << "_" << bag_base_dir_ << "_";
+    EXPECT_TRUE(closed_filename.find(expected_prefix.str()) == 0) <<
+      "Closed file '" << closed_filename << "' does not start with expected prefix '" <<
+      expected_prefix.str() << "'";
+
+    // Verify opened file format
+    if (i == 1) {
       // The last opened file shall be empty string when we do "writer->close();"
-      "" : fs::path(tmp_dir_storage_options_.uri) /
-      (bag_base_dir_ + "_" + std::to_string(i + 1));
-    ASSERT_STREQ(closed_files[i].c_str(), expected_closed.generic_string().c_str());
-    ASSERT_STREQ(opened_files[i].c_str(), expected_opened.generic_string().c_str());
+      EXPECT_EQ(opened_files[i], "") << "i = " << i;
+    } else {
+      fs::path opened_path(opened_files[i]);
+      std::string opened_filename = opened_path.filename().generic_string();
+      EXPECT_TRUE(std::regex_match(opened_filename, opened_file_pattern)) <<
+        "Opened file '" << opened_filename << "' does not match expected pattern for file " << i;
+
+      std::stringstream expected_opened_prefix;
+      expected_opened_prefix << (i + 1) << "_" << bag_base_dir_ << "_";
+      EXPECT_TRUE(opened_filename.find(expected_opened_prefix.str()) == 0) <<
+        "Opened file '" << opened_filename << "' does not start with expected prefix '" <<
+        expected_opened_prefix.str() << "'";
+    }
   }
 }
 

rosbag2_cpp/include/rosbag2_cpp/reindexer.hpp

@@ -89,7 +89,8 @@ class ROSBAG2_CPP_PUBLIC Reindexer
   std::vector<rosbag2_storage::TopicMetadata> topics_metadata_{};
 
 private:
-  std::string regex_bag_pattern_;
+  std::string new_format_regex_;
+  std::string old_format_regex_;
   std::filesystem::path base_folder_;   // The folder that the bag files are in
   std::shared_ptr<SerializationFormatConverterFactoryInterface> converter_factory_{};
   void get_bag_files(

rosbag2_cpp/include/rosbag2_cpp/writers/sequential_writer.hpp

@@ -52,6 +52,10 @@ namespace rosbag2_cpp
 namespace writers
 {
 
+// Timestamp pattern used in filenames: YYYY_MM_DD-HH_MM_SS
+// This pattern matches timestamps generated by format_storage_uri()
+constexpr const char * TIMESTAMP_PATTERN = R"(\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})";
+
 /**
  * The Writer allows writing messages to a new bag. For every topic, information about its type
  * needs to be added before writing the first message.

rosbag2_cpp/src/rosbag2_cpp/reindexer.cpp

@@ -49,7 +49,10 @@ Reindexer::Reindexer(
 : storage_factory_(std::move(storage_factory)),
   metadata_io_(std::move(metadata_io))
 {
-  regex_bag_pattern_ = R"(.+_(\d+)\.([a-zA-Z0-9])+)";
+  // Support both old format (prefix_index.ext) and new format (index_prefix_timestamp.ext[.zstd])
+  // Use separate regex patterns for better maintainability and debugging
+  new_format_regex_ = R"((\d+)_(.*)_(\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})(\.[a-zA-Z0-9]+){1,2})";
+  old_format_regex_ = R"((.*)_(\d+)(\.[a-zA-Z0-9]+){1,2})";
 }
 
 /// Determine which path should be placed first in a vector ordered by file number.
@@ -61,33 +64,59 @@ bool Reindexer::compare_relative_file(
   const fs::path & first_path,
   const fs::path & second_path)
 {
-  std::regex regex_rule(regex_bag_pattern_, std::regex_constants::ECMAScript);
+  std::regex new_format_rule(new_format_regex_, std::regex_constants::ECMAScript);
+  std::regex old_format_rule(old_format_regex_, std::regex_constants::ECMAScript);
 
   std::smatch first_match;
   std::smatch second_match;
 
   auto first_path_string = first_path.generic_string();
   auto second_path_string = second_path.generic_string();
 
-  auto first_regex_good = std::regex_match(first_path_string, first_match, regex_rule);
-  auto second_regex_good = std::regex_match(second_path_string, second_match, regex_rule);
+  // Try new format first, then old format
+  auto first_regex_good = std::regex_match(first_path_string, first_match, new_format_rule);
+  auto first_is_new_format = first_regex_good;
+
+  if (!first_regex_good) {
+    first_regex_good = std::regex_match(first_path_string, first_match, old_format_rule);
+  }
+
+  auto second_regex_good = std::regex_match(second_path_string, second_match, new_format_rule);
+  auto second_is_new_format = second_regex_good;
+
+  if (!second_regex_good) {
+    second_regex_good = std::regex_match(second_path_string, second_match, old_format_rule);
+  }
 
   if (!first_regex_good) {
     std::stringstream ss;
     ss << "Path " << first_path.generic_string() <<
-      "didn't meet expected naming convention: " << regex_bag_pattern_;
+      " didn't match any expected naming convention";
     std::string error_text = ss.str();
     throw std::runtime_error(error_text.c_str());
   } else if (!second_regex_good) {
     std::stringstream ss;
     ss << "Path " << second_path.generic_string() <<
-      "didn't meet expected naming convention: " << regex_bag_pattern_;
+      " didn't match any expected naming convention";
     std::string error_text = ss.str();
     throw std::runtime_error(error_text.c_str());
   }
 
-  auto first_file_num = std::stoul(first_match.str(1), nullptr, 10);
-  auto second_file_num = std::stoul(second_match.str(1), nullptr, 10);
+  // Extract file number - new format uses group 1, old format uses group 2
+  uint64_t first_file_num = 0;
+  uint64_t second_file_num = 0;
+
+  if (first_is_new_format) {
+    first_file_num = std::stoull(first_match.str(1), nullptr, 10);
+  } else {
+    first_file_num = std::stoull(first_match.str(2), nullptr, 10);
+  }
+
+  if (second_is_new_format) {
+    second_file_num = std::stoull(second_match.str(1), nullptr, 10);
+  } else {
+    second_file_num = std::stoull(second_match.str(2), nullptr, 10);
+  }
 
   return first_file_num < second_file_num;
 }
@@ -107,13 +136,16 @@ void Reindexer::get_bag_files(
     throw std::runtime_error("Empty directory.");
   }
 
-  std::regex regex_rule(regex_bag_pattern_, std::regex_constants::ECMAScript);
+  std::regex new_format_rule(new_format_regex_, std::regex_constants::ECMAScript);
+  std::regex old_format_rule(old_format_regex_, std::regex_constants::ECMAScript);
   // Get all file names in directory
   for (const auto & entry : fs::directory_iterator(base_folder)) {
     auto found_file = entry.path().filename();
     ROSBAG2_CPP_LOG_DEBUG_STREAM("Found file: " << found_file.generic_string());
 
-    if (std::regex_match(found_file.generic_string(), regex_rule)) {
+    if (std::regex_match(found_file.generic_string(), new_format_rule) ||
+      std::regex_match(found_file.generic_string(), old_format_rule))
+    {
       auto full_path = base_folder / found_file;
       output.emplace_back(full_path);
     }

rosbag2_cpp/src/rosbag2_cpp/writers/sequential_writer.cpp

@@ -16,8 +16,11 @@
 
 #include <algorithm>
 #include <chrono>
+#include <ctime>
 #include <filesystem>
+#include <iomanip>
 #include <memory>
+#include <regex>
 #include <stdexcept>
 #include <string>
 #include <sstream>
@@ -295,9 +298,45 @@ std::string SequentialWriter::format_storage_uri(
   // Right now `base_folder_` is always just the folder name for where to install the bagfile.
   // The name of the folder needs to be queried in case
   // SequentialWriter is opened with a relative path.
+
+  // Extract prefix from directory name by removing timestamp pattern if present
+  // This handles the case when --output is not specified and default timestamped directory is used
+  // Currently, the default timestamp format is `YYYY_MM_DD-HH_MM_SS`
+  std::string dir_name = fs::path(base_folder).filename().generic_string();
+  // Handle edge case where filename() returns empty (e.g., base_folder is "/" or ".")
+  // This should not happen in practice since base_folder is validated in open(), but
+  // we add this check for defensive programming.
+  if (dir_name.empty()) {
+    dir_name = "rosbag2";  // Use default prefix
+  }
+  static std::regex timestamp_pattern("_" + std::string(TIMESTAMP_PATTERN) + "$");
+  std::string prefix = std::regex_replace(dir_name, timestamp_pattern, "");
+
+  // Generate timestamp at file creation time
+  // Timestamp is generated in local time.
+  // During DST switches the same string may occur twice.
+  // The sequence counter is part of the filename, so duplicates
+  // still remain distinguishable.
+  auto now = std::chrono::system_clock::now();
+  auto time_t = std::chrono::system_clock::to_time_t(now);
+  std::tm tm_buf;
+#ifdef _WIN32
+  localtime_s(&tm_buf, &time_t);
+#else
+  localtime_r(&time_t, &tm_buf);
+#endif
+
+  std::stringstream timestamp_stream;
+  timestamp_stream << std::put_time(&tm_buf, "%Y_%m_%d-%H_%M_%S");
+  std::string timestamp = timestamp_stream.str();
+
+  // Generate filename in format {storage_count}_{prefix}_{timestamp}
+  // Note: Underscores are used as separators. If the prefix contains underscores,
+  // this creates theoretical ambiguity when parsing filenames. However, parsing is
+  // typically done by matching the timestamp pattern from the end, which avoids
+  // ambiguity in practice.
   std::stringstream storage_file_name;
-  storage_file_name << fs::path(base_folder).filename().generic_string() << "_" <<
-    storage_count;
+  storage_file_name << storage_count << "_" << prefix << "_" << timestamp;
 
   return (fs::path(base_folder) / storage_file_name.str()).generic_string();
 }