From 8142813215ca383696945cacb38c82659dd9b4c8 Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Fri, 2 Oct 2020 20:27:29 +0000 Subject: [PATCH 1/5] Change default value for max-cache-size to 1MB Signed-off-by: Jaison Titus --- ros2bag/ros2bag/verb/record.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ros2bag/ros2bag/verb/record.py b/ros2bag/ros2bag/verb/record.py index 41a7a2f11..760429614 100644 --- a/ros2bag/ros2bag/verb/record.py +++ b/ros2bag/ros2bag/verb/record.py @@ -67,9 +67,10 @@ def add_arguments(self, parser, cli_name): # noqa: D102 'the bag will split at whichever threshold is reached first.' ) parser.add_argument( - '--max-cache-size', type=int, default=0, - help='maximum amount of messages to hold in cache before writing to disk. ' - 'Default it is zero, writing every message directly to disk.' + '--max-cache-size', type=int, default=1024*1024, + help='maximum size (in bytes) of messages to hold in cache before writing to disk. ' + 'Default is 1 mebibyte, everytime the cache size equals or exceeds 1MB, ' + 'it will be written to disk' ) parser.add_argument( '--compression-mode', type=str, default='none', From 2106ee5953bd0a7a9a321262154cf60759401707 Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Fri, 2 Oct 2020 20:45:27 +0000 Subject: [PATCH 2/5] Better help message ros2bag record Signed-off-by: Jaison Titus --- ros2bag/ros2bag/verb/record.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ros2bag/ros2bag/verb/record.py b/ros2bag/ros2bag/verb/record.py index 760429614..ec41a6714 100644 --- a/ros2bag/ros2bag/verb/record.py +++ b/ros2bag/ros2bag/verb/record.py @@ -70,7 +70,8 @@ def add_arguments(self, parser, cli_name): # noqa: D102 '--max-cache-size', type=int, default=1024*1024, help='maximum size (in bytes) of messages to hold in cache before writing to disk. ' 'Default is 1 mebibyte, everytime the cache size equals or exceeds 1MB, ' - 'it will be written to disk' + 'it will be written to disk. If the value specified is 0, then every message is' + 'directly written to disk.' ) parser.add_argument( '--compression-mode', type=str, default='none', From aebe367830e7eb2ccab2c161cb9ba77d7d130668 Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Mon, 5 Oct 2020 21:03:55 +0000 Subject: [PATCH 3/5] Fix test which assumes --max-cache-size to be 0 Signed-off-by: Jaison Titus --- .../test/rosbag2_tests/test_rosbag2_record_end_to_end.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rosbag2_tests/test/rosbag2_tests/test_rosbag2_record_end_to_end.cpp b/rosbag2_tests/test/rosbag2_tests/test_rosbag2_record_end_to_end.cpp index aa3f18ae2..87940510b 100644 --- a/rosbag2_tests/test/rosbag2_tests/test_rosbag2_record_end_to_end.cpp +++ b/rosbag2_tests/test/rosbag2_tests/test_rosbag2_record_end_to_end.cpp @@ -67,6 +67,7 @@ TEST_F(RecordFixture, record_end_to_end_test_with_zstd_file_compression) { cmd << "ros2 bag record" << " --compression-mode file" << " --compression-format zstd" << + " --max-cache-size 0" << " --output " << root_bag_path_.string() << " " << topic_name; @@ -117,7 +118,7 @@ TEST_F(RecordFixture, record_end_to_end_test) { wrong_message->string_value = "wrong_content"; auto process_handle = start_execution( - "ros2 bag record --output " + root_bag_path_.string() + " /test_topic"); + "ros2 bag record --max-cache-size 0 --output " + root_bag_path_.string() + " /test_topic"); wait_for_db(); pub_man_.add_publisher("/test_topic", message, expected_test_messages); From ce408f307042eadc387ee349765b17ac0a0be16d Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Wed, 7 Oct 2020 17:58:34 +0000 Subject: [PATCH 4/5] Remove mention of default value from comments about max_cache_size Signed-off-by: Jaison Titus --- rosbag2_cpp/include/rosbag2_cpp/storage_options.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rosbag2_cpp/include/rosbag2_cpp/storage_options.hpp b/rosbag2_cpp/include/rosbag2_cpp/storage_options.hpp index 57793e1fb..4337dc45d 100644 --- a/rosbag2_cpp/include/rosbag2_cpp/storage_options.hpp +++ b/rosbag2_cpp/include/rosbag2_cpp/storage_options.hpp @@ -36,7 +36,7 @@ struct StorageOptions // The cache size indiciates how many messages can maximally be hold in cache // before these being written to disk. - // Defaults to 0, and effectively disables the caching. + // A value of 0 disables caching and every write happens directly to disk. uint64_t max_cache_size = 0; }; From 044c6644d77745a63fe2ae77e563991636671b50 Mon Sep 17 00:00:00 2001 From: Jaison Titus Date: Wed, 7 Oct 2020 17:59:17 +0000 Subject: [PATCH 5/5] Remove unnecessary assignment of default values for storageoptions in open Signed-off-by: Jaison Titus --- rosbag2_cpp/src/rosbag2_cpp/reader.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/rosbag2_cpp/src/rosbag2_cpp/reader.cpp b/rosbag2_cpp/src/rosbag2_cpp/reader.cpp index 08cfd1c3b..ef2670fb8 100644 --- a/rosbag2_cpp/src/rosbag2_cpp/reader.cpp +++ b/rosbag2_cpp/src/rosbag2_cpp/reader.cpp @@ -40,8 +40,6 @@ void Reader::open(const std::string & uri) rosbag2_cpp::StorageOptions storage_options; storage_options.uri = uri; storage_options.storage_id = "sqlite3"; - storage_options.max_bagfile_size = 0; // default - storage_options.max_cache_size = 0; // default rosbag2_cpp::ConverterOptions converter_options{}; return open(storage_options, converter_options);