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..ed74d59ef2 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); @@ -304,20 +307,52 @@ 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: - 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) { + // 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_) { + gap_handler->gap_event_handler( + gap_event, reinterpret_cast(&ble_event->event_.gap.scan_complete)); + } + } break; + } 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(); @@ -327,59 +362,55 @@ void ESP32BLE::loop() { } } -void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { +template void enqueue_ble_event(Args... args) { + if (global_ble->ble_events_.size() >= MAX_BLE_QUEUE_SIZE) { + ESP_LOGD(TAG, "Event queue full (%zu), 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, param); + new (new_event) BLEEvent(args...); global_ble->ble_events_.push(new_event); } // NOLINT(clang-analyzer-unix.Malloc) -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); +// 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) { + 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; + + // 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); } void ESP32BLE::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) { - 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) - -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); - } + enqueue_ble_event(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) { - 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) - -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); - } + enqueue_ble_event(event, gattc_if, param); } float ESP32BLE::get_setup_priority() const { return setup_priority::BLUETOOTH; } diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index 13ec3b6dd9..6508db1a00 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 @@ -22,6 +23,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 +65,11 @@ class GAPEventHandler { virtual void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) = 0; }; +class GAPScanEventHandler { + public: + 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 +114,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) { @@ -113,16 +129,16 @@ 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_(); 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_; 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..f51095effd 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -2,92 +2,232 @@ #ifdef USE_ESP32 +#include // for offsetof #include #include #include #include +#include "ble_scan_result.h" + 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 in a single type. +// This class stores each event with minimal memory usage. +// GAP events (99% of traffic) don't have the vector overhead. +// 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 +// 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 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 { 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; - }; - - BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { - 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. - 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(); - 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(); - break; - 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->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. - 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(); - break; - 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; - - // 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; - - // 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_; - - std::vector data{}; // NOLINTNEXTLINE(readability-identifier-naming) enum ble_event_t : uint8_t { GAP, GATTC, GATTS, - } type_; + }; + + // 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; + + 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: + // 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; + 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: + // We only handle 4 GAP event types, others are dropped + break; + } + } + + // 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; + + 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 + 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->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->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 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; + + 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 + 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->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 heap 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; + union { + BLEScanResult scan_result; // 73 bytes + // This matches ESP-IDF's scan complete event structures + // All three (scan_param_cmpl, scan_start_cmpl, scan_stop_cmpl) have identical layout + struct { + esp_bt_status_t status; + } scan_complete; // 1 byte + }; + } 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; // 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; // Heap-allocated + std::vector *data; // Heap-allocated + } gatts; // 16 bytes (pointers only) + } event_; // 80 bytes + + 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; } }; +// BLEEvent total size: 84 bytes (80 byte union + 1 byte type + 3 bytes padding) + } // namespace esp32_ble } // namespace esphome 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..49b0d5523d --- /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 __attribute__((packed)) 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 diff --git a/esphome/components/esp32_ble/queue.h b/esphome/components/esp32_ble/queue.h index c98477e121..f69878bf6e 100644 --- a/esphome/components/esp32_ble/queue.h +++ b/esphome/components/esp32_ble/queue.h @@ -45,6 +45,17 @@ template class Queue { return element; } + size_t size() const { + // 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. + // Trade-off: We prefer occasional dropped events over potential BLE stack delays. + return q_.size(); + } + 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..ab3efc3ad3 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); + RAMAllocator allocator; + 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!"); @@ -124,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."); } @@ -370,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->gap_scan_result_(param->scan_rst); - break; case ESP_GAP_BLE_SCAN_PARAM_SET_COMPLETE_EVT: this->gap_scan_set_param_complete_(param->scan_param_cmpl); break; @@ -385,11 +381,42 @@ void ESP32BLETracker::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_ga default: break; } + // 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); } } +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); + } +} + void ESP32BLETracker::gap_scan_set_param_complete_(const esp_ble_gap_cb_param_t::ble_scan_param_cmpl_evt_param ¶m) { ESP_LOGV(TAG, "gap_scan_set_param_complete - status %d", param.status); if (param.status == ESP_BT_STATUS_DONE) { @@ -444,34 +471,6 @@ 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); - } -} - 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_) { @@ -494,13 +493,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:"); @@ -558,13 +559,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 eea73a7d26..33c0caaa87 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; @@ -121,9 +118,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 +205,7 @@ class ESPBTClient : public ESPBTDeviceListener { class ESP32BLETracker : public Component, public GAPEventHandler, + public GAPScanEventHandler, public GATTcEventHandler, public BLEStatusEventHandler, public Parented { @@ -240,6 +236,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 +284,7 @@ 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_; + 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}; diff --git a/esphome/components/logger/logger.cpp b/esphome/components/logger/logger.cpp index 59a3398ce8..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,19 +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) { -#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); -} - 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 @@ -189,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