Bugfix: Recorder discovery does not restart after being stopped (#1894)

* Invert start discovery condition Repeated calls to start_discovery should not start additional topics discovery tasks without stop_discovery calls Signed-off-by: Øystein Sture <[email protected]> * Rename stop_discovery to discovery_running and change logic accordingly Signed-off-by: Øystein Sture <[email protected]> --------- Signed-off-by: Øystein Sture <[email protected]> (cherry picked from commit b483541)
ros2 · Jan 22, 2025 · 63dd4fa · 63dd4fa
1 parent 02f7b89
commit 63dd4fa
Showing 1 changed file with 8 additions and 8 deletions.
diff --git a/rosbag2_transport/src/rosbag2_transport/recorder.cpp b/rosbag2_transport/src/rosbag2_transport/recorder.cpp
@@ -143,7 +143,7 @@ class RecorderImpl
 
   std::mutex start_stop_transition_mutex_;
   std::mutex discovery_mutex_;
-  std::atomic<bool> stop_discovery_ = false;
+  std::atomic<bool> discovery_running_ = false;
   std::atomic_uchar paused_ = 0;
   std::atomic<bool> in_recording_ = false;
   std::shared_ptr<KeyboardHandler> keyboard_handler_;
@@ -436,7 +436,7 @@ bool RecorderImpl::is_paused()
 void RecorderImpl::start_discovery()
 {
   std::lock_guard<std::mutex> state_lock(discovery_mutex_);
-  if (stop_discovery_.exchange(false)) {
+  if (discovery_running_.exchange(true)) {
     RCLCPP_DEBUG(node->get_logger(), "Recorder topic discovery is already running.");
   } else {
     discovery_future_ =
@@ -447,10 +447,7 @@ void RecorderImpl::start_discovery()
 void RecorderImpl::stop_discovery()
 {
   std::lock_guard<std::mutex> state_lock(discovery_mutex_);
-  if (stop_discovery_.exchange(true)) {
-    RCLCPP_DEBUG(
-      node->get_logger(), "Recorder topic discovery has already been stopped or not running.");
-  } else {
+  if (discovery_running_.exchange(false)) {
     if (discovery_future_.valid()) {
       auto status = discovery_future_.wait_for(2 * record_options_.topic_polling_interval);
       if (status != std::future_status::ready) {
@@ -461,6 +458,9 @@ void RecorderImpl::stop_discovery()
             (status == std::future_status::timeout ? "timeout" : "deferred"));
       }
     }
+  } else {
+    RCLCPP_DEBUG(
+      node->get_logger(), "Recorder topic discovery has already been stopped or not running.");
   }
 }
 
@@ -471,7 +471,7 @@ void RecorderImpl::topics_discovery()
     RCLCPP_INFO(
       node->get_logger(),
       "use_sim_time set, waiting for /clock before starting recording...");
-    while (rclcpp::ok() && stop_discovery_ == false) {
+    while (rclcpp::ok() && discovery_running_) {
       if (node->get_clock()->wait_until_started(record_options_.topic_polling_interval)) {
         break;
       }
@@ -480,7 +480,7 @@ void RecorderImpl::topics_discovery()
       RCLCPP_INFO(node->get_logger(), "Sim time /clock found, starting recording.");
     }
   }
-  while (rclcpp::ok() && stop_discovery_ == false) {
+  while (rclcpp::ok() && discovery_running_) {
     try {
       auto topics_to_subscribe = get_requested_or_available_topics();
       for (const auto & topic_and_type : topics_to_subscribe) {