From 1eec1239ec10a186836529301dd2f94611a2b1c9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 09:56:02 -0500 Subject: [PATCH 01/33] wip --- .../bluetooth_proxy/bluetooth_proxy.cpp | 4 +- .../bluetooth_proxy/bluetooth_proxy.h | 2 +- esphome/components/esp32_ble/ble.cpp | 52 +++++++- esphome/components/esp32_ble/ble.h | 28 +++++ esphome/components/esp32_ble/ble_event.h | 78 ++++++++++-- esphome/components/esp32_ble/queue.h | 4 + .../components/esp32_ble_tracker/__init__.py | 1 + .../esp32_ble_tracker/esp32_ble_tracker.cpp | 113 ++++++++++++------ .../esp32_ble_tracker/esp32_ble_tracker.h | 14 +-- 9 files changed, 235 insertions(+), 61 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp index 7aeb818306..fbe2a3e67c 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp @@ -58,7 +58,7 @@ static std::vector &get_batch_buffer() { return batch_buffer; } -bool BluetoothProxy::parse_devices(esp_ble_gap_cb_param_t::ble_scan_result_evt_param *advertisements, size_t count) { +bool BluetoothProxy::parse_devices(const esp32_ble::BLEScanResult *scan_results, size_t count) { if (!api::global_api_server->is_connected() || this->api_connection_ == nullptr || !this->raw_advertisements_) return false; @@ -73,7 +73,7 @@ bool BluetoothProxy::parse_devices(esp_ble_gap_cb_param_t::ble_scan_result_evt_p // Add new advertisements to the batch buffer for (size_t i = 0; i < count; i++) { - auto &result = advertisements[i]; + auto &result = scan_results[i]; uint8_t length = result.adv_data_len + result.scan_rsp_len; batch_buffer.emplace_back(); diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.h b/esphome/components/bluetooth_proxy/bluetooth_proxy.h index f75e73e796..16db0a0a11 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.h +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.h @@ -52,7 +52,7 @@ class BluetoothProxy : public esp32_ble_tracker::ESPBTDeviceListener, public Com public: BluetoothProxy(); bool parse_device(const esp32_ble_tracker::ESPBTDevice &device) override; - bool parse_devices(esp_ble_gap_cb_param_t::ble_scan_result_evt_param *advertisements, size_t count) override; + bool parse_devices(const esp32_ble::BLEScanResult *scan_results, size_t count) override; void dump_config() override; void setup() override; void loop() override; diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 824c2b9dbc..24566e8f6d 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -312,9 +312,36 @@ void ESP32BLE::loop() { this->real_gattc_event_handler_(ble_event->event_.gattc.gattc_event, ble_event->event_.gattc.gattc_if, &ble_event->event_.gattc.gattc_param); break; - case BLEEvent::GAP: - this->real_gap_event_handler_(ble_event->event_.gap.gap_event, &ble_event->event_.gap.gap_param); + case BLEEvent::GAP: { + esp_gap_ble_cb_event_t gap_event = ble_event->event_.gap.gap_event; + if (gap_event == ESP_GAP_BLE_SCAN_RESULT_EVT) { + // Use the new scan event handler - no memcpy! + for (auto *scan_handler : this->gap_scan_event_handlers_) { + scan_handler->gap_scan_event_handler(ble_event->scan_result()); + } + } else if (gap_event == ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT || + gap_event == ESP_GAP_BLE_SCAN_START_COMPLETE_EVT || + gap_event == ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { + // Create temporary param for scan complete events + esp_ble_gap_cb_param_t param; + memset(¶m, 0, sizeof(param)); + + // Set the appropriate status field based on event type + if (gap_event == ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT) { + param.scan_param_cmpl.status = ble_event->event_.gap.scan_complete.status; + } else if (gap_event == ESP_GAP_BLE_SCAN_START_COMPLETE_EVT) { + param.scan_start_cmpl.status = ble_event->event_.gap.scan_complete.status; + } else if (gap_event == ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { + param.scan_stop_cmpl.status = ble_event->event_.gap.scan_complete.status; + } + + this->real_gap_event_handler_(gap_event, ¶m); + } else { + // Fallback for unexpected events (uses full param copy) + this->real_gap_event_handler_(gap_event, &ble_event->event_.gap.gap_param); + } break; + } default: break; } @@ -328,6 +355,13 @@ void ESP32BLE::loop() { } void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { + static constexpr size_t MAX_BLE_QUEUE_SIZE = SCAN_RESULT_BUFFER_SIZE * 2; + + if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { + ESP_LOGW(TAG, "BLE event queue full (%d), dropping GAP event %d", MAX_BLE_QUEUE_SIZE, event); + return; + } + BLEEvent *new_event = EVENT_ALLOCATOR.allocate(1); if (new_event == nullptr) { // Memory too fragmented to allocate new event. Can only drop it until memory comes back @@ -346,6 +380,13 @@ void ESP32BLE::real_gap_event_handler_(esp_gap_ble_cb_event_t event, esp_ble_gap void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) { + static constexpr size_t MAX_BLE_QUEUE_SIZE = SCAN_RESULT_BUFFER_SIZE * 2; + + if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { + ESP_LOGW(TAG, "BLE event queue full (%d), dropping GATTS event %d", MAX_BLE_QUEUE_SIZE, event); + return; + } + BLEEvent *new_event = EVENT_ALLOCATOR.allocate(1); if (new_event == nullptr) { // Memory too fragmented to allocate new event. Can only drop it until memory comes back @@ -365,6 +406,13 @@ void ESP32BLE::real_gatts_event_handler_(esp_gatts_cb_event_t event, esp_gatt_if void ESP32BLE::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) { + static constexpr size_t MAX_BLE_QUEUE_SIZE = SCAN_RESULT_BUFFER_SIZE * 2; + + if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { + ESP_LOGW(TAG, "BLE event queue full (%d), dropping GATTC event %d", MAX_BLE_QUEUE_SIZE, event); + return; + } + BLEEvent *new_event = EVENT_ALLOCATOR.allocate(1); if (new_event == nullptr) { // Memory too fragmented to allocate new event. Can only drop it until memory comes back diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index 13ec3b6dd9..c43ec8c7ed 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -22,6 +22,13 @@ namespace esphome { namespace esp32_ble { +// Maximum number of BLE scan results to buffer +#ifdef USE_PSRAM +static constexpr uint8_t SCAN_RESULT_BUFFER_SIZE = 32; +#else +static constexpr uint8_t SCAN_RESULT_BUFFER_SIZE = 20; +#endif + uint64_t ble_addr_to_uint64(const esp_bd_addr_t address); // NOLINTNEXTLINE(modernize-use-using) @@ -57,6 +64,23 @@ class GAPEventHandler { virtual void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) = 0; }; +// Structure for BLE scan results - only fields we actually use +struct BLEScanResult { + esp_bd_addr_t bda; + uint8_t ble_addr_type; + int8_t rssi; + uint8_t ble_adv[ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX]; + uint8_t adv_data_len; + uint8_t scan_rsp_len; + uint8_t search_evt; +}; // ~73 bytes vs ~400 bytes for full esp_ble_gap_cb_param_t + +class GAPScanEventHandler { + public: + // Receives scan results directly without memcpy + virtual void gap_scan_event_handler(const BLEScanResult &scan_result) = 0; +}; + class GATTcEventHandler { public: virtual void gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, @@ -101,6 +125,9 @@ class ESP32BLE : public Component { void advertising_register_raw_advertisement_callback(std::function &&callback); void register_gap_event_handler(GAPEventHandler *handler) { this->gap_event_handlers_.push_back(handler); } + void register_gap_scan_event_handler(GAPScanEventHandler *handler) { + this->gap_scan_event_handlers_.push_back(handler); + } void register_gattc_event_handler(GATTcEventHandler *handler) { this->gattc_event_handlers_.push_back(handler); } void register_gatts_event_handler(GATTsEventHandler *handler) { this->gatts_event_handlers_.push_back(handler); } void register_ble_status_event_handler(BLEStatusEventHandler *handler) { @@ -123,6 +150,7 @@ class ESP32BLE : public Component { void advertising_init_(); std::vector gap_event_handlers_; + std::vector gap_scan_event_handlers_; std::vector gattc_event_handlers_; std::vector gatts_event_handlers_; std::vector ble_status_event_handlers_; diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 1cf63b2fab..451c52b114 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -10,21 +10,56 @@ namespace esphome { namespace esp32_ble { + // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). -// This class stores each event in a single type. +// This class stores each event with minimal memory usage by only copying the data we actually need. class BLEEvent { public: BLEEvent(esp_gap_ble_cb_event_t e, esp_ble_gap_cb_param_t *p) { - this->event_.gap.gap_event = e; - memcpy(&this->event_.gap.gap_param, p, sizeof(esp_ble_gap_cb_param_t)); this->type_ = GAP; + this->event_.gap.gap_event = e; + + // Only copy the data we actually use for each GAP event type + switch (e) { + case ESP_GAP_BLE_SCAN_RESULT_EVT: + // Copy only the fields we use from scan results (~72 bytes) + memcpy(this->event_.gap.scan_result.bda, p->scan_rst.bda, sizeof(esp_bd_addr_t)); + this->event_.gap.scan_result.ble_addr_type = p->scan_rst.ble_addr_type; + this->event_.gap.scan_result.rssi = p->scan_rst.rssi; + this->event_.gap.scan_result.adv_data_len = p->scan_rst.adv_data_len; + this->event_.gap.scan_result.scan_rsp_len = p->scan_rst.scan_rsp_len; + this->event_.gap.scan_result.search_evt = p->scan_rst.search_evt; + memcpy(this->event_.gap.scan_result.ble_adv, p->scan_rst.ble_adv, + ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX); + break; + + case ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT: + this->event_.gap.scan_complete.status = p->scan_param_cmpl.status; + break; + + case ESP_GAP_BLE_SCAN_START_COMPLETE_EVT: + this->event_.gap.scan_complete.status = p->scan_start_cmpl.status; + break; + + case ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT: + this->event_.gap.scan_complete.status = p->scan_stop_cmpl.status; + break; + + default: + // For any other GAP events, copy the full param + // This is a safety fallback but shouldn't happen in normal operation + memcpy(&this->event_.gap.gap_param, p, sizeof(esp_ble_gap_cb_param_t)); + break; + } }; BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { + this->type_ = GATTC; this->event_.gattc.gattc_event = e; this->event_.gattc.gattc_if = i; memcpy(&this->event_.gattc.gattc_param, p, sizeof(esp_ble_gattc_cb_param_t)); - // Need to also make a copy of relevant event data. + + // Copy data for events that need it switch (e) { case ESP_GATTC_NOTIFY_EVT: this->data.assign(p->notify.value, p->notify.value + p->notify.value_len); @@ -38,14 +73,15 @@ class BLEEvent { default: break; } - this->type_ = GATTC; }; BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) { + this->type_ = GATTS; this->event_.gatts.gatts_event = e; this->event_.gatts.gatts_if = i; memcpy(&this->event_.gatts.gatts_param, p, sizeof(esp_ble_gatts_cb_param_t)); - // Need to also make a copy of relevant event data. + + // Copy data for events that need it switch (e) { case ESP_GATTS_WRITE_EVT: this->data.assign(p->write.value, p->write.value + p->write.len); @@ -54,39 +90,55 @@ class BLEEvent { default: break; } - this->type_ = GATTS; }; union { // NOLINTNEXTLINE(readability-identifier-naming) struct gap_event { esp_gap_ble_cb_event_t gap_event; - esp_ble_gap_cb_param_t gap_param; - } gap; + union { + BLEScanResult scan_result; // ~73 bytes + + // Minimal storage for scan complete events + struct { + esp_bt_status_t status; + } scan_complete; // 1 byte + + // Fallback for unexpected events (shouldn't be used) + esp_ble_gap_cb_param_t gap_param; + }; + } gap; // ~80 bytes instead of 400+ // NOLINTNEXTLINE(readability-identifier-naming) struct gattc_event { esp_gattc_cb_event_t gattc_event; esp_gatt_if_t gattc_if; esp_ble_gattc_cb_param_t gattc_param; - } gattc; + } gattc; // ~68 bytes // NOLINTNEXTLINE(readability-identifier-naming) struct gatts_event { esp_gatts_cb_event_t gatts_event; esp_gatt_if_t gatts_if; esp_ble_gatts_cb_param_t gatts_param; - } gatts; - } event_; + } gatts; // ~68 bytes + } event_; // Union size is now ~80 bytes (largest member) + + std::vector data{}; // For GATTC/GATTS data - std::vector data{}; // NOLINTNEXTLINE(readability-identifier-naming) enum ble_event_t : uint8_t { GAP, GATTC, GATTS, } type_; + + // Helper methods to access event data + esp_gap_ble_cb_event_t gap_event_type() const { return event_.gap.gap_event; } + const BLEScanResult &scan_result() const { return event_.gap.scan_result; } + esp_bt_status_t scan_complete_status() const { return event_.gap.scan_complete.status; } }; +// Total size: ~110 bytes instead of 440 bytes! } // namespace esp32_ble } // namespace esphome diff --git a/esphome/components/esp32_ble/queue.h b/esphome/components/esp32_ble/queue.h index c98477e121..afa9a9b668 100644 --- a/esphome/components/esp32_ble/queue.h +++ b/esphome/components/esp32_ble/queue.h @@ -45,6 +45,10 @@ template class Queue { return element; } + size_t size() const { + return q_.size(); // Atomic read, no lock needed + } + protected: std::queue q_; SemaphoreHandle_t m_; diff --git a/esphome/components/esp32_ble_tracker/__init__.py b/esphome/components/esp32_ble_tracker/__init__.py index 61eed1c029..2242d709a4 100644 --- a/esphome/components/esp32_ble_tracker/__init__.py +++ b/esphome/components/esp32_ble_tracker/__init__.py @@ -268,6 +268,7 @@ async def to_code(config): parent = await cg.get_variable(config[esp32_ble.CONF_BLE_ID]) cg.add(parent.register_gap_event_handler(var)) + cg.add(parent.register_gap_scan_event_handler(var)) cg.add(parent.register_gattc_event_handler(var)) cg.add(parent.register_ble_status_event_handler(var)) cg.add(var.set_parent(parent)) diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 6d60f1638c..09fe451a3c 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -50,9 +50,8 @@ void ESP32BLETracker::setup() { ESP_LOGE(TAG, "BLE Tracker was marked failed by ESP32BLE"); return; } - ExternalRAMAllocator allocator( - ExternalRAMAllocator::ALLOW_FAILURE); - this->scan_result_buffer_ = allocator.allocate(ESP32BLETracker::SCAN_RESULT_BUFFER_SIZE); + ExternalRAMAllocator allocator(ExternalRAMAllocator::ALLOW_FAILURE); + this->scan_result_buffer_ = allocator.allocate(SCAN_RESULT_BUFFER_SIZE); if (this->scan_result_buffer_ == nullptr) { ESP_LOGE(TAG, "Could not allocate buffer for BLE Tracker!"); @@ -140,7 +139,24 @@ void ESP32BLETracker::loop() { if (this->parse_advertisements_) { for (size_t i = 0; i < index; i++) { ESPBTDevice device; - device.parse_scan_rst(this->scan_result_buffer_[i]); + // Convert BLEScanResult to ESP-IDF format for parse_scan_rst + esp_ble_gap_cb_param_t::ble_scan_result_evt_param param; + memcpy(param.bda, this->scan_result_buffer_[i].bda, sizeof(esp_bd_addr_t)); + param.ble_addr_type = this->scan_result_buffer_[i].ble_addr_type; + param.rssi = this->scan_result_buffer_[i].rssi; + param.adv_data_len = this->scan_result_buffer_[i].adv_data_len; + param.scan_rsp_len = this->scan_result_buffer_[i].scan_rsp_len; + param.search_evt = this->scan_result_buffer_[i].search_evt; + memcpy(param.ble_adv, this->scan_result_buffer_[i].ble_adv, + ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX); + // Fill in fields we don't store + param.dev_type = 0; + param.ble_evt_type = 0; + param.flag = 0; + param.num_resps = 1; + param.num_dis = 0; + + device.parse_scan_rst(param); bool found = false; for (auto *listener : this->listeners_) { @@ -371,7 +387,7 @@ void ESP32BLETracker::recalculate_advertisement_parser_types() { void ESP32BLETracker::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { switch (event) { case ESP_GAP_BLE_SCAN_RESULT_EVT: - this->gap_scan_result_(param->scan_rst); + // This will be handled by gap_scan_event_handler instead break; case ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT: this->gap_scan_set_param_complete_(param->scan_param_cmpl); @@ -385,8 +401,63 @@ void ESP32BLETracker::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_ga default: break; } - for (auto *client : this->clients_) { - client->gap_event_handler(event, param); + // Still forward non-scan events to clients + if (event != ESP_GAP_BLE_SCAN_RESULT_EVT) { + for (auto *client : this->clients_) { + client->gap_event_handler(event, param); + } + } +} + +void ESP32BLETracker::gap_scan_event_handler(const BLEScanResult &scan_result) { + ESP_LOGV(TAG, "gap_scan_result - event %d", scan_result.search_evt); + + if (scan_result.search_evt == ESP_GAP_SEARCH_INQ_RES_EVT) { + if (xSemaphoreTake(this->scan_result_lock_, 0)) { + if (this->scan_result_index_ < SCAN_RESULT_BUFFER_SIZE) { + // Store BLEScanResult directly in our buffer + this->scan_result_buffer_[this->scan_result_index_++] = scan_result; + } + xSemaphoreGive(this->scan_result_lock_); + } + } else if (scan_result.search_evt == ESP_GAP_SEARCH_INQ_CMPL_EVT) { + // Scan finished on its own + if (this->scanner_state_ != ScannerState::RUNNING) { + if (this->scanner_state_ == ScannerState::STOPPING) { + ESP_LOGE(TAG, "Scan was not running when scan completed."); + } else if (this->scanner_state_ == ScannerState::STARTING) { + ESP_LOGE(TAG, "Scan was not started when scan completed."); + } else if (this->scanner_state_ == ScannerState::FAILED) { + ESP_LOGE(TAG, "Scan was in failed state when scan completed."); + } else if (this->scanner_state_ == ScannerState::IDLE) { + ESP_LOGE(TAG, "Scan was idle when scan completed."); + } else if (this->scanner_state_ == ScannerState::STOPPED) { + ESP_LOGE(TAG, "Scan was stopped when scan completed."); + } + } + this->set_scanner_state_(ScannerState::STOPPED); + } + + // Forward scan results to clients - they still expect the old format + if (scan_result.search_evt == ESP_GAP_SEARCH_INQ_RES_EVT) { + esp_ble_gap_cb_param_t param; + memset(¶m, 0, sizeof(param)); + memcpy(param.scan_rst.bda, scan_result.bda, sizeof(esp_bd_addr_t)); + param.scan_rst.ble_addr_type = scan_result.ble_addr_type; + param.scan_rst.rssi = scan_result.rssi; + param.scan_rst.adv_data_len = scan_result.adv_data_len; + param.scan_rst.scan_rsp_len = scan_result.scan_rsp_len; + param.scan_rst.search_evt = scan_result.search_evt; + memcpy(param.scan_rst.ble_adv, scan_result.ble_adv, ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX); + param.scan_rst.dev_type = 0; + param.scan_rst.ble_evt_type = 0; + param.scan_rst.flag = 0; + param.scan_rst.num_resps = 1; + param.scan_rst.num_dis = 0; + + for (auto *client : this->clients_) { + client->gap_event_handler(ESP_GAP_BLE_SCAN_RESULT_EVT, ¶m); + } } } @@ -444,33 +515,7 @@ void ESP32BLETracker::gap_scan_stop_complete_(const esp_ble_gap_cb_param_t::ble_ this->set_scanner_state_(ScannerState::STOPPED); } -void ESP32BLETracker::gap_scan_result_(const esp_ble_gap_cb_param_t::ble_scan_result_evt_param ¶m) { - ESP_LOGV(TAG, "gap_scan_result - event %d", param.search_evt); - if (param.search_evt == ESP_GAP_SEARCH_INQ_RES_EVT) { - if (xSemaphoreTake(this->scan_result_lock_, 0)) { - if (this->scan_result_index_ < ESP32BLETracker::SCAN_RESULT_BUFFER_SIZE) { - this->scan_result_buffer_[this->scan_result_index_++] = param; - } - xSemaphoreGive(this->scan_result_lock_); - } - } else if (param.search_evt == ESP_GAP_SEARCH_INQ_CMPL_EVT) { - // Scan finished on its own - if (this->scanner_state_ != ScannerState::RUNNING) { - if (this->scanner_state_ == ScannerState::STOPPING) { - ESP_LOGE(TAG, "Scan was not running when scan completed."); - } else if (this->scanner_state_ == ScannerState::STARTING) { - ESP_LOGE(TAG, "Scan was not started when scan completed."); - } else if (this->scanner_state_ == ScannerState::FAILED) { - ESP_LOGE(TAG, "Scan was in failed state when scan completed."); - } else if (this->scanner_state_ == ScannerState::IDLE) { - ESP_LOGE(TAG, "Scan was idle when scan completed."); - } else if (this->scanner_state_ == ScannerState::STOPPED) { - ESP_LOGE(TAG, "Scan was stopped when scan completed."); - } - } - this->set_scanner_state_(ScannerState::STOPPED); - } -} +// Removed - functionality moved to gap_scan_event_handler void ESP32BLETracker::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) { diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index eea73a7d26..75f164c137 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -121,9 +121,7 @@ class ESPBTDeviceListener { public: virtual void on_scan_end() {} virtual bool parse_device(const ESPBTDevice &device) = 0; - virtual bool parse_devices(esp_ble_gap_cb_param_t::ble_scan_result_evt_param *advertisements, size_t count) { - return false; - }; + virtual bool parse_devices(const BLEScanResult *scan_results, size_t count) { return false; }; virtual AdvertisementParserType get_advertisement_parser_type() { return AdvertisementParserType::PARSED_ADVERTISEMENTS; }; @@ -210,6 +208,7 @@ class ESPBTClient : public ESPBTDeviceListener { class ESP32BLETracker : public Component, public GAPEventHandler, + public GAPScanEventHandler, public GATTcEventHandler, public BLEStatusEventHandler, public Parented { @@ -240,6 +239,7 @@ class ESP32BLETracker : public Component, void gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) override; void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) override; + void gap_scan_event_handler(const BLEScanResult &scan_result) override; void ble_before_disabled_event_handler() override; void add_scanner_state_callback(std::function &&callback) { @@ -287,12 +287,8 @@ class ESP32BLETracker : public Component, bool parse_advertisements_{false}; SemaphoreHandle_t scan_result_lock_; size_t scan_result_index_{0}; -#ifdef USE_PSRAM - const static u_int8_t SCAN_RESULT_BUFFER_SIZE = 32; -#else - const static u_int8_t SCAN_RESULT_BUFFER_SIZE = 20; -#endif // USE_PSRAM - esp_ble_gap_cb_param_t::ble_scan_result_evt_param *scan_result_buffer_; + // SCAN_RESULT_BUFFER_SIZE is now defined in esp32_ble/ble.h + BLEScanResult *scan_result_buffer_; 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}; From 0ab69002dfae5711f7d38999ef41b661c4394535 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 10:05:15 -0500 Subject: [PATCH 02/33] preen --- esphome/components/esp32_ble/ble.h | 12 +---- esphome/components/esp32_ble/ble_event.h | 2 + .../components/esp32_ble/ble_scan_result.h | 24 +++++++++ .../esp32_ble_tracker/esp32_ble_tracker.cpp | 51 +++++++------------ .../esp32_ble_tracker/esp32_ble_tracker.h | 7 +-- 5 files changed, 47 insertions(+), 49 deletions(-) create mode 100644 esphome/components/esp32_ble/ble_scan_result.h diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index c43ec8c7ed..4d4fbe4de9 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -2,6 +2,7 @@ #include "ble_advertising.h" #include "ble_uuid.h" +#include "ble_scan_result.h" #include @@ -64,17 +65,6 @@ class GAPEventHandler { virtual void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) = 0; }; -// Structure for BLE scan results - only fields we actually use -struct BLEScanResult { - esp_bd_addr_t bda; - uint8_t ble_addr_type; - int8_t rssi; - uint8_t ble_adv[ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX]; - uint8_t adv_data_len; - uint8_t scan_rsp_len; - uint8_t search_evt; -}; // ~73 bytes vs ~400 bytes for full esp_ble_gap_cb_param_t - class GAPScanEventHandler { public: // Receives scan results directly without memcpy diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 451c52b114..7b1af08d54 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -8,6 +8,8 @@ #include #include +#include "ble_scan_result.h" + namespace esphome { namespace esp32_ble { diff --git a/esphome/components/esp32_ble/ble_scan_result.h b/esphome/components/esp32_ble/ble_scan_result.h new file mode 100644 index 0000000000..b46e9ea896 --- /dev/null +++ b/esphome/components/esp32_ble/ble_scan_result.h @@ -0,0 +1,24 @@ +#pragma once + +#ifdef USE_ESP32 + +#include + +namespace esphome { +namespace esp32_ble { + +// Structure for BLE scan results - only fields we actually use +struct BLEScanResult { + esp_bd_addr_t bda; + uint8_t ble_addr_type; + int8_t rssi; + uint8_t ble_adv[ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX]; + uint8_t adv_data_len; + uint8_t scan_rsp_len; + uint8_t search_evt; +}; // ~73 bytes vs ~400 bytes for full esp_ble_gap_cb_param_t + +} // namespace esp32_ble +} // namespace esphome + +#endif \ No newline at end of file diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 09fe451a3c..7168be0825 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -123,7 +123,7 @@ void ESP32BLETracker::loop() { this->scan_result_index_ && // if it looks like we have a scan result we will take the lock xSemaphoreTake(this->scan_result_lock_, 0)) { uint32_t index = this->scan_result_index_; - if (index >= ESP32BLETracker::SCAN_RESULT_BUFFER_SIZE) { + if (index >= SCAN_RESULT_BUFFER_SIZE) { ESP_LOGW(TAG, "Too many BLE events to process. Some devices may not show up."); } @@ -139,24 +139,7 @@ void ESP32BLETracker::loop() { if (this->parse_advertisements_) { for (size_t i = 0; i < index; i++) { ESPBTDevice device; - // Convert BLEScanResult to ESP-IDF format for parse_scan_rst - esp_ble_gap_cb_param_t::ble_scan_result_evt_param param; - memcpy(param.bda, this->scan_result_buffer_[i].bda, sizeof(esp_bd_addr_t)); - param.ble_addr_type = this->scan_result_buffer_[i].ble_addr_type; - param.rssi = this->scan_result_buffer_[i].rssi; - param.adv_data_len = this->scan_result_buffer_[i].adv_data_len; - param.scan_rsp_len = this->scan_result_buffer_[i].scan_rsp_len; - param.search_evt = this->scan_result_buffer_[i].search_evt; - memcpy(param.ble_adv, this->scan_result_buffer_[i].ble_adv, - ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX); - // Fill in fields we don't store - param.dev_type = 0; - param.ble_evt_type = 0; - param.flag = 0; - param.num_resps = 1; - param.num_dis = 0; - - device.parse_scan_rst(param); + device.parse_scan_rst(this->scan_result_buffer_[i]); bool found = false; for (auto *listener : this->listeners_) { @@ -443,14 +426,14 @@ void ESP32BLETracker::gap_scan_event_handler(const BLEScanResult &scan_result) { esp_ble_gap_cb_param_t param; memset(¶m, 0, sizeof(param)); memcpy(param.scan_rst.bda, scan_result.bda, sizeof(esp_bd_addr_t)); - param.scan_rst.ble_addr_type = scan_result.ble_addr_type; + param.scan_rst.ble_addr_type = static_cast(scan_result.ble_addr_type); param.scan_rst.rssi = scan_result.rssi; param.scan_rst.adv_data_len = scan_result.adv_data_len; param.scan_rst.scan_rsp_len = scan_result.scan_rsp_len; - param.scan_rst.search_evt = scan_result.search_evt; + param.scan_rst.search_evt = static_cast(scan_result.search_evt); memcpy(param.scan_rst.ble_adv, scan_result.ble_adv, ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX); - param.scan_rst.dev_type = 0; - param.scan_rst.ble_evt_type = 0; + param.scan_rst.dev_type = static_cast(0); + param.scan_rst.ble_evt_type = static_cast(0); param.scan_rst.flag = 0; param.scan_rst.num_resps = 1; param.scan_rst.num_dis = 0; @@ -539,13 +522,15 @@ optional ESPBLEiBeacon::from_manufacturer_data(const ServiceData return ESPBLEiBeacon(data.data.data()); } -void ESPBTDevice::parse_scan_rst(const esp_ble_gap_cb_param_t::ble_scan_result_evt_param ¶m) { - this->scan_result_ = param; +void ESPBTDevice::parse_scan_rst(const BLEScanResult &scan_result) { for (uint8_t i = 0; i < ESP_BD_ADDR_LEN; i++) - this->address_[i] = param.bda[i]; - this->address_type_ = param.ble_addr_type; - this->rssi_ = param.rssi; - this->parse_adv_(param); + this->address_[i] = scan_result.bda[i]; + this->address_type_ = static_cast(scan_result.ble_addr_type); + this->rssi_ = scan_result.rssi; + + // Parse advertisement data directly + uint8_t total_len = scan_result.adv_data_len + scan_result.scan_rsp_len; + this->parse_adv_(scan_result.ble_adv, total_len); #ifdef ESPHOME_LOG_HAS_VERY_VERBOSE ESP_LOGVV(TAG, "Parse Result:"); @@ -603,13 +588,13 @@ void ESPBTDevice::parse_scan_rst(const esp_ble_gap_cb_param_t::ble_scan_result_e ESP_LOGVV(TAG, " Data: %s", format_hex_pretty(data.data).c_str()); } - ESP_LOGVV(TAG, " Adv data: %s", format_hex_pretty(param.ble_adv, param.adv_data_len + param.scan_rsp_len).c_str()); + ESP_LOGVV(TAG, " Adv data: %s", + format_hex_pretty(scan_result.ble_adv, scan_result.adv_data_len + scan_result.scan_rsp_len).c_str()); #endif } -void ESPBTDevice::parse_adv_(const esp_ble_gap_cb_param_t::ble_scan_result_evt_param ¶m) { + +void ESPBTDevice::parse_adv_(const uint8_t *payload, uint8_t len) { size_t offset = 0; - const uint8_t *payload = param.ble_adv; - uint8_t len = param.adv_data_len + param.scan_rsp_len; while (offset + 2 < len) { const uint8_t field_length = payload[offset++]; // First byte is length of adv record diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index 75f164c137..50a1a14740 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -62,7 +62,7 @@ class ESPBLEiBeacon { class ESPBTDevice { public: - void parse_scan_rst(const esp_ble_gap_cb_param_t::ble_scan_result_evt_param ¶m); + void parse_scan_rst(const BLEScanResult &scan_result); std::string address_str() const; @@ -84,8 +84,6 @@ class ESPBTDevice { const std::vector &get_service_datas() const { return service_datas_; } - const esp_ble_gap_cb_param_t::ble_scan_result_evt_param &get_scan_result() const { return scan_result_; } - bool resolve_irk(const uint8_t *irk) const; optional get_ibeacon() const { @@ -98,7 +96,7 @@ class ESPBTDevice { } protected: - void parse_adv_(const esp_ble_gap_cb_param_t::ble_scan_result_evt_param ¶m); + void parse_adv_(const uint8_t *payload, uint8_t len); esp_bd_addr_t address_{ 0, @@ -112,7 +110,6 @@ class ESPBTDevice { std::vector service_uuids_{}; std::vector manufacturer_datas_{}; std::vector service_datas_{}; - esp_ble_gap_cb_param_t::ble_scan_result_evt_param scan_result_{}; }; class ESP32BLETracker; From 78315fd3880669618ea505d36ca8b07136c53a1d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 10:08:30 -0500 Subject: [PATCH 03/33] preen --- .../esp32_ble_tracker/esp32_ble_tracker.cpp | 23 ++----------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 7168be0825..7e153c317d 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -421,27 +421,8 @@ void ESP32BLETracker::gap_scan_event_handler(const BLEScanResult &scan_result) { this->set_scanner_state_(ScannerState::STOPPED); } - // Forward scan results to clients - they still expect the old format - if (scan_result.search_evt == ESP_GAP_SEARCH_INQ_RES_EVT) { - esp_ble_gap_cb_param_t param; - memset(¶m, 0, sizeof(param)); - memcpy(param.scan_rst.bda, scan_result.bda, sizeof(esp_bd_addr_t)); - param.scan_rst.ble_addr_type = static_cast(scan_result.ble_addr_type); - param.scan_rst.rssi = scan_result.rssi; - param.scan_rst.adv_data_len = scan_result.adv_data_len; - param.scan_rst.scan_rsp_len = scan_result.scan_rsp_len; - param.scan_rst.search_evt = static_cast(scan_result.search_evt); - memcpy(param.scan_rst.ble_adv, scan_result.ble_adv, ESP_BLE_ADV_DATA_LEN_MAX + ESP_BLE_SCAN_RSP_DATA_LEN_MAX); - param.scan_rst.dev_type = static_cast(0); - param.scan_rst.ble_evt_type = static_cast(0); - param.scan_rst.flag = 0; - param.scan_rst.num_resps = 1; - param.scan_rst.num_dis = 0; - - for (auto *client : this->clients_) { - client->gap_event_handler(ESP_GAP_BLE_SCAN_RESULT_EVT, ¶m); - } - } + // Note: BLE clients don't actually process ESP_GAP_BLE_SCAN_RESULT_EVT + // They use parse_device() instead, so we don't need to forward scan results } void ESP32BLETracker::gap_scan_set_param_complete_(const esp_ble_gap_cb_param_t::ble_scan_param_cmpl_evt_param ¶m) { From 0e9f14f969326f1fc70a2218369faa95f7c1df90 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 10:20:18 -0500 Subject: [PATCH 04/33] wip --- esphome/components/esp32_ble/ble.cpp | 31 +++++++++++++----------- esphome/components/esp32_ble/ble_event.h | 15 ++++++------ 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 24566e8f6d..a77496540d 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -322,23 +322,19 @@ void ESP32BLE::loop() { } else if (gap_event == ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT || gap_event == ESP_GAP_BLE_SCAN_START_COMPLETE_EVT || gap_event == ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { - // Create temporary param for scan complete events - esp_ble_gap_cb_param_t param; - memset(¶m, 0, sizeof(param)); + // All three scan complete events have the same structure with just status + // We can create a minimal structure that matches their layout + struct { + esp_bt_status_t status; + } scan_complete_param; - // Set the appropriate status field based on event type - if (gap_event == ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT) { - param.scan_param_cmpl.status = ble_event->event_.gap.scan_complete.status; - } else if (gap_event == ESP_GAP_BLE_SCAN_START_COMPLETE_EVT) { - param.scan_start_cmpl.status = ble_event->event_.gap.scan_complete.status; - } else if (gap_event == ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { - param.scan_stop_cmpl.status = ble_event->event_.gap.scan_complete.status; - } + scan_complete_param.status = ble_event->event_.gap.scan_complete.status; - this->real_gap_event_handler_(gap_event, ¶m); + // Cast is safe because all three event structures start with status + this->real_gap_event_handler_(gap_event, (esp_ble_gap_cb_param_t *) &scan_complete_param); } else { - // Fallback for unexpected events (uses full param copy) - this->real_gap_event_handler_(gap_event, &ble_event->event_.gap.gap_param); + // Unexpected GAP event - log and drop + ESP_LOGW(TAG, "Unexpected GAP event type: %d", gap_event); } break; } @@ -357,6 +353,13 @@ void ESP32BLE::loop() { void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { static constexpr size_t MAX_BLE_QUEUE_SIZE = SCAN_RESULT_BUFFER_SIZE * 2; + // Only queue the 4 GAP events we actually handle + if (event != ESP_GAP_BLE_SCAN_RESULT_EVT && event != ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT && + event != ESP_GAP_BLE_SCAN_START_COMPLETE_EVT && event != ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { + ESP_LOGW(TAG, "Ignoring unexpected GAP event type: %d", event); + return; + } + if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { ESP_LOGW(TAG, "BLE event queue full (%d), dropping GAP event %d", MAX_BLE_QUEUE_SIZE, event); return; diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 7b1af08d54..03c86f09e9 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -48,9 +48,8 @@ class BLEEvent { break; default: - // For any other GAP events, copy the full param - // This is a safety fallback but shouldn't happen in normal operation - memcpy(&this->event_.gap.gap_param, p, sizeof(esp_ble_gap_cb_param_t)); + // We only handle 4 GAP event types, others are dropped + // This should never happen in normal operation break; } }; @@ -106,10 +105,10 @@ class BLEEvent { esp_bt_status_t status; } scan_complete; // 1 byte - // Fallback for unexpected events (shouldn't be used) - esp_ble_gap_cb_param_t gap_param; + // We only handle 4 GAP event types, no need for full fallback + // If we ever get an unexpected event, we'll just drop it in ble.cpp }; - } gap; // ~80 bytes instead of 400+ + } gap; // ~73 bytes (size of BLEScanResult) // NOLINTNEXTLINE(readability-identifier-naming) struct gattc_event { @@ -124,7 +123,7 @@ class BLEEvent { esp_gatt_if_t gatts_if; esp_ble_gatts_cb_param_t gatts_param; } gatts; // ~68 bytes - } event_; // Union size is now ~80 bytes (largest member) + } event_; // Union size is now ~73 bytes (BLEScanResult is largest) std::vector data{}; // For GATTC/GATTS data @@ -140,7 +139,7 @@ class BLEEvent { const BLEScanResult &scan_result() const { return event_.gap.scan_result; } esp_bt_status_t scan_complete_status() const { return event_.gap.scan_complete.status; } }; -// Total size: ~110 bytes instead of 440 bytes! +// Total size: ~100 bytes instead of 440 bytes! } // namespace esp32_ble } // namespace esphome From 068c62c6fe40b1a0831171ee7705c17048fdc8da Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 10:43:48 -0500 Subject: [PATCH 05/33] adjust --- esphome/components/esp32_ble/ble.cpp | 5 +- esphome/components/esp32_ble/ble_event.h | 101 +++++++++++++++-------- 2 files changed, 70 insertions(+), 36 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index a77496540d..917467d1ad 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -306,11 +306,11 @@ void ESP32BLE::loop() { switch (ble_event->type_) { case BLEEvent::GATTS: this->real_gatts_event_handler_(ble_event->event_.gatts.gatts_event, ble_event->event_.gatts.gatts_if, - &ble_event->event_.gatts.gatts_param); + ble_event->event_.gatts.gatts_param); break; case BLEEvent::GATTC: this->real_gattc_event_handler_(ble_event->event_.gattc.gattc_event, ble_event->event_.gattc.gattc_if, - &ble_event->event_.gattc.gattc_param); + ble_event->event_.gattc.gattc_param); break; case BLEEvent::GAP: { esp_gap_ble_cb_event_t gap_event = ble_event->event_.gap.gap_event; @@ -341,6 +341,7 @@ void ESP32BLE::loop() { default: break; } + // Destructor will clean up external allocations for GATTC/GATTS ble_event->~BLEEvent(); EVENT_ALLOCATOR.deallocate(ble_event, 1); ble_event = this->ble_events_.pop(); diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 03c86f09e9..0e8dac4b83 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -14,12 +14,25 @@ namespace esphome { namespace esp32_ble { // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). -// This class stores each event with minimal memory usage by only copying the data we actually need. +// This class stores each event with minimal memory usage. +// GAP events (99% of traffic) don't have the vector overhead. +// GATTC/GATTS events use external storage for their param and data. class BLEEvent { public: + // NOLINTNEXTLINE(readability-identifier-naming) + enum ble_event_t : uint8_t { + GAP, + GATTC, + GATTS, + }; + + BLEEvent() = default; + + // Constructor for GAP events - no external allocations needed BLEEvent(esp_gap_ble_cb_event_t e, esp_ble_gap_cb_param_t *p) { this->type_ = GAP; this->event_.gap.gap_event = e; + this->event_.gap.ext_data = nullptr; // GAP events don't use external data // Only copy the data we actually use for each GAP event type switch (e) { @@ -49,97 +62,117 @@ class BLEEvent { default: // We only handle 4 GAP event types, others are dropped - // This should never happen in normal operation break; } - }; + } + // Constructor for GATTC events - uses external storage BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { this->type_ = GATTC; this->event_.gattc.gattc_event = e; this->event_.gattc.gattc_if = i; - memcpy(&this->event_.gattc.gattc_param, p, sizeof(esp_ble_gattc_cb_param_t)); + + // Allocate external storage for param and data + this->event_.gattc.gattc_param = new esp_ble_gattc_cb_param_t(*p); // Copy data for events that need it switch (e) { case ESP_GATTC_NOTIFY_EVT: - this->data.assign(p->notify.value, p->notify.value + p->notify.value_len); - this->event_.gattc.gattc_param.notify.value = this->data.data(); + this->event_.gattc.data = new std::vector(p->notify.value, p->notify.value + p->notify.value_len); + this->event_.gattc.gattc_param->notify.value = this->event_.gattc.data->data(); break; case ESP_GATTC_READ_CHAR_EVT: case ESP_GATTC_READ_DESCR_EVT: - this->data.assign(p->read.value, p->read.value + p->read.value_len); - this->event_.gattc.gattc_param.read.value = this->data.data(); + this->event_.gattc.data = new std::vector(p->read.value, p->read.value + p->read.value_len); + this->event_.gattc.gattc_param->read.value = this->event_.gattc.data->data(); break; default: + this->event_.gattc.data = nullptr; break; } - }; + } + // Constructor for GATTS events - uses external storage BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) { this->type_ = GATTS; this->event_.gatts.gatts_event = e; this->event_.gatts.gatts_if = i; - memcpy(&this->event_.gatts.gatts_param, p, sizeof(esp_ble_gatts_cb_param_t)); + + // Allocate external storage for param and data + this->event_.gatts.gatts_param = new esp_ble_gatts_cb_param_t(*p); // Copy data for events that need it switch (e) { case ESP_GATTS_WRITE_EVT: - this->data.assign(p->write.value, p->write.value + p->write.len); - this->event_.gatts.gatts_param.write.value = this->data.data(); + this->event_.gatts.data = new std::vector(p->write.value, p->write.value + p->write.len); + this->event_.gatts.gatts_param->write.value = this->event_.gatts.data->data(); + break; + default: + this->event_.gatts.data = nullptr; + break; + } + } + + // Destructor to clean up external allocations + ~BLEEvent() { + switch (this->type_) { + case GATTC: + delete this->event_.gattc.gattc_param; + delete this->event_.gattc.data; + break; + case GATTS: + delete this->event_.gatts.gatts_param; + delete this->event_.gatts.data; break; default: break; } - }; + } + + // Disable copy to prevent double-delete + BLEEvent(const BLEEvent &) = delete; + BLEEvent &operator=(const BLEEvent &) = delete; union { // NOLINTNEXTLINE(readability-identifier-naming) struct gap_event { esp_gap_ble_cb_event_t gap_event; + void *ext_data; // Always nullptr for GAP, just for alignment union { - BLEScanResult scan_result; // ~73 bytes - - // Minimal storage for scan complete events + BLEScanResult scan_result; // 73 bytes struct { esp_bt_status_t status; } scan_complete; // 1 byte - - // We only handle 4 GAP event types, no need for full fallback - // If we ever get an unexpected event, we'll just drop it in ble.cpp }; - } gap; // ~73 bytes (size of BLEScanResult) + } gap; // 80 bytes (with alignment) // NOLINTNEXTLINE(readability-identifier-naming) struct gattc_event { esp_gattc_cb_event_t gattc_event; esp_gatt_if_t gattc_if; - esp_ble_gattc_cb_param_t gattc_param; - } gattc; // ~68 bytes + esp_ble_gattc_cb_param_t *gattc_param; // External allocation + std::vector *data; // External allocation + } gattc; // 16 bytes (4 + 4 + 4 + 4) // NOLINTNEXTLINE(readability-identifier-naming) struct gatts_event { esp_gatts_cb_event_t gatts_event; esp_gatt_if_t gatts_if; - esp_ble_gatts_cb_param_t gatts_param; - } gatts; // ~68 bytes - } event_; // Union size is now ~73 bytes (BLEScanResult is largest) + esp_ble_gatts_cb_param_t *gatts_param; // External allocation + std::vector *data; // External allocation + } gatts; // 16 bytes (4 + 4 + 4 + 4) + } event_; // Union size is 80 bytes (largest member is gap) - std::vector data{}; // For GATTC/GATTS data - - // NOLINTNEXTLINE(readability-identifier-naming) - enum ble_event_t : uint8_t { - GAP, - GATTC, - GATTS, - } type_; + ble_event_t type_; // Helper methods to access event data + ble_event_t type() const { return type_; } esp_gap_ble_cb_event_t gap_event_type() const { return event_.gap.gap_event; } const BLEScanResult &scan_result() const { return event_.gap.scan_result; } esp_bt_status_t scan_complete_status() const { return event_.gap.scan_complete.status; } }; -// Total size: ~100 bytes instead of 440 bytes! +// Total size for GAP events: ~84 bytes (was 296 bytes - 71.6% reduction!) +// GATTC/GATTS events use external storage, keeping the queue size minimal } // namespace esp32_ble } // namespace esphome From a1b5a2abcb9cf0d386944fee287c8332f8653ae5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 10:58:56 -0500 Subject: [PATCH 06/33] tweak --- esphome/components/esp32_ble/ble_event.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 0e8dac4b83..932d0fef5e 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -32,7 +32,6 @@ class BLEEvent { BLEEvent(esp_gap_ble_cb_event_t e, esp_ble_gap_cb_param_t *p) { this->type_ = GAP; this->event_.gap.gap_event = e; - this->event_.gap.ext_data = nullptr; // GAP events don't use external data // Only copy the data we actually use for each GAP event type switch (e) { @@ -137,14 +136,13 @@ class BLEEvent { // NOLINTNEXTLINE(readability-identifier-naming) struct gap_event { esp_gap_ble_cb_event_t gap_event; - void *ext_data; // Always nullptr for GAP, just for alignment union { BLEScanResult scan_result; // 73 bytes struct { esp_bt_status_t status; } scan_complete; // 1 byte }; - } gap; // 80 bytes (with alignment) + } gap; // 77 bytes (4 + 73) // NOLINTNEXTLINE(readability-identifier-naming) struct gattc_event { @@ -161,7 +159,7 @@ class BLEEvent { esp_ble_gatts_cb_param_t *gatts_param; // External allocation std::vector *data; // External allocation } gatts; // 16 bytes (4 + 4 + 4 + 4) - } event_; // Union size is 80 bytes (largest member is gap) + } event_; // Union size is 80 bytes with padding ble_event_t type_; @@ -171,7 +169,8 @@ class BLEEvent { const BLEScanResult &scan_result() const { return event_.gap.scan_result; } esp_bt_status_t scan_complete_status() const { return event_.gap.scan_complete.status; } }; -// Total size for GAP events: ~84 bytes (was 296 bytes - 71.6% reduction!) +// Total size: 84 bytes (80 byte union + 1 byte type + 3 bytes padding) +// Was 296 bytes - 71.6% reduction! // GATTC/GATTS events use external storage, keeping the queue size minimal } // namespace esp32_ble From 0adf514bd6e5a6fe12e11a96629b79ad138b8654 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:09:19 -0500 Subject: [PATCH 07/33] preen --- esphome/components/esp32_ble/ble.cpp | 20 ++++++++------------ esphome/components/esp32_ble/ble.h | 1 + esphome/components/esp32_ble/ble_event.h | 23 +++++++++++------------ 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 917467d1ad..24bb8cc642 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -23,6 +23,9 @@ namespace esp32_ble { static const char *const TAG = "esp32_ble"; +// Maximum size of the BLE event queue +static constexpr size_t MAX_BLE_QUEUE_SIZE = SCAN_RESULT_BUFFER_SIZE * 2; + static RAMAllocator EVENT_ALLOCATOR( // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) RAMAllocator::ALLOW_FAILURE | RAMAllocator::ALLOC_INTERNAL); @@ -333,8 +336,8 @@ void ESP32BLE::loop() { // Cast is safe because all three event structures start with status this->real_gap_event_handler_(gap_event, (esp_ble_gap_cb_param_t *) &scan_complete_param); } else { - // Unexpected GAP event - log and drop - ESP_LOGW(TAG, "Unexpected GAP event type: %d", gap_event); + // Unexpected GAP event - drop it + ESP_LOGV(TAG, "Unexpected GAP event type: %d", gap_event); } break; } @@ -352,17 +355,14 @@ void ESP32BLE::loop() { } void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { - static constexpr size_t MAX_BLE_QUEUE_SIZE = SCAN_RESULT_BUFFER_SIZE * 2; - // Only queue the 4 GAP events we actually handle if (event != ESP_GAP_BLE_SCAN_RESULT_EVT && event != ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT && event != ESP_GAP_BLE_SCAN_START_COMPLETE_EVT && event != ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { - ESP_LOGW(TAG, "Ignoring unexpected GAP event type: %d", event); return; } if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGW(TAG, "BLE event queue full (%d), dropping GAP event %d", MAX_BLE_QUEUE_SIZE, event); + ESP_LOGV(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); return; } @@ -384,10 +384,8 @@ void ESP32BLE::real_gap_event_handler_(esp_gap_ble_cb_event_t event, esp_ble_gap void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) { - static constexpr size_t MAX_BLE_QUEUE_SIZE = SCAN_RESULT_BUFFER_SIZE * 2; - if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGW(TAG, "BLE event queue full (%d), dropping GATTS event %d", MAX_BLE_QUEUE_SIZE, event); + ESP_LOGV(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); return; } @@ -410,10 +408,8 @@ void ESP32BLE::real_gatts_event_handler_(esp_gatts_cb_event_t event, esp_gatt_if void ESP32BLE::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) { - static constexpr size_t MAX_BLE_QUEUE_SIZE = SCAN_RESULT_BUFFER_SIZE * 2; - if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGW(TAG, "BLE event queue full (%d), dropping GATTC event %d", MAX_BLE_QUEUE_SIZE, event); + ESP_LOGV(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); return; } diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index 4d4fbe4de9..ef006ef73c 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -139,6 +139,7 @@ class ESP32BLE : public Component { bool ble_pre_setup_(); void advertising_init_(); + private: std::vector gap_event_handlers_; std::vector gap_scan_event_handlers_; std::vector gattc_event_handlers_; diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 932d0fef5e..c7574be6cc 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -36,7 +36,7 @@ class BLEEvent { // Only copy the data we actually use for each GAP event type switch (e) { case ESP_GAP_BLE_SCAN_RESULT_EVT: - // Copy only the fields we use from scan results (~72 bytes) + // Copy only the fields we use from scan results memcpy(this->event_.gap.scan_result.bda, p->scan_rst.bda, sizeof(esp_bd_addr_t)); this->event_.gap.scan_result.ble_addr_type = p->scan_rst.ble_addr_type; this->event_.gap.scan_result.rssi = p->scan_rst.rssi; @@ -142,24 +142,24 @@ class BLEEvent { esp_bt_status_t status; } scan_complete; // 1 byte }; - } gap; // 77 bytes (4 + 73) + } gap; // 80 bytes total // NOLINTNEXTLINE(readability-identifier-naming) struct gattc_event { esp_gattc_cb_event_t gattc_event; esp_gatt_if_t gattc_if; - esp_ble_gattc_cb_param_t *gattc_param; // External allocation - std::vector *data; // External allocation - } gattc; // 16 bytes (4 + 4 + 4 + 4) + esp_ble_gattc_cb_param_t *gattc_param; + std::vector *data; + } gattc; // 16 bytes (pointers only) // NOLINTNEXTLINE(readability-identifier-naming) struct gatts_event { esp_gatts_cb_event_t gatts_event; esp_gatt_if_t gatts_if; - esp_ble_gatts_cb_param_t *gatts_param; // External allocation - std::vector *data; // External allocation - } gatts; // 16 bytes (4 + 4 + 4 + 4) - } event_; // Union size is 80 bytes with padding + esp_ble_gatts_cb_param_t *gatts_param; + std::vector *data; + } gatts; // 16 bytes (pointers only) + } event_; // 80 bytes ble_event_t type_; @@ -169,9 +169,8 @@ class BLEEvent { const BLEScanResult &scan_result() const { return event_.gap.scan_result; } esp_bt_status_t scan_complete_status() const { return event_.gap.scan_complete.status; } }; -// Total size: 84 bytes (80 byte union + 1 byte type + 3 bytes padding) -// Was 296 bytes - 71.6% reduction! -// GATTC/GATTS events use external storage, keeping the queue size minimal + +// BLEEvent total size: 84 bytes (80 byte union + 1 byte type + 3 bytes padding) } // namespace esp32_ble } // namespace esphome From 88a3df4008edf9952284a190d19451f2b2e8af1e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:13:34 -0500 Subject: [PATCH 08/33] cleanup --- esphome/components/esp32_ble/ble_event.h | 15 +++++++++++++++ .../esp32_ble_tracker/esp32_ble_tracker.cpp | 2 -- .../esp32_ble_tracker/esp32_ble_tracker.h | 1 - 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index c7574be6cc..a29d668f4d 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -17,6 +17,19 @@ namespace esp32_ble { // This class stores each event with minimal memory usage. // GAP events (99% of traffic) don't have the vector overhead. // GATTC/GATTS events use external storage for their param and data. +// +// Event flow: +// 1. ESP-IDF BLE stack calls our static handlers in the BLE task context +// 2. The handlers create a BLEEvent instance, copying only the data we need +// 3. The event is pushed to a thread-safe queue +// 4. In the main loop(), events are popped from the queue and processed +// 5. The event destructor cleans up any external allocations +// +// Thread safety: +// - GAP events: We copy only the fields we need directly into the union +// - GATTC/GATTS events: We allocate and copy the entire param struct, ensuring +// the data remains valid even after the BLE callback returns. The original +// param pointer from ESP-IDF is only valid during the callback. class BLEEvent { public: // NOLINTNEXTLINE(readability-identifier-naming) @@ -66,6 +79,7 @@ class BLEEvent { } // Constructor for GATTC events - uses external storage + // Creates a copy of the param struct since the original is only valid during the callback BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { this->type_ = GATTC; this->event_.gattc.gattc_event = e; @@ -92,6 +106,7 @@ class BLEEvent { } // Constructor for GATTS events - uses external storage + // Creates a copy of the param struct since the original is only valid during the callback BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) { this->type_ = GATTS; this->event_.gatts.gatts_event = e; diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 7e153c317d..d1f0c67e99 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -479,8 +479,6 @@ void ESP32BLETracker::gap_scan_stop_complete_(const esp_ble_gap_cb_param_t::ble_ this->set_scanner_state_(ScannerState::STOPPED); } -// Removed - functionality moved to gap_scan_event_handler - void ESP32BLETracker::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) { for (auto *client : this->clients_) { diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index 50a1a14740..33c0caaa87 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -284,7 +284,6 @@ class ESP32BLETracker : public Component, bool parse_advertisements_{false}; SemaphoreHandle_t scan_result_lock_; size_t scan_result_index_{0}; - // SCAN_RESULT_BUFFER_SIZE is now defined in esp32_ble/ble.h BLEScanResult *scan_result_buffer_; esp_bt_status_t scan_start_failed_{ESP_BT_STATUS_SUCCESS}; esp_bt_status_t scan_set_param_failed_{ESP_BT_STATUS_SUCCESS}; From 2f8946f86cb41328e47815df86d96a58fc124716 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:14:10 -0500 Subject: [PATCH 09/33] cleanup --- esphome/components/esp32_ble/ble.h | 1 - 1 file changed, 1 deletion(-) diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index ef006ef73c..d33ebf0f59 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -67,7 +67,6 @@ class GAPEventHandler { class GAPScanEventHandler { public: - // Receives scan results directly without memcpy virtual void gap_scan_event_handler(const BLEScanResult &scan_result) = 0; }; From 0331cb09e8909ef0d77b87966e33f52a75de3cc7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:17:01 -0500 Subject: [PATCH 10/33] reduce --- esphome/components/esp32_ble/ble.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 24bb8cc642..d8e1a8afc6 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -326,15 +326,8 @@ void ESP32BLE::loop() { gap_event == ESP_GAP_BLE_SCAN_START_COMPLETE_EVT || gap_event == ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { // All three scan complete events have the same structure with just status - // We can create a minimal structure that matches their layout - struct { - esp_bt_status_t status; - } scan_complete_param; - - scan_complete_param.status = ble_event->event_.gap.scan_complete.status; - - // Cast is safe because all three event structures start with status - this->real_gap_event_handler_(gap_event, (esp_ble_gap_cb_param_t *) &scan_complete_param); + // Cast is safe because all three ESP-IDF event structures are identical with just status field + this->real_gap_event_handler_(gap_event, (esp_ble_gap_cb_param_t *) &ble_event->event_.gap.scan_complete); } else { // Unexpected GAP event - drop it ESP_LOGV(TAG, "Unexpected GAP event type: %d", gap_event); From 9f0051c21ff196bbbcecb2b00014e563f57600a0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:17:10 -0500 Subject: [PATCH 11/33] cleanup --- esphome/components/esp32_ble/ble.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index d8e1a8afc6..1727b308bc 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -327,6 +327,7 @@ void ESP32BLE::loop() { gap_event == ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { // All three scan complete events have the same structure with just status // Cast is safe because all three ESP-IDF event structures are identical with just status field + // The scan_complete struct already contains our copy of the status (copied in BLEEvent constructor) this->real_gap_event_handler_(gap_event, (esp_ble_gap_cb_param_t *) &ble_event->event_.gap.scan_complete); } else { // Unexpected GAP event - drop it From 4641f73d19cba2fc4e148f01393fc4d9a3407fbd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:19:36 -0500 Subject: [PATCH 12/33] comments --- esphome/components/esp32_ble/ble_event.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index a29d668f4d..86eaadfc13 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -86,6 +86,8 @@ class BLEEvent { this->event_.gattc.gattc_if = i; // Allocate external storage for param and data + // External allocation is used because GATTC/GATTS events are rare (<1% of events) + // while GAP events (99%) are stored inline to minimize memory usage this->event_.gattc.gattc_param = new esp_ble_gattc_cb_param_t(*p); // Copy data for events that need it @@ -113,6 +115,8 @@ class BLEEvent { this->event_.gatts.gatts_if = i; // Allocate external storage for param and data + // External allocation is used because GATTC/GATTS events are rare (<1% of events) + // while GAP events (99%) are stored inline to minimize memory usage this->event_.gatts.gatts_param = new esp_ble_gatts_cb_param_t(*p); // Copy data for events that need it From d9ffd0ac8e96efb9b1ac760c92413feb422ff191 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:22:01 -0500 Subject: [PATCH 13/33] wip --- esphome/components/esp32_ble/ble.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 1727b308bc..9af2bef480 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -331,7 +331,7 @@ void ESP32BLE::loop() { this->real_gap_event_handler_(gap_event, (esp_ble_gap_cb_param_t *) &ble_event->event_.gap.scan_complete); } else { // Unexpected GAP event - drop it - ESP_LOGV(TAG, "Unexpected GAP event type: %d", gap_event); + ESP_LOGW(TAG, "Unexpected GAP event type: %d", gap_event); } break; } From 6e70aca4582184363b35927347af9f42f42ac840 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:23:13 -0500 Subject: [PATCH 14/33] wip --- esphome/components/esp32_ble/ble.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 9af2bef480..00697ee15c 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -329,9 +329,6 @@ void ESP32BLE::loop() { // Cast is safe because all three ESP-IDF event structures are identical with just status field // The scan_complete struct already contains our copy of the status (copied in BLEEvent constructor) this->real_gap_event_handler_(gap_event, (esp_ble_gap_cb_param_t *) &ble_event->event_.gap.scan_complete); - } else { - // Unexpected GAP event - drop it - ESP_LOGW(TAG, "Unexpected GAP event type: %d", gap_event); } break; } @@ -352,6 +349,7 @@ void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_pa // Only queue the 4 GAP events we actually handle if (event != ESP_GAP_BLE_SCAN_RESULT_EVT && event != ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT && event != ESP_GAP_BLE_SCAN_START_COMPLETE_EVT && event != ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { + ESP_LOGW(TAG, "Ignoring unexpected GAP event type: %d", event); return; } From c91e16549d53202ebf403aed249446a1264b11ae Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:27:13 -0500 Subject: [PATCH 15/33] lint --- esphome/components/esp32_ble/ble_event.h | 32 +++++++++---------- .../components/esp32_ble/ble_scan_result.h | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 86eaadfc13..eb6453801f 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -16,7 +16,7 @@ namespace esp32_ble { // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). // This class stores each event with minimal memory usage. // GAP events (99% of traffic) don't have the vector overhead. -// GATTC/GATTS events use external storage for their param and data. +// GATTC/GATTS events use heap allocation for their param and data. // // Event flow: // 1. ESP-IDF BLE stack calls our static handlers in the BLE task context @@ -27,7 +27,7 @@ namespace esp32_ble { // // Thread safety: // - GAP events: We copy only the fields we need directly into the union -// - GATTC/GATTS events: We allocate and copy the entire param struct, ensuring +// - GATTC/GATTS events: We heap-allocate and copy the entire param struct, ensuring // the data remains valid even after the BLE callback returns. The original // param pointer from ESP-IDF is only valid during the callback. class BLEEvent { @@ -78,15 +78,15 @@ class BLEEvent { } } - // Constructor for GATTC events - uses external storage + // Constructor for GATTC events - uses heap allocation // Creates a copy of the param struct since the original is only valid during the callback BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { this->type_ = GATTC; this->event_.gattc.gattc_event = e; this->event_.gattc.gattc_if = i; - // Allocate external storage for param and data - // External allocation is used because GATTC/GATTS events are rare (<1% of events) + // Heap-allocate param and data + // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) // while GAP events (99%) are stored inline to minimize memory usage this->event_.gattc.gattc_param = new esp_ble_gattc_cb_param_t(*p); @@ -107,15 +107,15 @@ class BLEEvent { } } - // Constructor for GATTS events - uses external storage + // Constructor for GATTS events - uses heap allocation // Creates a copy of the param struct since the original is only valid during the callback BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) { this->type_ = GATTS; this->event_.gatts.gatts_event = e; this->event_.gatts.gatts_if = i; - // Allocate external storage for param and data - // External allocation is used because GATTC/GATTS events are rare (<1% of events) + // Heap-allocate param and data + // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) // while GAP events (99%) are stored inline to minimize memory usage this->event_.gatts.gatts_param = new esp_ble_gatts_cb_param_t(*p); @@ -131,7 +131,7 @@ class BLEEvent { } } - // Destructor to clean up external allocations + // Destructor to clean up heap allocations ~BLEEvent() { switch (this->type_) { case GATTC: @@ -167,18 +167,18 @@ class BLEEvent { struct gattc_event { esp_gattc_cb_event_t gattc_event; esp_gatt_if_t gattc_if; - esp_ble_gattc_cb_param_t *gattc_param; - std::vector *data; - } gattc; // 16 bytes (pointers only) + esp_ble_gattc_cb_param_t *gattc_param; // Heap-allocated + std::vector *data; // Heap-allocated + } gattc; // 16 bytes (pointers only) // NOLINTNEXTLINE(readability-identifier-naming) struct gatts_event { esp_gatts_cb_event_t gatts_event; esp_gatt_if_t gatts_if; - esp_ble_gatts_cb_param_t *gatts_param; - std::vector *data; - } gatts; // 16 bytes (pointers only) - } event_; // 80 bytes + esp_ble_gatts_cb_param_t *gatts_param; // Heap-allocated + std::vector *data; // Heap-allocated + } gatts; // 16 bytes (pointers only) + } event_; // 80 bytes ble_event_t type_; diff --git a/esphome/components/esp32_ble/ble_scan_result.h b/esphome/components/esp32_ble/ble_scan_result.h index b46e9ea896..42e1789437 100644 --- a/esphome/components/esp32_ble/ble_scan_result.h +++ b/esphome/components/esp32_ble/ble_scan_result.h @@ -21,4 +21,4 @@ struct BLEScanResult { } // namespace esp32_ble } // namespace esphome -#endif \ No newline at end of file +#endif From c24b7cb7bdcf4d82004b329bba87f431b524a083 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:34:30 -0500 Subject: [PATCH 16/33] v->d --- esphome/components/esp32_ble/ble.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 00697ee15c..3a0e259972 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -354,7 +354,7 @@ void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_pa } if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGV(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); + ESP_LOGD(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); return; } @@ -377,7 +377,7 @@ void ESP32BLE::real_gap_event_handler_(esp_gap_ble_cb_event_t event, esp_ble_gap void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) { if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGV(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); + ESP_LOGD(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); return; } @@ -401,7 +401,7 @@ void ESP32BLE::real_gatts_event_handler_(esp_gatts_cb_event_t event, esp_gatt_if void ESP32BLE::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) { if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGV(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); + ESP_LOGD(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); return; } From e1c3862586c851491de2f90c6711d98649fe4f1a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:36:50 -0500 Subject: [PATCH 17/33] preen --- esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index d1f0c67e99..995f2ef18c 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -420,9 +420,6 @@ void ESP32BLETracker::gap_scan_event_handler(const BLEScanResult &scan_result) { } this->set_scanner_state_(ScannerState::STOPPED); } - - // Note: BLE clients don't actually process ESP_GAP_BLE_SCAN_RESULT_EVT - // They use parse_device() instead, so we don't need to forward scan results } void ESP32BLETracker::gap_scan_set_param_complete_(const esp_ble_gap_cb_param_t::ble_scan_param_cmpl_evt_param ¶m) { From bbc7c9fb37b7715d78ca2a87754a836318aaf61a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:46:17 -0500 Subject: [PATCH 18/33] dry --- esphome/components/esp32_ble/ble.cpp | 52 +++++++++------------------- esphome/components/esp32_ble/ble.h | 2 ++ 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 3a0e259972..85aab73fa1 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -345,14 +345,7 @@ void ESP32BLE::loop() { } } -void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { - // Only queue the 4 GAP events we actually handle - if (event != ESP_GAP_BLE_SCAN_RESULT_EVT && event != ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT && - event != ESP_GAP_BLE_SCAN_START_COMPLETE_EVT && event != ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { - ESP_LOGW(TAG, "Ignoring unexpected GAP event type: %d", event); - return; - } - +template static void enqueue_ble_event(Args... args) { if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { ESP_LOGD(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); return; @@ -363,10 +356,21 @@ void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_pa // Memory too fragmented to allocate new event. Can only drop it until memory comes back return; } - new (new_event) BLEEvent(event, param); + new (new_event) BLEEvent(args...); global_ble->ble_events_.push(new_event); } // NOLINT(clang-analyzer-unix.Malloc) +void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { + // Only queue the 4 GAP events we actually handle + if (event != ESP_GAP_BLE_SCAN_RESULT_EVT && event != ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT && + event != ESP_GAP_BLE_SCAN_START_COMPLETE_EVT && event != ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { + ESP_LOGW(TAG, "Ignoring unexpected GAP event type: %d", event); + return; + } + + enqueue_ble_event(event, param); +} + void ESP32BLE::real_gap_event_handler_(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { ESP_LOGV(TAG, "(BLE) gap_event_handler - %d", event); for (auto *gap_handler : this->gap_event_handlers_) { @@ -376,19 +380,8 @@ void ESP32BLE::real_gap_event_handler_(esp_gap_ble_cb_event_t event, esp_ble_gap void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) { - if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGD(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); - return; - } - - BLEEvent *new_event = EVENT_ALLOCATOR.allocate(1); - if (new_event == nullptr) { - // Memory too fragmented to allocate new event. Can only drop it until memory comes back - return; - } - new (new_event) BLEEvent(event, gatts_if, param); - global_ble->ble_events_.push(new_event); -} // NOLINT(clang-analyzer-unix.Malloc) + enqueue_ble_event(event, gatts_if, param); +} void ESP32BLE::real_gatts_event_handler_(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) { @@ -400,19 +393,8 @@ void ESP32BLE::real_gatts_event_handler_(esp_gatts_cb_event_t event, esp_gatt_if void ESP32BLE::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) { - if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGD(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); - return; - } - - BLEEvent *new_event = EVENT_ALLOCATOR.allocate(1); - if (new_event == nullptr) { - // Memory too fragmented to allocate new event. Can only drop it until memory comes back - return; - } - new (new_event) BLEEvent(event, gattc_if, param); - global_ble->ble_events_.push(new_event); -} // NOLINT(clang-analyzer-unix.Malloc) + enqueue_ble_event(event, gattc_if, param); +} void ESP32BLE::real_gattc_event_handler_(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) { diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index d33ebf0f59..18f5dd3111 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -139,6 +139,8 @@ class ESP32BLE : public Component { void advertising_init_(); private: + template friend void enqueue_ble_event(Args... args); + std::vector gap_event_handlers_; std::vector gap_scan_event_handlers_; std::vector gattc_event_handlers_; From 3c208050b0dbf9b852aa7739447dcfdf75779ee9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:47:34 -0500 Subject: [PATCH 19/33] comments --- esphome/components/esp32_ble/queue.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/esphome/components/esp32_ble/queue.h b/esphome/components/esp32_ble/queue.h index afa9a9b668..49b0ec5480 100644 --- a/esphome/components/esp32_ble/queue.h +++ b/esphome/components/esp32_ble/queue.h @@ -46,7 +46,13 @@ template class Queue { } size_t size() const { - return q_.size(); // Atomic read, no lock needed + // Lock-free size check. While std::queue::size() is not thread-safe, we intentionally + // avoid locking here to prevent blocking the BLE callback thread. The size is only + // used to decide whether to drop incoming events when the queue is near capacity. + // With a queue limit of 40-64 events and normal processing, dropping events should + // be extremely rare. When it does approach capacity, being off by 1-2 events is + // acceptable to avoid blocking the BLE stack's time-sensitive callbacks. + return q_.size(); } protected: From 67602799166a355c85d7fe40c209630e832da265 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:51:43 -0500 Subject: [PATCH 20/33] cleanup compacted code --- esphome/components/esp32_ble/ble.cpp | 52 ++++++++++++---------------- esphome/components/esp32_ble/ble.h | 4 --- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 85aab73fa1..501fc9d981 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -307,14 +307,26 @@ void ESP32BLE::loop() { BLEEvent *ble_event = this->ble_events_.pop(); while (ble_event != nullptr) { switch (ble_event->type_) { - case BLEEvent::GATTS: - this->real_gatts_event_handler_(ble_event->event_.gatts.gatts_event, ble_event->event_.gatts.gatts_if, - ble_event->event_.gatts.gatts_param); + case BLEEvent::GATTS: { + esp_gatts_cb_event_t event = ble_event->event_.gatts.gatts_event; + esp_gatt_if_t gatts_if = ble_event->event_.gatts.gatts_if; + esp_ble_gatts_cb_param_t *param = ble_event->event_.gatts.gatts_param; + ESP_LOGV(TAG, "gatts_event [esp_gatt_if: %d] - %d", gatts_if, event); + for (auto *gatts_handler : this->gatts_event_handlers_) { + gatts_handler->gatts_event_handler(event, gatts_if, param); + } break; - case BLEEvent::GATTC: - this->real_gattc_event_handler_(ble_event->event_.gattc.gattc_event, ble_event->event_.gattc.gattc_if, - ble_event->event_.gattc.gattc_param); + } + case BLEEvent::GATTC: { + esp_gattc_cb_event_t event = ble_event->event_.gattc.gattc_event; + esp_gatt_if_t gattc_if = ble_event->event_.gattc.gattc_if; + esp_ble_gattc_cb_param_t *param = ble_event->event_.gattc.gattc_param; + ESP_LOGV(TAG, "gattc_event [esp_gatt_if: %d] - %d", gattc_if, event); + for (auto *gattc_handler : this->gattc_event_handlers_) { + gattc_handler->gattc_event_handler(event, gattc_if, param); + } break; + } case BLEEvent::GAP: { esp_gap_ble_cb_event_t gap_event = ble_event->event_.gap.gap_event; if (gap_event == ESP_GAP_BLE_SCAN_RESULT_EVT) { @@ -328,7 +340,10 @@ void ESP32BLE::loop() { // All three scan complete events have the same structure with just status // Cast is safe because all three ESP-IDF event structures are identical with just status field // The scan_complete struct already contains our copy of the status (copied in BLEEvent constructor) - this->real_gap_event_handler_(gap_event, (esp_ble_gap_cb_param_t *) &ble_event->event_.gap.scan_complete); + ESP_LOGV(TAG, "gap_event_handler - %d", gap_event); + for (auto *gap_handler : this->gap_event_handlers_) { + gap_handler->gap_event_handler(gap_event, (esp_ble_gap_cb_param_t *) &ble_event->event_.gap.scan_complete); + } } break; } @@ -371,39 +386,16 @@ void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_pa enqueue_ble_event(event, param); } -void ESP32BLE::real_gap_event_handler_(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { - ESP_LOGV(TAG, "(BLE) gap_event_handler - %d", event); - for (auto *gap_handler : this->gap_event_handlers_) { - gap_handler->gap_event_handler(event, param); - } -} - void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) { enqueue_ble_event(event, gatts_if, param); } -void ESP32BLE::real_gatts_event_handler_(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, - esp_ble_gatts_cb_param_t *param) { - ESP_LOGV(TAG, "(BLE) gatts_event [esp_gatt_if: %d] - %d", gatts_if, event); - for (auto *gatts_handler : this->gatts_event_handlers_) { - gatts_handler->gatts_event_handler(event, gatts_if, param); - } -} - void ESP32BLE::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) { enqueue_ble_event(event, gattc_if, param); } -void ESP32BLE::real_gattc_event_handler_(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, - esp_ble_gattc_cb_param_t *param) { - ESP_LOGV(TAG, "(BLE) gattc_event [esp_gatt_if: %d] - %d", gattc_if, event); - for (auto *gattc_handler : this->gattc_event_handlers_) { - gattc_handler->gattc_event_handler(event, gattc_if, param); - } -} - float ESP32BLE::get_setup_priority() const { return setup_priority::BLUETOOTH; } void ESP32BLE::dump_config() { diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index 18f5dd3111..6508db1a00 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -129,10 +129,6 @@ class ESP32BLE : public Component { static void gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param); static void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param); - void real_gatts_event_handler_(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param); - void real_gattc_event_handler_(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param); - void real_gap_event_handler_(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param); - bool ble_setup_(); bool ble_dismantle_(); bool ble_pre_setup_(); From ae066d5627bf45a1b1af0e82b5520fc61df427a5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 11:55:28 -0500 Subject: [PATCH 21/33] cleanup --- .../esp32_ble_tracker/esp32_ble_tracker.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 995f2ef18c..da7b35658b 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -369,9 +369,6 @@ void ESP32BLETracker::recalculate_advertisement_parser_types() { void ESP32BLETracker::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { switch (event) { - case ESP_GAP_BLE_SCAN_RESULT_EVT: - // This will be handled by gap_scan_event_handler instead - break; case ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT: this->gap_scan_set_param_complete_(param->scan_param_cmpl); break; @@ -384,11 +381,9 @@ void ESP32BLETracker::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_ga default: break; } - // Still forward non-scan events to clients - if (event != ESP_GAP_BLE_SCAN_RESULT_EVT) { - for (auto *client : this->clients_) { - client->gap_event_handler(event, param); - } + // Forward all events to clients (scan results are handled separately via gap_scan_event_handler) + for (auto *client : this->clients_) { + client->gap_event_handler(event, param); } } From 8e51590c32341324cc80dcd5320d376ea4958b30 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 12:59:57 -0500 Subject: [PATCH 22/33] remove workaround --- esphome/components/logger/logger.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/esphome/components/logger/logger.cpp b/esphome/components/logger/logger.cpp index 59a3398ce8..a364b93cf5 100644 --- a/esphome/components/logger/logger.cpp +++ b/esphome/components/logger/logger.cpp @@ -130,15 +130,6 @@ inline int Logger::level_for(const char *tag) { } void HOT Logger::call_log_callbacks_(int level, const char *tag, const char *msg) { -#ifdef USE_ESP32 - // Suppress network-logging if memory constrained - // In some configurations (eg BLE enabled) there may be some transient - // memory exhaustion, and trying to log when OOM can lead to a crash. Skipping - // here usually allows the stack to recover instead. - // See issue #1234 for analysis. - if (xPortGetFreeHeapSize() < 2048) - return; -#endif this->log_callback_.call(level, tag, msg); } From 8fe6a323d82a9fbe266e3510e1d6afd852f78390 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 13:00:55 -0500 Subject: [PATCH 23/33] remove workaround --- esphome/components/logger/logger.cpp | 8 ++------ esphome/components/logger/logger.h | 3 +-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/esphome/components/logger/logger.cpp b/esphome/components/logger/logger.cpp index a364b93cf5..28a66b23b7 100644 --- a/esphome/components/logger/logger.cpp +++ b/esphome/components/logger/logger.cpp @@ -116,7 +116,7 @@ void Logger::log_vprintf_(int level, const char *tag, int line, const __FlashStr if (this->baud_rate_ > 0) { this->write_msg_(this->tx_buffer_ + msg_start); } - this->call_log_callbacks_(level, tag, this->tx_buffer_ + msg_start); + this->log_callback_.call(level, tag, this->tx_buffer_ + msg_start); global_recursion_guard_ = false; } @@ -129,10 +129,6 @@ inline int Logger::level_for(const char *tag) { return this->current_level_; } -void HOT Logger::call_log_callbacks_(int level, const char *tag, const char *msg) { - this->log_callback_.call(level, tag, msg); -} - Logger::Logger(uint32_t baud_rate, size_t tx_buffer_size) : baud_rate_(baud_rate), tx_buffer_size_(tx_buffer_size) { // add 1 to buffer size for null terminator this->tx_buffer_ = new char[this->tx_buffer_size_ + 1]; // NOLINT @@ -180,7 +176,7 @@ void Logger::loop() { this->tx_buffer_size_); this->write_footer_to_buffer_(this->tx_buffer_, &this->tx_buffer_at_, this->tx_buffer_size_); this->tx_buffer_[this->tx_buffer_at_] = '\0'; - this->call_log_callbacks_(message->level, message->tag, this->tx_buffer_); + this->log_callback_.call(message->level, message->tag, this->tx_buffer_); // At this point all the data we need from message has been transferred to the tx_buffer // so we can release the message to allow other tasks to use it as soon as possible. this->log_buffer_->release_message_main_loop(received_token); diff --git a/esphome/components/logger/logger.h b/esphome/components/logger/logger.h index 6030d9e8f2..9f09208b66 100644 --- a/esphome/components/logger/logger.h +++ b/esphome/components/logger/logger.h @@ -156,7 +156,6 @@ class Logger : public Component { #endif protected: - void call_log_callbacks_(int level, const char *tag, const char *msg); void write_msg_(const char *msg); // Format a log message with printf-style arguments and write it to a buffer with header, footer, and null terminator @@ -191,7 +190,7 @@ class Logger : public Component { if (this->baud_rate_ > 0) { this->write_msg_(this->tx_buffer_); // If logging is enabled, write to console } - this->call_log_callbacks_(level, tag, this->tx_buffer_); + this->log_callback_.call(level, tag, this->tx_buffer_); } // Write the body of the log message to the buffer From c6957c08bc958858e0b2d2faa7d35ee52ceb10c2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 13:02:08 -0500 Subject: [PATCH 24/33] lint --- esphome/components/esp32_ble/ble.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 501fc9d981..edf6f3254b 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -360,7 +360,7 @@ void ESP32BLE::loop() { } } -template static void enqueue_ble_event(Args... args) { +template void enqueue_ble_event(Args... args) { if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { ESP_LOGD(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); return; @@ -375,6 +375,11 @@ template static void enqueue_ble_event(Args... args) { global_ble->ble_events_.push(new_event); } // NOLINT(clang-analyzer-unix.Malloc) +// Explicit template instantiations for the friend function +template void enqueue_ble_event(esp_gap_ble_cb_event_t, esp_ble_gap_cb_param_t *); +template void enqueue_ble_event(esp_gatts_cb_event_t, esp_gatt_if_t, esp_ble_gatts_cb_param_t *); +template void enqueue_ble_event(esp_gattc_cb_event_t, esp_gatt_if_t, esp_ble_gattc_cb_param_t *); + void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { // Only queue the 4 GAP events we actually handle if (event != ESP_GAP_BLE_SCAN_RESULT_EVT && event != ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT && From 55ee0b116d6b7c768d8cb2b22ef54856d51467a8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 13:03:50 -0500 Subject: [PATCH 25/33] lint --- esphome/components/esp32_ble/ble.cpp | 7 ++++--- esphome/components/esp32_ble/ble_event.h | 4 +++- esphome/components/esp32_ble/ble_scan_result.h | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index edf6f3254b..3b561883f8 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -338,11 +338,12 @@ void ESP32BLE::loop() { gap_event == ESP_GAP_BLE_SCAN_START_COMPLETE_EVT || gap_event == ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { // All three scan complete events have the same structure with just status - // Cast is safe because all three ESP-IDF event structures are identical with just status field - // The scan_complete struct already contains our copy of the status (copied in BLEEvent constructor) + // The scan_complete struct matches ESP-IDF's layout exactly, so this reinterpret_cast is safe + // The struct already contains our copy of the status (copied in BLEEvent constructor) ESP_LOGV(TAG, "gap_event_handler - %d", gap_event); for (auto *gap_handler : this->gap_event_handlers_) { - gap_handler->gap_event_handler(gap_event, (esp_ble_gap_cb_param_t *) &ble_event->event_.gap.scan_complete); + gap_handler->gap_event_handler( + gap_event, reinterpret_cast(&ble_event->event_.gap.scan_complete)); } } break; diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index eb6453801f..e0178c41db 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -157,7 +157,9 @@ class BLEEvent { esp_gap_ble_cb_event_t gap_event; union { BLEScanResult scan_result; // 73 bytes - struct { + // This struct matches ESP-IDF's scan complete event structures + // All three (scan_param_cmpl, scan_start_cmpl, scan_stop_cmpl) have identical layout + struct ble_scan_complete_evt_param { esp_bt_status_t status; } scan_complete; // 1 byte }; diff --git a/esphome/components/esp32_ble/ble_scan_result.h b/esphome/components/esp32_ble/ble_scan_result.h index 42e1789437..49b0d5523d 100644 --- a/esphome/components/esp32_ble/ble_scan_result.h +++ b/esphome/components/esp32_ble/ble_scan_result.h @@ -8,7 +8,7 @@ namespace esphome { namespace esp32_ble { // Structure for BLE scan results - only fields we actually use -struct BLEScanResult { +struct __attribute__((packed)) BLEScanResult { esp_bd_addr_t bda; uint8_t ble_addr_type; int8_t rssi; From dc47faa4b6bcbf8471e9df3337eead71fce9e2e5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 13:05:01 -0500 Subject: [PATCH 26/33] safety --- esphome/components/esp32_ble/ble_event.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index e0178c41db..433eb4feda 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -46,6 +46,10 @@ class BLEEvent { this->type_ = GAP; this->event_.gap.gap_event = e; + if (p == nullptr) { + return; // Invalid event, but we can't log in header file + } + // Only copy the data we actually use for each GAP event type switch (e) { case ESP_GAP_BLE_SCAN_RESULT_EVT: @@ -85,6 +89,12 @@ class BLEEvent { this->event_.gattc.gattc_event = e; this->event_.gattc.gattc_if = i; + if (p == nullptr) { + this->event_.gattc.gattc_param = nullptr; + this->event_.gattc.data = nullptr; + return; // Invalid event, but we can't log in header file + } + // Heap-allocate param and data // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) // while GAP events (99%) are stored inline to minimize memory usage @@ -114,6 +124,12 @@ class BLEEvent { this->event_.gatts.gatts_event = e; this->event_.gatts.gatts_if = i; + if (p == nullptr) { + this->event_.gatts.gatts_param = nullptr; + this->event_.gatts.data = nullptr; + return; // Invalid event, but we can't log in header file + } + // Heap-allocate param and data // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) // while GAP events (99%) are stored inline to minimize memory usage From 66bd4c96c4e33382229c8142c98bf2ed24dd569b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 13:05:30 -0500 Subject: [PATCH 27/33] safety --- esphome/components/esp32_ble/queue.h | 1 + 1 file changed, 1 insertion(+) diff --git a/esphome/components/esp32_ble/queue.h b/esphome/components/esp32_ble/queue.h index 49b0ec5480..f69878bf6e 100644 --- a/esphome/components/esp32_ble/queue.h +++ b/esphome/components/esp32_ble/queue.h @@ -52,6 +52,7 @@ template class Queue { // With a queue limit of 40-64 events and normal processing, dropping events should // be extremely rare. When it does approach capacity, being off by 1-2 events is // acceptable to avoid blocking the BLE stack's time-sensitive callbacks. + // Trade-off: We prefer occasional dropped events over potential BLE stack delays. return q_.size(); } From 2cbb5c7d8e860ad44c53df9976c53c87b0d60774 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 13:16:44 -0500 Subject: [PATCH 28/33] fix error --- esphome/components/esp32_ble/ble_event.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 433eb4feda..effcb43aea 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -173,9 +173,9 @@ class BLEEvent { esp_gap_ble_cb_event_t gap_event; union { BLEScanResult scan_result; // 73 bytes - // This struct matches ESP-IDF's scan complete event structures + // This matches ESP-IDF's scan complete event structures // All three (scan_param_cmpl, scan_start_cmpl, scan_stop_cmpl) have identical layout - struct ble_scan_complete_evt_param { + struct { esp_bt_status_t status; } scan_complete; // 1 byte }; From 2b9b1d12e62c371260d8b60312284e4da2f37b3b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 13:32:47 -0500 Subject: [PATCH 29/33] lets be sure --- esphome/components/esp32_ble/ble.cpp | 1 + esphome/components/esp32_ble/ble_event.h | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 3b561883f8..41e4ddc930 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -339,6 +339,7 @@ void ESP32BLE::loop() { gap_event == ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { // All three scan complete events have the same structure with just status // The scan_complete struct matches ESP-IDF's layout exactly, so this reinterpret_cast is safe + // This is verified at compile-time by static_assert checks in ble_event.h // The struct already contains our copy of the status (copied in BLEEvent constructor) ESP_LOGV(TAG, "gap_event_handler - %d", gap_event); for (auto *gap_handler : this->gap_event_handlers_) { diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index effcb43aea..ae988f2a2a 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -13,6 +13,26 @@ namespace esphome { namespace esp32_ble { +// Compile-time verification that ESP-IDF scan complete events only contain a status field +// This ensures our reinterpret_cast in ble.cpp is safe +static_assert(sizeof(esp_ble_gap_cb_param_t::ble_scan_param_cmpl_evt_param) == sizeof(esp_bt_status_t), + "ESP-IDF scan_param_cmpl structure has unexpected size"); +static_assert(sizeof(esp_ble_gap_cb_param_t::ble_scan_start_cmpl_evt_param) == sizeof(esp_bt_status_t), + "ESP-IDF scan_start_cmpl structure has unexpected size"); +static_assert(sizeof(esp_ble_gap_cb_param_t::ble_scan_stop_cmpl_evt_param) == sizeof(esp_bt_status_t), + "ESP-IDF scan_stop_cmpl structure has unexpected size"); + +// Verify the status field is at offset 0 (first member) +static_assert(offsetof(esp_ble_gap_cb_param_t, scan_param_cmpl.status) == + offsetof(esp_ble_gap_cb_param_t, scan_param_cmpl), + "status must be first member of scan_param_cmpl"); +static_assert(offsetof(esp_ble_gap_cb_param_t, scan_start_cmpl.status) == + offsetof(esp_ble_gap_cb_param_t, scan_start_cmpl), + "status must be first member of scan_start_cmpl"); +static_assert(offsetof(esp_ble_gap_cb_param_t, scan_stop_cmpl.status) == + offsetof(esp_ble_gap_cb_param_t, scan_stop_cmpl), + "status must be first member of scan_stop_cmpl"); + // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). // This class stores each event with minimal memory usage. // GAP events (99% of traffic) don't have the vector overhead. From ee7d95272da8ccbe6ffca80e17280bf903c764a3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 13:32:55 -0500 Subject: [PATCH 30/33] lets be sure --- esphome/components/esp32_ble/ble_event.h | 1 + 1 file changed, 1 insertion(+) diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index ae988f2a2a..af70d2f899 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -2,6 +2,7 @@ #ifdef USE_ESP32 +#include // for offsetof #include #include From f7533dfc5cf96228b19ba09ef862da44e9158685 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Jun 2025 16:25:31 -0500 Subject: [PATCH 31/33] review --- esphome/components/esp32_ble/ble.cpp | 2 +- esphome/components/esp32_ble/ble_event.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 41e4ddc930..83c68f7843 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -364,7 +364,7 @@ void ESP32BLE::loop() { template void enqueue_ble_event(Args... args) { if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { - ESP_LOGD(TAG, "BLE event queue full (%d), dropping event", MAX_BLE_QUEUE_SIZE); + ESP_LOGD(TAG, "BLE event queue full (%zu), dropping event", MAX_BLE_QUEUE_SIZE); return; } diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index af70d2f899..f51095effd 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -60,8 +60,6 @@ class BLEEvent { GATTS, }; - BLEEvent() = default; - // Constructor for GAP events - no external allocations needed BLEEvent(esp_gap_ble_cb_event_t e, esp_ble_gap_cb_param_t *p) { this->type_ = GAP; From 0877b3e2af74965040468d8ead27136d795291ae Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 12 Jun 2025 19:18:22 -0500 Subject: [PATCH 32/33] suppress unused events --- esphome/components/esp32_ble/ble.cpp | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 83c68f7843..b4ba970bce 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -383,14 +383,22 @@ template void enqueue_ble_event(esp_gatts_cb_event_t, esp_gatt_if_t, esp_ble_gat template void enqueue_ble_event(esp_gattc_cb_event_t, esp_gatt_if_t, esp_ble_gattc_cb_param_t *); void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { - // Only queue the 4 GAP events we actually handle - if (event != ESP_GAP_BLE_SCAN_RESULT_EVT && event != ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT && - event != ESP_GAP_BLE_SCAN_START_COMPLETE_EVT && event != ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT) { - ESP_LOGW(TAG, "Ignoring unexpected GAP event type: %d", event); - return; - } + switch (event) { + // Only queue the 4 GAP events we actually handle + case ESP_GAP_BLE_SCAN_RESULT_EVT: + case ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT: + case ESP_GAP_BLE_SCAN_START_COMPLETE_EVT: + case ESP_GAP_BLE_SCAN_STOP_COMPLETE_EVT: + enqueue_ble_event(event, param); + return; - enqueue_ble_event(event, param); + // Ignore these GAP events as they are not relevant for our use case + case ESP_GAP_BLE_UPDATE_CONN_PARAMS_EVT: + case ESP_GAP_BLE_SET_PKT_LENGTH_COMPLETE_EVT: + + return; + } + ESP_LOGW(TAG, "Ignoring unexpected GAP event type: %d", event); } void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, From a8eb3f79615f347b343790eefc554c58101bb69c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 13 Jun 2025 10:46:09 -0500 Subject: [PATCH 33/33] lint --- esphome/components/esp32_ble/ble.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index b4ba970bce..0ddeccec17 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -395,8 +395,10 @@ void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_pa // Ignore these GAP events as they are not relevant for our use case case ESP_GAP_BLE_UPDATE_CONN_PARAMS_EVT: case ESP_GAP_BLE_SET_PKT_LENGTH_COMPLETE_EVT: - return; + + default: + break; } ESP_LOGW(TAG, "Ignoring unexpected GAP event type: %d", event); }