comments

Signed-off-by: Nicola Loi <[email protected]>

rosbag2_transport/src/rosbag2_transport/player.cpp

@@ -317,13 +317,19 @@ class PlayerImpl
   std::atomic_bool is_delayed_status_{false};
 
   void print_progress_bar_help_str() const;
-  // Call update_progress_bar_after_message_published function only if the function cannot run
+  // Update progress bar taking into account the frequency set by the user.
+  // The function should be called for regular progress bar updates, for example
+  // after the recurrent message publishing.
+  // Call update_progress_bar_check_frequency function only if the function cannot run
   // contemporaneously in multiple threads, i.e. function calls are already protected by a mutex:
   // to avoid locking overhead no new mutex inside the function is directly protecting
   // the access to the class attribute progress_bar_last_time_updated_.
-  void update_progress_bar_after_message_published(
+  void update_progress_bar_check_frequency(
     const rcutils_time_point_value_t & timestamp,
     const PlayerStatus & status);
+  // Update progress bar irrespective of the frequency set by the user.
+  // The function should be called for extraordinary progress bar updates, for example
+  // when a log message is printed and we want to 'redraw' the progress bar.
   void update_progress_bar(const PlayerStatus & status) const;
   void cout_progress_bar(
     const rcutils_time_point_value_t & timestamp,
@@ -628,6 +634,8 @@ bool PlayerImpl::play()
         ready_to_play_from_queue_cv_.notify_all();
       }
 
+      update_progress_bar(clock_->is_paused() ? PlayerStatus::PAUSED : PlayerStatus::RUNNING);
+
       // Wait for all published messages to be acknowledged.
       if (play_options_.wait_acked_timeout >= 0) {
         std::chrono::milliseconds timeout(play_options_.wait_acked_timeout);
@@ -702,6 +710,8 @@ void PlayerImpl::stop()
       cancel_wait_for_next_message_ = true;
     }
 
+    update_progress_bar(clock_->is_paused() ? PlayerStatus::PAUSED : PlayerStatus::RUNNING);
+
     if (clock_->is_paused()) {
       clock_->resume();  // Temporary resume clock to force wakeup in clock_->sleep_until(time)
       clock_->pause();   // Return in pause mode to preserve original state of the player
@@ -1098,10 +1108,10 @@ void PlayerImpl::play_messages_from_queue()
             finished_play_next_cv_.notify_all();
           }
         } else {
-          // update_progress_bar_after_message_published in this code section is protected
+          // update_progress_bar_check_frequency in this code section is protected
           // by the mutex skip_message_in_main_play_loop_mutex_.
-          update_progress_bar_after_message_published(
-            message_ptr->recv_timestamp, PlayerStatus::RUNNING);
+          update_progress_bar_check_frequency(
+             message_ptr->recv_timestamp, PlayerStatus::RUNNING);
         }
       }
       message_ptr = take_next_message_from_queue();
@@ -1728,7 +1738,7 @@ void PlayerImpl::print_progress_bar_help_str() const
     } else {
       std::ostringstream oss;
       oss << "Progress bar enabled at " <<
-        play_options_.progress_bar_print_frequency << " Hz.";
+        play_options_.progress_bar_print_frequency << " Hz";
       help_str = oss.str();
     }
     RCLCPP_INFO_STREAM(owner_->get_logger(), help_str);
@@ -1740,14 +1750,16 @@ void PlayerImpl::print_progress_bar_help_str() const
   }
 }
 
-void PlayerImpl::update_progress_bar_after_message_published(
+void PlayerImpl::update_progress_bar_check_frequency(
   const rcutils_time_point_value_t & timestamp,
   const PlayerStatus & status)
 {
   if (!enable_progress_bar_) {
     return;
   }
 
+  // If we are not updating the progress bar for every call, check if we should update it now
+  // based on the frequency set by the user
   if (!progress_bar_update_always_) {
     std::chrono::steady_clock::time_point steady_time_now = std::chrono::steady_clock::now();
     if (std::chrono::duration_cast<std::chrono::nanoseconds>(
@@ -1767,6 +1779,8 @@ void PlayerImpl::update_progress_bar(const PlayerStatus & status) const
     return;
   }
 
+  // Update progress bar irrespective of the frequency set by the user.
+  // Overwrite the input status if we are in a special case.
   if (is_delayed_status_) {
     cout_progress_bar(starting_time_, PlayerStatus::DELAYED);
   } else {
@@ -1784,7 +1798,7 @@ void PlayerImpl::cout_progress_bar(
   std::ostringstream oss;
   oss << " Bag Time " << std::setw(13) << std::fixed << std::setprecision(6) <<
     current_time_secs << " Duration " << std::setprecision(3) << progress_secs <<
-    "/" << duration_secs_ << " [" << static_cast<char>(status) << "]\r";
+    "/" << duration_secs_ << " [" << static_cast<char>(status) << "]    \r";
   std::cout << oss.str() << std::flush;
 }