mirror of
https://github.com/esphome/esphome.git
synced 2025-09-02 03:12:20 +01:00
[esp32_ble] Store GATTC/GATTS param and small data inline to nearly eliminate heap allocations (#10249)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
Jesse Hills
parent
07a98d2525
commit
cc6c892678
@@ -306,7 +306,7 @@ void ESP32BLE::loop() {
|
||||
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_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);
|
||||
@@ -316,7 +316,7 @@ void ESP32BLE::loop() {
|
||||
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_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);
|
||||
|
@@ -61,10 +61,24 @@ static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.rssi) == sizeof(es
|
||||
static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.remote_addr) == sizeof(esp_bt_status_t) + sizeof(int8_t),
|
||||
"remote_addr must follow rssi in read_rssi_cmpl");
|
||||
|
||||
// Param struct sizes on ESP32
|
||||
static constexpr size_t GATTC_PARAM_SIZE = 28;
|
||||
static constexpr size_t GATTS_PARAM_SIZE = 32;
|
||||
|
||||
// Maximum size for inline storage of data
|
||||
// GATTC: 80 - 28 (param) - 8 (other fields) = 44 bytes for data
|
||||
// GATTS: 80 - 32 (param) - 8 (other fields) = 40 bytes for data
|
||||
static constexpr size_t GATTC_INLINE_DATA_SIZE = 44;
|
||||
static constexpr size_t GATTS_INLINE_DATA_SIZE = 40;
|
||||
|
||||
// Verify param struct sizes
|
||||
static_assert(sizeof(esp_ble_gattc_cb_param_t) == GATTC_PARAM_SIZE, "GATTC param size unexpected");
|
||||
static_assert(sizeof(esp_ble_gatts_cb_param_t) == GATTS_PARAM_SIZE, "GATTS param size unexpected");
|
||||
|
||||
// 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 heap allocation overhead.
|
||||
// GATTC/GATTS events use heap allocation for their param and data.
|
||||
// GATTC/GATTS events use heap allocation for their param and inline storage for small data.
|
||||
//
|
||||
// Event flow:
|
||||
// 1. ESP-IDF BLE stack calls our static handlers in the BLE task context
|
||||
@@ -111,21 +125,21 @@ class BLEEvent {
|
||||
this->init_gap_data_(e, p);
|
||||
}
|
||||
|
||||
// Constructor for GATTC events - uses heap allocation
|
||||
// IMPORTANT: The heap allocation is REQUIRED and must not be removed as an optimization.
|
||||
// The param pointer from ESP-IDF is only valid during the callback execution.
|
||||
// Since BLE events are processed asynchronously in the main loop, we must create
|
||||
// our own copy to ensure the data remains valid until the event is processed.
|
||||
// Constructor for GATTC events - param stored inline, data may use heap
|
||||
// IMPORTANT: We MUST copy the param struct because the pointer from ESP-IDF
|
||||
// is only valid during the callback execution. Since BLE events are processed
|
||||
// asynchronously in the main loop, we store our own copy inline to ensure
|
||||
// the data remains valid until the event is processed.
|
||||
BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) {
|
||||
this->type_ = GATTC;
|
||||
this->init_gattc_data_(e, i, p);
|
||||
}
|
||||
|
||||
// Constructor for GATTS events - uses heap allocation
|
||||
// IMPORTANT: The heap allocation is REQUIRED and must not be removed as an optimization.
|
||||
// The param pointer from ESP-IDF is only valid during the callback execution.
|
||||
// Since BLE events are processed asynchronously in the main loop, we must create
|
||||
// our own copy to ensure the data remains valid until the event is processed.
|
||||
// Constructor for GATTS events - param stored inline, data may use heap
|
||||
// IMPORTANT: We MUST copy the param struct because the pointer from ESP-IDF
|
||||
// is only valid during the callback execution. Since BLE events are processed
|
||||
// asynchronously in the main loop, we store our own copy inline to ensure
|
||||
// the data remains valid until the event is processed.
|
||||
BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) {
|
||||
this->type_ = GATTS;
|
||||
this->init_gatts_data_(e, i, p);
|
||||
@@ -135,27 +149,32 @@ class BLEEvent {
|
||||
~BLEEvent() { this->release(); }
|
||||
|
||||
// Default constructor for pre-allocation in pool
|
||||
BLEEvent() : type_(GAP) {}
|
||||
BLEEvent() : event_{}, type_(GAP) {}
|
||||
|
||||
// Invoked on return to EventPool - clean up any heap-allocated data
|
||||
void release() {
|
||||
if (this->type_ == GAP) {
|
||||
return;
|
||||
}
|
||||
if (this->type_ == GATTC) {
|
||||
delete this->event_.gattc.gattc_param;
|
||||
delete[] this->event_.gattc.data;
|
||||
this->event_.gattc.gattc_param = nullptr;
|
||||
this->event_.gattc.data = nullptr;
|
||||
this->event_.gattc.data_len = 0;
|
||||
return;
|
||||
}
|
||||
if (this->type_ == GATTS) {
|
||||
delete this->event_.gatts.gatts_param;
|
||||
delete[] this->event_.gatts.data;
|
||||
this->event_.gatts.gatts_param = nullptr;
|
||||
this->event_.gatts.data = nullptr;
|
||||
this->event_.gatts.data_len = 0;
|
||||
switch (this->type_) {
|
||||
case GAP:
|
||||
// GAP events don't have heap allocations
|
||||
break;
|
||||
case GATTC:
|
||||
// Param is now stored inline, only delete heap data if it was heap-allocated
|
||||
if (!this->event_.gattc.is_inline && this->event_.gattc.data.heap_data != nullptr) {
|
||||
delete[] this->event_.gattc.data.heap_data;
|
||||
}
|
||||
// Clear critical fields to prevent issues if type changes
|
||||
this->event_.gattc.is_inline = false;
|
||||
this->event_.gattc.data.heap_data = nullptr;
|
||||
break;
|
||||
case GATTS:
|
||||
// Param is now stored inline, only delete heap data if it was heap-allocated
|
||||
if (!this->event_.gatts.is_inline && this->event_.gatts.data.heap_data != nullptr) {
|
||||
delete[] this->event_.gatts.data.heap_data;
|
||||
}
|
||||
// Clear critical fields to prevent issues if type changes
|
||||
this->event_.gatts.is_inline = false;
|
||||
this->event_.gatts.data.heap_data = nullptr;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -207,22 +226,30 @@ class BLEEvent {
|
||||
|
||||
// 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
|
||||
uint8_t *data; // Heap-allocated raw buffer (manually managed)
|
||||
uint16_t data_len; // Track size separately
|
||||
} gattc;
|
||||
esp_ble_gattc_cb_param_t gattc_param; // Stored inline (28 bytes)
|
||||
esp_gattc_cb_event_t gattc_event; // 4 bytes
|
||||
union {
|
||||
uint8_t *heap_data; // 4 bytes when heap-allocated
|
||||
uint8_t inline_data[GATTC_INLINE_DATA_SIZE]; // 44 bytes when stored inline
|
||||
} data; // 44 bytes total
|
||||
uint16_t data_len; // 2 bytes
|
||||
esp_gatt_if_t gattc_if; // 1 byte
|
||||
bool is_inline; // 1 byte - true when data is stored inline
|
||||
} gattc; // Total: 80 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; // Heap-allocated
|
||||
uint8_t *data; // Heap-allocated raw buffer (manually managed)
|
||||
uint16_t data_len; // Track size separately
|
||||
} gatts;
|
||||
} event_; // 80 bytes
|
||||
esp_ble_gatts_cb_param_t gatts_param; // Stored inline (32 bytes)
|
||||
esp_gatts_cb_event_t gatts_event; // 4 bytes
|
||||
union {
|
||||
uint8_t *heap_data; // 4 bytes when heap-allocated
|
||||
uint8_t inline_data[GATTS_INLINE_DATA_SIZE]; // 40 bytes when stored inline
|
||||
} data; // 40 bytes total
|
||||
uint16_t data_len; // 2 bytes
|
||||
esp_gatt_if_t gatts_if; // 1 byte
|
||||
bool is_inline; // 1 byte - true when data is stored inline
|
||||
} gatts; // Total: 80 bytes
|
||||
} event_; // 80 bytes
|
||||
|
||||
ble_event_t type_;
|
||||
|
||||
@@ -236,6 +263,29 @@ class BLEEvent {
|
||||
const esp_ble_sec_t &security() const { return event_.gap.security; }
|
||||
|
||||
private:
|
||||
// Helper to copy data with inline storage optimization
|
||||
template<typename EventStruct, size_t InlineSize>
|
||||
void copy_data_with_inline_storage_(EventStruct &event, const uint8_t *src_data, uint16_t len,
|
||||
uint8_t **param_value_ptr) {
|
||||
event.data_len = len;
|
||||
if (len > 0) {
|
||||
if (len <= InlineSize) {
|
||||
event.is_inline = true;
|
||||
memcpy(event.data.inline_data, src_data, len);
|
||||
*param_value_ptr = event.data.inline_data;
|
||||
} else {
|
||||
event.is_inline = false;
|
||||
event.data.heap_data = new uint8_t[len];
|
||||
memcpy(event.data.heap_data, src_data, len);
|
||||
*param_value_ptr = event.data.heap_data;
|
||||
}
|
||||
} else {
|
||||
event.is_inline = false;
|
||||
event.data.heap_data = nullptr;
|
||||
*param_value_ptr = nullptr;
|
||||
}
|
||||
}
|
||||
|
||||
// Initialize GAP event data
|
||||
void init_gap_data_(esp_gap_ble_cb_event_t e, esp_ble_gap_cb_param_t *p) {
|
||||
this->event_.gap.gap_event = e;
|
||||
@@ -320,48 +370,37 @@ class BLEEvent {
|
||||
this->event_.gattc.gattc_if = i;
|
||||
|
||||
if (p == nullptr) {
|
||||
this->event_.gattc.gattc_param = nullptr;
|
||||
this->event_.gattc.data = nullptr;
|
||||
// Zero out the param struct when null
|
||||
memset(&this->event_.gattc.gattc_param, 0, sizeof(this->event_.gattc.gattc_param));
|
||||
this->event_.gattc.is_inline = false;
|
||||
this->event_.gattc.data.heap_data = nullptr;
|
||||
this->event_.gattc.data_len = 0;
|
||||
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
|
||||
// IMPORTANT: This heap allocation provides clear ownership semantics:
|
||||
// - The BLEEvent owns the allocated memory for its lifetime
|
||||
// - The data remains valid from the BLE callback context until processed in the main loop
|
||||
// - Without this copy, we'd have use-after-free bugs as ESP-IDF reuses the callback memory
|
||||
this->event_.gattc.gattc_param = new esp_ble_gattc_cb_param_t(*p);
|
||||
// Copy param struct inline (no heap allocation!)
|
||||
// GATTC/GATTS events are rare (<1% of events) but we can still store them inline
|
||||
// along with small data payloads, eliminating all heap allocations for typical BLE operations
|
||||
// CRITICAL: This copy is REQUIRED for memory safety - the ESP-IDF param pointer
|
||||
// is only valid during the callback and will be reused/freed after we return
|
||||
this->event_.gattc.gattc_param = *p;
|
||||
|
||||
// Copy data for events that need it
|
||||
// The param struct contains pointers (e.g., notify.value) that point to temporary buffers.
|
||||
// We must copy this data to ensure it remains valid when the event is processed later.
|
||||
switch (e) {
|
||||
case ESP_GATTC_NOTIFY_EVT:
|
||||
this->event_.gattc.data_len = p->notify.value_len;
|
||||
if (p->notify.value_len > 0) {
|
||||
this->event_.gattc.data = new uint8_t[p->notify.value_len];
|
||||
memcpy(this->event_.gattc.data, p->notify.value, p->notify.value_len);
|
||||
} else {
|
||||
this->event_.gattc.data = nullptr;
|
||||
}
|
||||
this->event_.gattc.gattc_param->notify.value = this->event_.gattc.data;
|
||||
copy_data_with_inline_storage_<decltype(this->event_.gattc), GATTC_INLINE_DATA_SIZE>(
|
||||
this->event_.gattc, p->notify.value, p->notify.value_len, &this->event_.gattc.gattc_param.notify.value);
|
||||
break;
|
||||
case ESP_GATTC_READ_CHAR_EVT:
|
||||
case ESP_GATTC_READ_DESCR_EVT:
|
||||
this->event_.gattc.data_len = p->read.value_len;
|
||||
if (p->read.value_len > 0) {
|
||||
this->event_.gattc.data = new uint8_t[p->read.value_len];
|
||||
memcpy(this->event_.gattc.data, p->read.value, p->read.value_len);
|
||||
} else {
|
||||
this->event_.gattc.data = nullptr;
|
||||
}
|
||||
this->event_.gattc.gattc_param->read.value = this->event_.gattc.data;
|
||||
copy_data_with_inline_storage_<decltype(this->event_.gattc), GATTC_INLINE_DATA_SIZE>(
|
||||
this->event_.gattc, p->read.value, p->read.value_len, &this->event_.gattc.gattc_param.read.value);
|
||||
break;
|
||||
default:
|
||||
this->event_.gattc.data = nullptr;
|
||||
this->event_.gattc.is_inline = false;
|
||||
this->event_.gattc.data.heap_data = nullptr;
|
||||
this->event_.gattc.data_len = 0;
|
||||
break;
|
||||
}
|
||||
@@ -373,37 +412,32 @@ class BLEEvent {
|
||||
this->event_.gatts.gatts_if = i;
|
||||
|
||||
if (p == nullptr) {
|
||||
this->event_.gatts.gatts_param = nullptr;
|
||||
this->event_.gatts.data = nullptr;
|
||||
// Zero out the param struct when null
|
||||
memset(&this->event_.gatts.gatts_param, 0, sizeof(this->event_.gatts.gatts_param));
|
||||
this->event_.gatts.is_inline = false;
|
||||
this->event_.gatts.data.heap_data = nullptr;
|
||||
this->event_.gatts.data_len = 0;
|
||||
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
|
||||
// IMPORTANT: This heap allocation provides clear ownership semantics:
|
||||
// - The BLEEvent owns the allocated memory for its lifetime
|
||||
// - The data remains valid from the BLE callback context until processed in the main loop
|
||||
// - Without this copy, we'd have use-after-free bugs as ESP-IDF reuses the callback memory
|
||||
this->event_.gatts.gatts_param = new esp_ble_gatts_cb_param_t(*p);
|
||||
// Copy param struct inline (no heap allocation!)
|
||||
// GATTC/GATTS events are rare (<1% of events) but we can still store them inline
|
||||
// along with small data payloads, eliminating all heap allocations for typical BLE operations
|
||||
// CRITICAL: This copy is REQUIRED for memory safety - the ESP-IDF param pointer
|
||||
// is only valid during the callback and will be reused/freed after we return
|
||||
this->event_.gatts.gatts_param = *p;
|
||||
|
||||
// Copy data for events that need it
|
||||
// The param struct contains pointers (e.g., write.value) that point to temporary buffers.
|
||||
// We must copy this data to ensure it remains valid when the event is processed later.
|
||||
switch (e) {
|
||||
case ESP_GATTS_WRITE_EVT:
|
||||
this->event_.gatts.data_len = p->write.len;
|
||||
if (p->write.len > 0) {
|
||||
this->event_.gatts.data = new uint8_t[p->write.len];
|
||||
memcpy(this->event_.gatts.data, p->write.value, p->write.len);
|
||||
} else {
|
||||
this->event_.gatts.data = nullptr;
|
||||
}
|
||||
this->event_.gatts.gatts_param->write.value = this->event_.gatts.data;
|
||||
copy_data_with_inline_storage_<decltype(this->event_.gatts), GATTS_INLINE_DATA_SIZE>(
|
||||
this->event_.gatts, p->write.value, p->write.len, &this->event_.gatts.gatts_param.write.value);
|
||||
break;
|
||||
default:
|
||||
this->event_.gatts.data = nullptr;
|
||||
this->event_.gatts.is_inline = false;
|
||||
this->event_.gatts.data.heap_data = nullptr;
|
||||
this->event_.gatts.data_len = 0;
|
||||
break;
|
||||
}
|
||||
@@ -414,6 +448,15 @@ class BLEEvent {
|
||||
// The gap member in the union should be 80 bytes (including the gap_event enum)
|
||||
static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)) <= 80, "gap_event struct has grown beyond 80 bytes");
|
||||
|
||||
// Verify GATTC and GATTS structs don't exceed GAP struct size
|
||||
// This ensures the union size is determined by GAP (the most common event type)
|
||||
static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gattc)) <=
|
||||
sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)),
|
||||
"gattc_event struct exceeds gap_event size - union size would increase");
|
||||
static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gatts)) <=
|
||||
sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)),
|
||||
"gatts_event struct exceeds gap_event size - union size would increase");
|
||||
|
||||
// Verify esp_ble_sec_t fits within our union
|
||||
static_assert(sizeof(esp_ble_sec_t) <= 73, "esp_ble_sec_t is larger than BLEScanResult");
|
||||
|
||||
|
Reference in New Issue
Block a user