Release fixups (#72)

* GH-157 Adapt error messages of python code to C++ error format

* GH-157 Expose read ahead queue size property

* GH-157 Fixes for CTRL-C during playback

- for large read-ahead-queue-size: Error message after closing
- for small read-ahead-queue-size: CTRL-C doesn't interrupt

* GH-157 Clarify that -a or topics must be given

* GH-157 Get rid of --spin-time argument from record

* GH-157 Improve error output when info cannot read metadata

* python touchups

ros2bag/ros2bag/verb/info.py

@@ -13,7 +13,6 @@
 # limitations under the License.
 
 import os
-import sys
 
 from ros2bag.verb import VerbExtension
 
@@ -30,6 +29,6 @@ def add_arguments(self, parser, cli_name):  # noqa: D102
     def main(self, *, args):  # noqa: D102
         bag_file = args.bag_file
         if not os.path.exists(bag_file):
-            return "Error: bag file '{}' does not exist!".format(bag_file)
+            return "[ERROR] [ros2bag]: bag file '{}' does not exist!".format(bag_file)
 
         rosbag2_transport_py.info(uri=bag_file)

ros2bag/ros2bag/verb/play.py

@@ -13,7 +13,6 @@
 # limitations under the License.
 
 import os
-import sys
 
 from ros2bag.verb import VerbExtension
 
@@ -26,10 +25,21 @@ class PlayVerb(VerbExtension):
     def add_arguments(self, parser, cli_name):  # noqa: D102
         parser.add_argument(
             'bag_file', help='bag file to replay')
+        parser.add_argument(
+            '-s', '--storage', default='sqlite3',
+            help='storage identifier to be used, defaults to "sqlite3"')
+        parser.add_argument(
+            '-r', '--read-ahead-queue-size', type=int, default=1000,
+            help='size of message queue rosbag tries to hold in memory to help deterministic '
+                 'playback. Larger size will result in larger memory needs but might prevent '
+                 'delay of message playback.')
 
     def main(self, *, args):  # noqa: D102
         bag_file = args.bag_file
         if not os.path.exists(bag_file):
-            return "Error: bag file '{}' does not exist!".format(bag_file)
+            return "[ERROR] [ros2bag] bag file '{}' does not exist!".format(bag_file)
 
-        rosbag2_transport_py.play(uri=bag_file, storage_id='sqlite3')
+        rosbag2_transport_py.play(
+            uri=bag_file,
+            storage_id=args.storage,
+            read_ahead_queue_size=args.read_ahead_queue_size)

ros2bag/ros2bag/verb/record.py

@@ -14,7 +14,6 @@
 
 import datetime
 import os
-import sys
 
 from ros2bag.verb import VerbExtension
 
@@ -28,11 +27,9 @@ class RecordVerb(VerbExtension):
     """ros2 bag record."""
 
     def add_arguments(self, parser, cli_name):  # noqa: D102
-        self._subparser = parser
-
-        add_arguments(parser)
         parser.add_argument(
-            '-a', '--all', action='store_true', help='recording all topics')
+            '-a', '--all', action='store_true',
+            help='recording all topics, required if no topics are listed explicitly.')
         parser.add_argument(
             'topics', nargs='*', help='topics to be recorded')
         parser.add_argument(
@@ -59,8 +56,8 @@ def add_arguments(self, parser, cli_name):  # noqa: D102
     def create_bag_directory(self, uri):
         try:
             os.makedirs(uri)
-        except:
-            return "Error: Could not create bag folder '{}'.".format(uri)
+        except OSError:
+            return "[ERROR] [ros2bag]: Could not create bag folder '{}'.".format(uri)
 
     def main(self, *, args):  # noqa: D102
         if args.all and args.topics:
@@ -69,7 +66,7 @@ def main(self, *, args):  # noqa: D102
         uri = args.output if args.output else datetime.datetime.now().strftime("rosbag2_%Y_%m_%d-%H_%M_%S")
 
         if os.path.isdir(uri):
-            return "Error: Output folder '{}' already exists.".format(uri)
+            return "[ERROR] [ros2bag]: Output folder '{}' already exists.".format(uri)
 
         self.create_bag_directory(uri)
 

rosbag2_transport/src/rosbag2_transport/player.cpp

@@ -68,7 +68,7 @@ void Player::wait_for_filled_queue(const PlayOptions & options) const
 {
   while (
     message_queue_.size_approx() < options.read_ahead_queue_size &&
-    !is_storage_completely_loaded())
+    !is_storage_completely_loaded() && rclcpp::ok())
   {
     std::this_thread::sleep_for(queue_read_wait_period_);
   }
@@ -90,7 +90,7 @@ void Player::load_storage_content(const PlayOptions & options)
     static_cast<size_t>(options.read_ahead_queue_size * read_ahead_lower_bound_percentage_);
   auto queue_upper_boundary = options.read_ahead_queue_size;
 
-  while (reader_->has_next()) {
+  while (reader_->has_next() && rclcpp::ok()) {
     if (message_queue_.size_approx() < queue_lower_boundary) {
       enqueue_up_to_boundary(time_first_message, queue_upper_boundary);
     } else {
@@ -122,7 +122,9 @@ void Player::play_messages_from_queue()
     ReplayableMessage message;
     if (message_queue_.try_dequeue(message)) {
       std::this_thread::sleep_until(start_time + message.time_since_start);
-      publishers_[message.message->topic_name]->publish(message.message->serialized_data);
+      if (rclcpp::ok()) {
+        publishers_[message.message->topic_name]->publish(message.message->serialized_data);
+      }
     }
   }
 }

rosbag2_transport/src/rosbag2_transport/rosbag2_transport.cpp

@@ -107,7 +107,8 @@ void Rosbag2Transport::print_bag_info(const std::string & uri)
   } catch (std::runtime_error & e) {
     (void) e;
     ROSBAG2_TRANSPORT_LOG_ERROR_STREAM("Could not read metadata for " << uri << ". Please specify "
-      "the path to the folder containing an existing 'metadata.yaml' file");
+      "the path to the folder containing an existing 'metadata.yaml' file and make sure that the "
+      "file 'metadata.yaml' exists and has the correct format.");
     return;
   }
 

rosbag2_transport/src/rosbag2_transport/rosbag2_transport_python.cpp

@@ -109,7 +109,7 @@ rosbag2_transport_play(PyObject * Py_UNUSED(self), PyObject * args, PyObject * k
   storage_options.uri = std::string(uri);
   storage_options.storage_id = std::string(storage_id);
 
-  play_options.read_ahead_queue_size = read_ahead_queue_size ? read_ahead_queue_size : 1000;
+  play_options.read_ahead_queue_size = read_ahead_queue_size;
 
   rosbag2_transport::Rosbag2Transport transport;
   transport.init();