[esp32_ble_tracker] Refactor loop() method for improved readability and performance

esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp

@@ -49,13 +49,6 @@ void ESP32BLETracker::setup() {
     ESP_LOGE(TAG, "BLE Tracker was marked failed by ESP32BLE");
     return;
   }
-  RAMAllocator<BLEScanResult> allocator;
-  this->scan_ring_buffer_ = allocator.allocate(SCAN_RESULT_BUFFER_SIZE);
-
-  if (this->scan_ring_buffer_ == nullptr) {
-    ESP_LOGE(TAG, "Could not allocate ring buffer for BLE Tracker!");
-    this->mark_failed();
-  }
 
   global_esp32_ble_tracker = this;
 
@@ -83,124 +76,17 @@ void ESP32BLETracker::loop() {
       this->start_scan();
     }
   }
-  int connecting = 0;
+  ClientStateCounts counts = this->count_client_states_();
-  int discovered = 0;
+  if (counts != this->client_state_counts_) {
-  int searching = 0;
+    this->client_state_counts_ = counts;
-  int disconnecting = 0;
+    ESP_LOGD(TAG, "connecting: %d, discovered: %d, searching: %d, disconnecting: %d",
-  for (auto *client : this->clients_) {
+             this->client_state_counts_.connecting, this->client_state_counts_.discovered,
-    switch (client->state()) {
+             this->client_state_counts_.searching, this->client_state_counts_.disconnecting);
-      case ClientState::DISCONNECTING:
-        disconnecting++;
-        break;
-      case ClientState::DISCOVERED:
-        discovered++;
-        break;
-      case ClientState::SEARCHING:
-        searching++;
-        break;
-      case ClientState::CONNECTING:
-      case ClientState::READY_TO_CONNECT:
-        connecting++;
-        break;
-      default:
-        break;
-    }
   }
-  if (connecting != connecting_ || discovered != discovered_ || searching != searching_ ||
-      disconnecting != disconnecting_) {
-    connecting_ = connecting;
-    discovered_ = discovered;
-    searching_ = searching;
-    disconnecting_ = disconnecting;
-    ESP_LOGD(TAG, "connecting: %d, discovered: %d, searching: %d, disconnecting: %d", connecting_, discovered_,
-             searching_, disconnecting_);
-  }
-  bool promote_to_connecting = discovered && !searching && !connecting;
 
-  // Process scan results from lock-free SPSC ring buffer
-  // Consumer side: This runs in the main loop thread
-  if (this->scanner_state_ == ScannerState::RUNNING) {
-    // Load our own index with relaxed ordering (we're the only writer)
-    uint8_t read_idx = this->ring_read_index_.load(std::memory_order_relaxed);
-
-    // Load producer's index with acquire to see their latest writes
-    uint8_t write_idx = this->ring_write_index_.load(std::memory_order_acquire);
-
-    while (read_idx != write_idx) {
-      // Calculate how many contiguous results we can process in one batch
-      // If write > read: process all results from read to write
-      // If write <= read (wraparound): process from read to end of buffer first
-      size_t batch_size = (write_idx > read_idx) ? (write_idx - read_idx) : (SCAN_RESULT_BUFFER_SIZE - read_idx);
-
-      // Process the batch for raw advertisements
-      if (this->raw_advertisements_) {
-        for (auto *listener : this->listeners_) {
-          listener->parse_devices(&this->scan_ring_buffer_[read_idx], batch_size);
-        }
-        for (auto *client : this->clients_) {
-          client->parse_devices(&this->scan_ring_buffer_[read_idx], batch_size);
-        }
-      }
-
-      // Process individual results for parsed advertisements
-      if (this->parse_advertisements_) {
-#ifdef USE_ESP32_BLE_DEVICE
-        for (size_t i = 0; i < batch_size; i++) {
-          BLEScanResult &scan_result = this->scan_ring_buffer_[read_idx + i];
-          ESPBTDevice device;
-          device.parse_scan_rst(scan_result);
-
-          bool found = false;
-          for (auto *listener : this->listeners_) {
-            if (listener->parse_device(device))
-              found = true;
-          }
-
-          for (auto *client : this->clients_) {
-            if (client->parse_device(device)) {
-              found = true;
-              if (!connecting && client->state() == ClientState::DISCOVERED) {
-                promote_to_connecting = true;
-              }
-            }
-          }
-
-          if (!found && !this->scan_continuous_) {
-            this->print_bt_device_info(device);
-          }
-        }
-#endif  // USE_ESP32_BLE_DEVICE
-      }
-
-      // Update read index for entire batch
-      read_idx = (read_idx + batch_size) % SCAN_RESULT_BUFFER_SIZE;
-
-      // Store with release to ensure reads complete before index update
-      this->ring_read_index_.store(read_idx, std::memory_order_release);
-    }
-
-    // Log dropped results periodically
-    size_t dropped = this->scan_results_dropped_.exchange(0, std::memory_order_relaxed);
-    if (dropped > 0) {
-      ESP_LOGW(TAG, "Dropped %zu BLE scan results due to buffer overflow", dropped);
-    }
-  }
   if (this->scanner_state_ == ScannerState::FAILED ||
       (this->scan_set_param_failed_ && this->scanner_state_ == ScannerState::RUNNING)) {
-    this->stop_scan_();
+    this->handle_scanner_failure_();
-    if (this->scan_start_fail_count_ == std::numeric_limits<uint8_t>::max()) {
-      ESP_LOGE(TAG, "Scan could not restart after %d attempts, rebooting to restore stack (IDF)",
-               std::numeric_limits<uint8_t>::max());
-      App.reboot();
-    }
-    if (this->scan_start_failed_) {
-      ESP_LOGE(TAG, "Scan start failed: %d", this->scan_start_failed_);
-      this->scan_start_failed_ = ESP_BT_STATUS_SUCCESS;
-    }
-    if (this->scan_set_param_failed_) {
-      ESP_LOGE(TAG, "Scan set param failed: %d", this->scan_set_param_failed_);
-      this->scan_set_param_failed_ = ESP_BT_STATUS_SUCCESS;
-    }
   }
   /*
 
@@ -215,13 +101,12 @@ void ESP32BLETracker::loop() {
     https://github.com/espressif/esp-idf/issues/6688
 
   */
-  if (this->scanner_state_ == ScannerState::IDLE && !connecting && !disconnecting && !promote_to_connecting) {
+  bool promote_to_connecting = counts.discovered && !counts.searching && !counts.connecting;
+
+  if (this->scanner_state_ == ScannerState::IDLE && !counts.connecting && !counts.disconnecting &&
+      !promote_to_connecting) {
 #ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE
-    if (this->coex_prefer_ble_) {
+    this->update_coex_preference_(false);
-      this->coex_prefer_ble_ = false;
-      ESP_LOGD(TAG, "Setting coexistence preference to balanced.");
-      esp_coex_preference_set(ESP_COEX_PREFER_BALANCE);  // Reset to default
-    }
 #endif
     if (this->scan_continuous_) {
       this->start_scan_(false);  // first = false
@@ -229,31 +114,13 @@ void ESP32BLETracker::loop() {
   }
   // If there is a discovered client and no connecting
   // clients and no clients using the scanner to search for
-  // devices, then stop scanning and promote the discovered
+  // devices, then promote the discovered client to ready to connect.
-  // client to ready to connect.
+  // We check both RUNNING and IDLE states because:
+  // - RUNNING: gap_scan_event_handler initiates stop_scan_() but promotion can happen immediately
+  // - IDLE: Scanner has already stopped (naturally or by gap_scan_event_handler)
   if (promote_to_connecting &&
       (this->scanner_state_ == ScannerState::RUNNING || this->scanner_state_ == ScannerState::IDLE)) {
-    for (auto *client : this->clients_) {
+    this->try_promote_discovered_clients_();
-      if (client->state() == ClientState::DISCOVERED) {
-        if (this->scanner_state_ == ScannerState::RUNNING) {
-          ESP_LOGD(TAG, "Stopping scan to make connection");
-          this->stop_scan_();
-        } else if (this->scanner_state_ == ScannerState::IDLE) {
-          ESP_LOGD(TAG, "Promoting client to connect");
-          // We only want to promote one client at a time.
-          // once the scanner is fully stopped.
-#ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE
-          ESP_LOGD(TAG, "Setting coexistence to Bluetooth to make connection.");
-          if (!this->coex_prefer_ble_) {
-            this->coex_prefer_ble_ = true;
-            esp_coex_preference_set(ESP_COEX_PREFER_BT);  // Prioritize Bluetooth
-          }
-#endif
-          client->set_state(ClientState::READY_TO_CONNECT);
-        }
-        break;
-      }
-    }
   }
 }
 
@@ -390,31 +257,18 @@ void ESP32BLETracker::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_ga
 
 void ESP32BLETracker::gap_scan_event_handler(const BLEScanResult &scan_result) {
   // Note: This handler is called from the main loop context via esp32_ble's event queue.
-  // However, we still use a lock-free ring buffer to batch results efficiently.
+  // We process advertisements immediately instead of buffering them.
   ESP_LOGV(TAG, "gap_scan_result - event %d", scan_result.search_evt);
 
   if (scan_result.search_evt == ESP_GAP_SEARCH_INQ_RES_EVT) {
-    // Ring buffer write (Producer side)
+    // Process the scan result immediately
-    // Even though we're in the main loop, the ring buffer design allows efficient batching
+    bool found_discovered_client = this->process_scan_result_(scan_result);
-    // IMPORTANT: Only this thread writes to ring_write_index_
 
-    // Load our own index with relaxed ordering (we're the only writer)
+    // If we found a discovered client that needs promotion, stop scanning
-    uint8_t write_idx = this->ring_write_index_.load(std::memory_order_relaxed);
+    // This replaces the promote_to_connecting logic from loop()
-    uint8_t next_write_idx = (write_idx + 1) % SCAN_RESULT_BUFFER_SIZE;
+    if (found_discovered_client && this->scanner_state_ == ScannerState::RUNNING) {
-
+      ESP_LOGD(TAG, "Found discovered client, stopping scan for connection");
-    // Load consumer's index with acquire to see their latest updates
+      this->stop_scan_();
-    uint8_t read_idx = this->ring_read_index_.load(std::memory_order_acquire);
-
-    // Check if buffer is full
-    if (next_write_idx != read_idx) {
-      // Write to ring buffer
-      this->scan_ring_buffer_[write_idx] = scan_result;
-
-      // Store with release to ensure the write is visible before index update
-      this->ring_write_index_.store(next_write_idx, std::memory_order_release);
-    } else {
-      // Buffer full, track dropped results
-      this->scan_results_dropped_.fetch_add(1, std::memory_order_relaxed);
     }
   } else if (scan_result.search_evt == ESP_GAP_SEARCH_INQ_CMPL_EVT) {
     // Scan finished on its own
@@ -781,8 +635,9 @@ void ESP32BLETracker::dump_config() {
       ESP_LOGCONFIG(TAG, "  Scanner State: FAILED");
       break;
   }
-  ESP_LOGCONFIG(TAG, "  Connecting: %d, discovered: %d, searching: %d, disconnecting: %d", connecting_, discovered_,
+  ESP_LOGCONFIG(TAG, "  Connecting: %d, discovered: %d, searching: %d, disconnecting: %d",
-                searching_, disconnecting_);
+                this->client_state_counts_.connecting, this->client_state_counts_.discovered,
+                this->client_state_counts_.searching, this->client_state_counts_.disconnecting);
   if (this->scan_start_fail_count_) {
     ESP_LOGCONFIG(TAG, "  Scan Start Fail Count: %d", this->scan_start_fail_count_);
   }
@@ -859,8 +714,66 @@ bool ESPBTDevice::resolve_irk(const uint8_t *irk) const {
   return ecb_ciphertext[15] == (addr64 & 0xff) && ecb_ciphertext[14] == ((addr64 >> 8) & 0xff) &&
          ecb_ciphertext[13] == ((addr64 >> 16) & 0xff);
 }
+
+bool ESP32BLETracker::has_connecting_clients_() const {
+  for (auto *client : this->clients_) {
+    auto state = client->state();
+    if (state == ClientState::CONNECTING || state == ClientState::READY_TO_CONNECT) {
+      return true;
+    }
+  }
+  return false;
+}
 #endif  // USE_ESP32_BLE_DEVICE
 
+bool ESP32BLETracker::process_scan_result_(const BLEScanResult &scan_result) {
+  bool found_discovered_client = false;
+
+  // Process raw advertisements
+  if (this->raw_advertisements_) {
+    for (auto *listener : this->listeners_) {
+      listener->parse_devices(&scan_result, 1);
+    }
+    for (auto *client : this->clients_) {
+      client->parse_devices(&scan_result, 1);
+    }
+  }
+
+  // Process parsed advertisements
+  if (this->parse_advertisements_) {
+#ifdef USE_ESP32_BLE_DEVICE
+    ESPBTDevice device;
+    device.parse_scan_rst(scan_result);
+
+    bool found = false;
+    for (auto *listener : this->listeners_) {
+      if (listener->parse_device(device))
+        found = true;
+    }
+
+    for (auto *client : this->clients_) {
+      if (client->parse_device(device)) {
+        found = true;
+        // Check if this client is discovered and needs promotion
+        if (client->state() == ClientState::DISCOVERED) {
+          // Only check for connecting clients if we found a discovered client
+          // This matches the original logic: !connecting && client->state() == DISCOVERED
+          if (!this->has_connecting_clients_()) {
+            found_discovered_client = true;
+          }
+        }
+      }
+    }
+
+    if (!found && !this->scan_continuous_) {
+      this->print_bt_device_info(device);
+    }
+#endif  // USE_ESP32_BLE_DEVICE
+  }
+
+  return found_discovered_client;
+}
+
 void ESP32BLETracker::cleanup_scan_state_(bool is_stop_complete) {
   ESP_LOGD(TAG, "Scan %scomplete, set scanner state to IDLE.", is_stop_complete ? "stop " : "");
   this->already_discovered_.clear();
@@ -872,6 +785,61 @@ void ESP32BLETracker::cleanup_scan_state_(bool is_stop_complete) {
   this->set_scanner_state_(ScannerState::IDLE);
 }
 
+void ESP32BLETracker::handle_scanner_failure_() {
+  this->stop_scan_();
+  if (this->scan_start_fail_count_ == std::numeric_limits<uint8_t>::max()) {
+    ESP_LOGE(TAG, "Scan could not restart after %d attempts, rebooting to restore stack (IDF)",
+             std::numeric_limits<uint8_t>::max());
+    App.reboot();
+  }
+  if (this->scan_start_failed_) {
+    ESP_LOGE(TAG, "Scan start failed: %d", this->scan_start_failed_);
+    this->scan_start_failed_ = ESP_BT_STATUS_SUCCESS;
+  }
+  if (this->scan_set_param_failed_) {
+    ESP_LOGE(TAG, "Scan set param failed: %d", this->scan_set_param_failed_);
+    this->scan_set_param_failed_ = ESP_BT_STATUS_SUCCESS;
+  }
+}
+
+void ESP32BLETracker::try_promote_discovered_clients_() {
+  for (auto *client : this->clients_) {
+    if (client->state() != ClientState::DISCOVERED) {
+      continue;
+    }
+
+    if (this->scanner_state_ == ScannerState::RUNNING) {
+      ESP_LOGD(TAG, "Stopping scan to make connection");
+      this->stop_scan_();
+      // Don't wait for scan stop complete - promote immediately.
+      // This is safe because ESP-IDF processes BLE commands sequentially through its internal mailbox queue.
+      // This guarantees that the stop scan command will be fully processed before any subsequent connect command,
+      // preventing race conditions or overlapping operations.
+    }
+
+    ESP_LOGD(TAG, "Promoting client to connect");
+#ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE
+    this->update_coex_preference_(true);
+#endif
+    client->set_state(ClientState::READY_TO_CONNECT);
+    break;
+  }
+}
+
+#ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE
+void ESP32BLETracker::update_coex_preference_(bool force_ble) {
+  if (force_ble && !this->coex_prefer_ble_) {
+    ESP_LOGD(TAG, "Setting coexistence to Bluetooth to make connection.");
+    this->coex_prefer_ble_ = true;
+    esp_coex_preference_set(ESP_COEX_PREFER_BT);  // Prioritize Bluetooth
+  } else if (!force_ble && this->coex_prefer_ble_) {
+    ESP_LOGD(TAG, "Setting coexistence preference to balanced.");
+    this->coex_prefer_ble_ = false;
+    esp_coex_preference_set(ESP_COEX_PREFER_BALANCE);  // Reset to default
+  }
+}
+#endif
+
 }  // namespace esphome::esp32_ble_tracker
 
 #endif  // USE_ESP32

esphome/components/esp32_ble_tracker/esp32_ble_tracker.h

@@ -6,7 +6,6 @@
 #include "esphome/core/helpers.h"
 
 #include <array>
-#include <atomic>
 #include <string>
 #include <vector>
 
@@ -21,6 +20,7 @@
 
 #include "esphome/components/esp32_ble/ble.h"
 #include "esphome/components/esp32_ble/ble_uuid.h"
+#include "esphome/components/esp32_ble/ble_scan_result.h"
 
 namespace esphome::esp32_ble_tracker {
 
@@ -136,6 +136,18 @@ class ESPBTDeviceListener {
   ESP32BLETracker *parent_{nullptr};
 };
 
+struct ClientStateCounts {
+  uint8_t connecting = 0;
+  uint8_t discovered = 0;
+  uint8_t searching = 0;
+  uint8_t disconnecting = 0;
+
+  bool operator!=(const ClientStateCounts &other) const {
+    return connecting != other.connecting || discovered != other.discovered || searching != other.searching ||
+           disconnecting != other.disconnecting;
+  }
+};
+
 enum class ClientState : uint8_t {
   // Connection is allocated
   INIT,
@@ -272,6 +284,45 @@ class ESP32BLETracker : public Component,
   void set_scanner_state_(ScannerState state);
   /// Common cleanup logic when transitioning scanner to IDLE state
   void cleanup_scan_state_(bool is_stop_complete);
+  /// Process a single scan result immediately
+  /// Returns true if a discovered client needs promotion to READY_TO_CONNECT
+  bool process_scan_result_(const BLEScanResult &scan_result);
+#ifdef USE_ESP32_BLE_DEVICE
+  /// Check if any clients are in connecting or ready to connect state
+  bool has_connecting_clients_() const;
+#endif
+  /// Handle scanner failure states
+  void handle_scanner_failure_();
+  /// Try to promote discovered clients to ready to connect
+  void try_promote_discovered_clients_();
+#ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE
+  /// Update BLE coexistence preference
+  void update_coex_preference_(bool force_ble);
+#endif
+  /// Count clients in each state
+  ClientStateCounts count_client_states_() const {
+    ClientStateCounts counts;
+    for (auto *client : this->clients_) {
+      switch (client->state()) {
+        case ClientState::DISCONNECTING:
+          counts.disconnecting++;
+          break;
+        case ClientState::DISCOVERED:
+          counts.discovered++;
+          break;
+        case ClientState::SEARCHING:
+          counts.searching++;
+          break;
+        case ClientState::CONNECTING:
+        case ClientState::READY_TO_CONNECT:
+          counts.connecting++;
+          break;
+        default:
+          break;
+      }
+    }
+    return counts;
+  }
 
   uint8_t app_id_{0};
 
@@ -295,21 +346,9 @@ class ESP32BLETracker : public Component,
   bool raw_advertisements_{false};
   bool parse_advertisements_{false};
 
-  // Lock-free Single-Producer Single-Consumer (SPSC) ring buffer for scan results
-  // Producer: ESP-IDF Bluetooth stack callback (gap_scan_event_handler)
-  // Consumer: ESPHome main loop (loop() method)
-  // This design ensures zero blocking in the BT callback and prevents scan result loss
-  BLEScanResult *scan_ring_buffer_;
-  std::atomic<uint8_t> ring_write_index_{0};       // Written only by BT callback (producer)
-  std::atomic<uint8_t> ring_read_index_{0};        // Written only by main loop (consumer)
-  std::atomic<uint16_t> scan_results_dropped_{0};  // Tracks buffer overflow events
-
   esp_bt_status_t scan_start_failed_{ESP_BT_STATUS_SUCCESS};
   esp_bt_status_t scan_set_param_failed_{ESP_BT_STATUS_SUCCESS};
-  int connecting_{0};
+  ClientStateCounts client_state_counts_;
-  int discovered_{0};
-  int searching_{0};
-  int disconnecting_{0};
 #ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE
   bool coex_prefer_ble_{false};
 #endif