From 739cc5ff500ca42faba465f928d2b477b6dd9ad5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 4 Aug 2025 09:24:22 -1000 Subject: [PATCH] [esp32_ble_tracker] Refactor loop() method for improved readability and performance --- .../esp32_ble_tracker/esp32_ble_tracker.cpp | 314 ++++++++---------- .../esp32_ble_tracker/esp32_ble_tracker.h | 67 +++- 2 files changed, 194 insertions(+), 187 deletions(-) diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 254eddd1d9..9143f25a25 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/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 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; - int discovered = 0; - int searching = 0; - int disconnecting = 0; - for (auto *client : this->clients_) { - switch (client->state()) { - 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; - } + ClientStateCounts counts = this->count_client_states_(); + if (counts != this->client_state_counts_) { + this->client_state_counts_ = counts; + ESP_LOGD(TAG, "connecting: %d, discovered: %d, searching: %d, disconnecting: %d", + this->client_state_counts_.connecting, this->client_state_counts_.discovered, + this->client_state_counts_.searching, this->client_state_counts_.disconnecting); } - 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_(); - if (this->scan_start_fail_count_ == std::numeric_limits::max()) { - ESP_LOGE(TAG, "Scan could not restart after %d attempts, rebooting to restore stack (IDF)", - std::numeric_limits::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; - } + this->handle_scanner_failure_(); } /* @@ -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->coex_prefer_ble_ = false; - ESP_LOGD(TAG, "Setting coexistence preference to balanced."); - esp_coex_preference_set(ESP_COEX_PREFER_BALANCE); // Reset to default - } + this->update_coex_preference_(false); #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 - // client to ready to connect. + // devices, then promote the discovered 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_) { - 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; - } - } + this->try_promote_discovered_clients_(); } } @@ -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) - // Even though we're in the main loop, the ring buffer design allows efficient batching - // IMPORTANT: Only this thread writes to ring_write_index_ + // Process the scan result immediately + bool found_discovered_client = this->process_scan_result_(scan_result); - // Load our own index with relaxed ordering (we're the only writer) - uint8_t write_idx = this->ring_write_index_.load(std::memory_order_relaxed); - uint8_t next_write_idx = (write_idx + 1) % SCAN_RESULT_BUFFER_SIZE; - - // Load consumer's index with acquire to see their latest updates - 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); + // If we found a discovered client that needs promotion, stop scanning + // This replaces the promote_to_connecting logic from loop() + if (found_discovered_client && this->scanner_state_ == ScannerState::RUNNING) { + ESP_LOGD(TAG, "Found discovered client, stopping scan for connection"); + this->stop_scan_(); } } 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_, - searching_, disconnecting_); + ESP_LOGCONFIG(TAG, " Connecting: %d, discovered: %d, searching: %d, disconnecting: %d", + 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::max()) { + ESP_LOGE(TAG, "Scan could not restart after %d attempts, rebooting to restore stack (IDF)", + std::numeric_limits::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 diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index c274e64b12..77020d3222 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -6,7 +6,6 @@ #include "esphome/core/helpers.h" #include -#include #include #include @@ -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 ring_write_index_{0}; // Written only by BT callback (producer) - std::atomic ring_read_index_{0}; // Written only by main loop (consumer) - std::atomic 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}; - int discovered_{0}; - int searching_{0}; - int disconnecting_{0}; + ClientStateCounts client_state_counts_; #ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE bool coex_prefer_ble_{false}; #endif