From 01899d70bc838f92bd926a1b911d00704d000f4e Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Fri, 24 May 2024 16:48:23 +1200 Subject: [PATCH] [esp32_ble] Attempt to manage memory better with shared_ptr --- esphome/components/esp32_ble/ble_uuid.cpp | 57 ++++++------ esphome/components/esp32_ble/ble_uuid.h | 9 +- .../components/esp32_ble_server/ble_2901.cpp | 3 + .../components/esp32_ble_server/ble_2902.cpp | 5 ++ .../esp32_ble_server/ble_characteristic.cpp | 49 +++++++--- .../esp32_ble_server/ble_characteristic.h | 23 +++-- .../esp32_ble_server/ble_descriptor.cpp | 21 ++++- .../esp32_ble_server/ble_descriptor.h | 4 +- .../esp32_ble_server/ble_server.cpp | 59 ++++++------ .../components/esp32_ble_server/ble_server.h | 13 +-- .../esp32_ble_server/ble_service.cpp | 36 ++++---- .../components/esp32_ble_server/ble_service.h | 23 ++--- .../esp32_improv/esp32_improv_component.cpp | 89 ++++++++++++++----- .../esp32_improv/esp32_improv_component.h | 14 +-- 14 files changed, 260 insertions(+), 145 deletions(-) diff --git a/esphome/components/esp32_ble/ble_uuid.cpp b/esphome/components/esp32_ble/ble_uuid.cpp index 57c2f9df94..bcc08d8022 100644 --- a/esphome/components/esp32_ble/ble_uuid.cpp +++ b/esphome/components/esp32_ble/ble_uuid.cpp @@ -2,9 +2,9 @@ #ifdef USE_ESP32 -#include -#include #include +#include +#include #include "esphome/core/log.h" namespace esphome { @@ -31,14 +31,15 @@ ESPBTUUID ESPBTUUID::from_raw(const uint8_t *data) { memcpy(ret.uuid_.uuid.uuid128, data, ESP_UUID_LEN_128); return ret; } -ESPBTUUID ESPBTUUID::from_raw(const std::string &data) { +ESPBTUUID ESPBTUUID::from_raw(const std::string &data) { return ESPBTUUID::from_raw(data.data(), data.length()); } +ESPBTUUID ESPBTUUID::from_raw(const char *data, size_t len) { ESPBTUUID ret; - if (data.length() == 4) { + if (len == 4) { ret.uuid_.len = ESP_UUID_LEN_16; ret.uuid_.uuid.uuid16 = 0; - for (int i = 0; i < data.length();) { - uint8_t msb = data.c_str()[i]; - uint8_t lsb = data.c_str()[i + 1]; + for (int i = 0; i < len;) { + uint8_t msb = data[i]; + uint8_t lsb = data[i + 1]; if (msb > '9') msb -= 7; @@ -47,12 +48,12 @@ ESPBTUUID ESPBTUUID::from_raw(const std::string &data) { ret.uuid_.uuid.uuid16 += (((msb & 0x0F) << 4) | (lsb & 0x0F)) << (2 - i) * 4; i += 2; } - } else if (data.length() == 8) { + } else if (len == 8) { ret.uuid_.len = ESP_UUID_LEN_32; ret.uuid_.uuid.uuid32 = 0; - for (int i = 0; i < data.length();) { - uint8_t msb = data.c_str()[i]; - uint8_t lsb = data.c_str()[i + 1]; + for (int i = 0; i < len;) { + uint8_t msb = data[i]; + uint8_t lsb = data[i + 1]; if (msb > '9') msb -= 7; @@ -61,20 +62,20 @@ ESPBTUUID ESPBTUUID::from_raw(const std::string &data) { ret.uuid_.uuid.uuid32 += (((msb & 0x0F) << 4) | (lsb & 0x0F)) << (6 - i) * 4; i += 2; } - } else if (data.length() == 16) { // how we can have 16 byte length string reprezenting 128 bit uuid??? needs to be - // investigated (lack of time) + } else if (len == 16) { // how we can have 16 byte length string reprezenting 128 bit uuid??? needs to be + // investigated (lack of time) ret.uuid_.len = ESP_UUID_LEN_128; - memcpy(ret.uuid_.uuid.uuid128, (uint8_t *) data.data(), 16); - } else if (data.length() == 36) { + memcpy(ret.uuid_.uuid.uuid128, (uint8_t *) data, 16); + } else if (len == 36) { // If the length of the string is 36 bytes then we will assume it is a long hex string in // UUID format. ret.uuid_.len = ESP_UUID_LEN_128; int n = 0; - for (int i = 0; i < data.length();) { - if (data.c_str()[i] == '-') + for (int i = 0; i < len;) { + if (data[i] == '-') i++; - uint8_t msb = data.c_str()[i]; - uint8_t lsb = data.c_str()[i + 1]; + uint8_t msb = data[i]; + uint8_t lsb = data[i + 1]; if (msb > '9') msb -= 7; @@ -84,7 +85,7 @@ ESPBTUUID ESPBTUUID::from_raw(const std::string &data) { i += 2; } } else { - ESP_LOGE(TAG, "ERROR: UUID value not 2, 4, 16 or 36 bytes - %s", data.c_str()); + ESP_LOGE(TAG, "ERROR: UUID value not 2, 4, 16 or 36 bytes - %s", data); } return ret; } @@ -162,14 +163,19 @@ bool ESPBTUUID::operator==(const ESPBTUUID &uuid) const { return false; } esp_bt_uuid_t ESPBTUUID::get_uuid() const { return this->uuid_; } -std::string ESPBTUUID::to_string() const { +std::string ESPBTUUID::to_string() { + if (!this->string_.empty()) + return this->string_; + switch (this->uuid_.len) { case ESP_UUID_LEN_16: - return str_snprintf("0x%02X%02X", 6, this->uuid_.uuid.uuid16 >> 8, this->uuid_.uuid.uuid16 & 0xff); + this->string_ = str_snprintf("0x%02X%02X", 6, this->uuid_.uuid.uuid16 >> 8, this->uuid_.uuid.uuid16 & 0xff); + return this->string_; case ESP_UUID_LEN_32: - return str_snprintf("0x%02" PRIX32 "%02" PRIX32 "%02" PRIX32 "%02" PRIX32, 10, (this->uuid_.uuid.uuid32 >> 24), - (this->uuid_.uuid.uuid32 >> 16 & 0xff), (this->uuid_.uuid.uuid32 >> 8 & 0xff), - this->uuid_.uuid.uuid32 & 0xff); + this->string_ = str_snprintf("0x%02" PRIX32 "%02" PRIX32 "%02" PRIX32 "%02" PRIX32, 10, + (this->uuid_.uuid.uuid32 >> 24), (this->uuid_.uuid.uuid32 >> 16 & 0xff), + (this->uuid_.uuid.uuid32 >> 8 & 0xff), this->uuid_.uuid.uuid32 & 0xff); + return this->string_; default: case ESP_UUID_LEN_128: std::string buf; @@ -178,6 +184,7 @@ std::string ESPBTUUID::to_string() const { if (i == 6 || i == 8 || i == 10 || i == 12) buf += "-"; } + this->string_ = buf; return buf; } return ""; diff --git a/esphome/components/esp32_ble/ble_uuid.h b/esphome/components/esp32_ble/ble_uuid.h index 790a57c59d..0469f06f60 100644 --- a/esphome/components/esp32_ble/ble_uuid.h +++ b/esphome/components/esp32_ble/ble_uuid.h @@ -1,12 +1,12 @@ #pragma once -#include "esphome/core/helpers.h" #include "esphome/core/hal.h" +#include "esphome/core/helpers.h" #ifdef USE_ESP32 -#include #include +#include namespace esphome { namespace esp32_ble { @@ -23,6 +23,8 @@ class ESPBTUUID { static ESPBTUUID from_raw(const std::string &data); + static ESPBTUUID from_raw(const char *uuid, size_t len); + static ESPBTUUID from_uuid(esp_bt_uuid_t uuid); ESPBTUUID as_128bit() const; @@ -34,10 +36,11 @@ class ESPBTUUID { esp_bt_uuid_t get_uuid() const; - std::string to_string() const; + std::string to_string(); protected: esp_bt_uuid_t uuid_; + std::string string_; }; } // namespace esp32_ble diff --git a/esphome/components/esp32_ble_server/ble_2901.cpp b/esphome/components/esp32_ble_server/ble_2901.cpp index ee0808d2c4..d12c2034d1 100644 --- a/esphome/components/esp32_ble_server/ble_2901.cpp +++ b/esphome/components/esp32_ble_server/ble_2901.cpp @@ -8,6 +8,9 @@ namespace esp32_ble_server { BLE2901::BLE2901(const std::string &value) : BLE2901((uint8_t *) value.data(), value.length()) {} BLE2901::BLE2901(const uint8_t *data, size_t length) : BLEDescriptor(esp32_ble::ESPBTUUID::from_uint16(0x2901)) { + if (this->state_ == FAILED) { + return; + } this->set_value(data, length); this->permissions_ = ESP_GATT_PERM_READ; } diff --git a/esphome/components/esp32_ble_server/ble_2902.cpp b/esphome/components/esp32_ble_server/ble_2902.cpp index 2f34573c37..1ccd28b4fc 100644 --- a/esphome/components/esp32_ble_server/ble_2902.cpp +++ b/esphome/components/esp32_ble_server/ble_2902.cpp @@ -1,6 +1,8 @@ #include "ble_2902.h" #include "esphome/components/esp32_ble/ble_uuid.h" +#include "esphome/core/log.h" + #ifdef USE_ESP32 #include @@ -9,6 +11,9 @@ namespace esphome { namespace esp32_ble_server { BLE2902::BLE2902() : BLEDescriptor(esp32_ble::ESPBTUUID::from_uint16(0x2902)) { + if (this->state_ == FAILED) { + return; + } this->value_.attr_len = 2; uint8_t data[2] = {0, 0}; memcpy(this->value_.attr_value, data, 2); diff --git a/esphome/components/esp32_ble_server/ble_characteristic.cpp b/esphome/components/esp32_ble_server/ble_characteristic.cpp index 6ff7d615f9..c2527d16dd 100644 --- a/esphome/components/esp32_ble_server/ble_characteristic.cpp +++ b/esphome/components/esp32_ble_server/ble_characteristic.cpp @@ -1,4 +1,6 @@ #include "ble_characteristic.h" +#include "ble_2901.h" +#include "ble_2902.h" #include "ble_server.h" #include "ble_service.h" @@ -12,14 +14,17 @@ namespace esp32_ble_server { static const char *const TAG = "esp32_ble_server.characteristic"; BLECharacteristic::~BLECharacteristic() { - for (auto *descriptor : this->descriptors_) { - delete descriptor; // NOLINT(cppcoreguidelines-owning-memory) - } + this->descriptors_.clear(); vSemaphoreDelete(this->set_value_lock_); } BLECharacteristic::BLECharacteristic(const ESPBTUUID uuid, uint32_t properties) : uuid_(uuid) { this->set_value_lock_ = xSemaphoreCreateBinary(); + if (this->set_value_lock_ == nullptr) { + ESP_LOGE(TAG, "Failed to create set_value_lock_ semaphore"); + this->state_ = FAILED; + return; + } xSemaphoreGive(this->set_value_lock_); this->properties_ = (esp_gatt_char_prop_t) 0; @@ -103,14 +108,36 @@ void BLECharacteristic::notify(bool notification) { } } -void BLECharacteristic::add_descriptor(BLEDescriptor *descriptor) { this->descriptors_.push_back(descriptor); } +void BLECharacteristic::add_descriptor(std::shared_ptr descriptor) { + this->descriptors_.push_back(descriptor); +} -void BLECharacteristic::remove_descriptor(BLEDescriptor *descriptor) { +void BLECharacteristic::remove_descriptor(std::shared_ptr descriptor) { this->descriptors_.erase(std::remove(this->descriptors_.begin(), this->descriptors_.end(), descriptor), this->descriptors_.end()); } -void BLECharacteristic::do_create(BLEService *service) { +std::shared_ptr BLECharacteristic::make_2901_descriptor(const std::string &value) { + auto descriptor = std::make_shared(value); + if (descriptor == nullptr) { + ESP_LOGE(TAG, "Failed to allocate BLE2901 descriptor"); + return nullptr; + } + this->add_descriptor(descriptor); + return descriptor; +} + +std::shared_ptr BLECharacteristic::make_2902_descriptor() { + auto descriptor = std::make_shared(); + if (descriptor == nullptr) { + ESP_LOGE(TAG, "Failed to allocate BLE2902 descriptor"); + return nullptr; + } + this->add_descriptor(descriptor); + return descriptor; +} + +void BLECharacteristic::do_create(std::shared_ptr service) { this->service_ = service; esp_attr_control_t control; control.auto_rsp = ESP_GATT_RSP_BY_APP; @@ -137,7 +164,7 @@ bool BLECharacteristic::is_created() { return false; bool created = true; - for (auto *descriptor : this->descriptors_) { + for (auto descriptor : this->descriptors_) { created &= descriptor->is_created(); } if (created) @@ -150,7 +177,7 @@ bool BLECharacteristic::is_failed() { return true; bool failed = false; - for (auto *descriptor : this->descriptors_) { + for (auto descriptor : this->descriptors_) { failed |= descriptor->is_failed(); } if (failed) @@ -208,8 +235,8 @@ void BLECharacteristic::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt if (this->uuid_ == ESPBTUUID::from_uuid(param->add_char.char_uuid)) { this->handle_ = param->add_char.attr_handle; - for (auto *descriptor : this->descriptors_) { - descriptor->do_create(this); + for (auto descriptor : this->descriptors_) { + descriptor->do_create(shared_from_this()); } this->state_ = CREATING_DEPENDENTS; @@ -313,7 +340,7 @@ void BLECharacteristic::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt break; } - for (auto *descriptor : this->descriptors_) { + for (auto descriptor : this->descriptors_) { descriptor->gatts_event_handler(event, gatts_if, param); } } diff --git a/esphome/components/esp32_ble_server/ble_characteristic.h b/esphome/components/esp32_ble_server/ble_characteristic.h index 8837c796a5..bc027d77bb 100644 --- a/esphome/components/esp32_ble_server/ble_characteristic.h +++ b/esphome/components/esp32_ble_server/ble_characteristic.h @@ -1,17 +1,21 @@ #pragma once +#include "ble_2901.h" +#include "ble_2902.h" #include "ble_descriptor.h" + #include "esphome/components/esp32_ble/ble_uuid.h" +#include #include #ifdef USE_ESP32 +#include #include #include #include #include -#include #include #include @@ -22,7 +26,7 @@ using namespace esp32_ble; class BLEService; -class BLECharacteristic { +class BLECharacteristic : public std::enable_shared_from_this { public: BLECharacteristic(ESPBTUUID uuid, uint32_t properties); ~BLECharacteristic(); @@ -47,15 +51,18 @@ class BLECharacteristic { void notify(bool notification = true); - void do_create(BLEService *service); + void do_create(std::shared_ptr service); void gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param); void on_write(const std::function &)> &&func) { this->on_write_ = func; } - void add_descriptor(BLEDescriptor *descriptor); - void remove_descriptor(BLEDescriptor *descriptor); + void add_descriptor(std::shared_ptr descriptor); + void remove_descriptor(std::shared_ptr descriptor); - BLEService *get_service() { return this->service_; } + std::shared_ptr make_2901_descriptor(const std::string &value); + std::shared_ptr make_2902_descriptor(); + + std::shared_ptr get_service() { return this->service_; } ESPBTUUID get_uuid() { return this->uuid_; } std::vector &get_value() { return this->value_; } @@ -71,7 +78,7 @@ class BLECharacteristic { protected: bool write_event_{false}; - BLEService *service_; + std::shared_ptr service_; ESPBTUUID uuid_; esp_gatt_char_prop_t properties_; uint16_t handle_{0xFFFF}; @@ -80,7 +87,7 @@ class BLECharacteristic { std::vector value_; SemaphoreHandle_t set_value_lock_; - std::vector descriptors_; + std::vector> descriptors_; std::function &)> on_write_; diff --git a/esphome/components/esp32_ble_server/ble_descriptor.cpp b/esphome/components/esp32_ble_server/ble_descriptor.cpp index bfb6224335..b69081af35 100644 --- a/esphome/components/esp32_ble_server/ble_descriptor.cpp +++ b/esphome/components/esp32_ble_server/ble_descriptor.cpp @@ -1,8 +1,12 @@ #include "ble_descriptor.h" #include "ble_characteristic.h" #include "ble_service.h" + +#include "esphome/core/helpers.h" #include "esphome/core/log.h" +#include "esphome/core/application.h" + #include #ifdef USE_ESP32 @@ -16,12 +20,23 @@ BLEDescriptor::BLEDescriptor(ESPBTUUID uuid, uint16_t max_len) { this->uuid_ = uuid; this->value_.attr_len = 0; this->value_.attr_max_len = max_len; - this->value_.attr_value = (uint8_t *) malloc(max_len); // NOLINT + + ExternalRAMAllocator allocator(ExternalRAMAllocator::ALLOW_FAILURE); + this->value_.attr_value = allocator.allocate(max_len); + if (this->value_.attr_value == nullptr) { + ESP_LOGE(TAG, "Failed to allocate %d bytes for value", max_len); + this->state_ = FAILED; + return; + } } -BLEDescriptor::~BLEDescriptor() { free(this->value_.attr_value); } // NOLINT +BLEDescriptor::~BLEDescriptor() { + ExternalRAMAllocator deallocator(ExternalRAMAllocator::ALLOW_FAILURE); + deallocator.deallocate(this->value_.attr_value, this->value_.attr_max_len); + this->value_.attr_value = nullptr; +} -void BLEDescriptor::do_create(BLECharacteristic *characteristic) { +void BLEDescriptor::do_create(std::shared_ptr characteristic) { this->characteristic_ = characteristic; esp_attr_control_t control; control.auto_rsp = ESP_GATT_AUTO_RSP; diff --git a/esphome/components/esp32_ble_server/ble_descriptor.h b/esphome/components/esp32_ble_server/ble_descriptor.h index 4b8fb345c3..fdb9fa5f53 100644 --- a/esphome/components/esp32_ble_server/ble_descriptor.h +++ b/esphome/components/esp32_ble_server/ble_descriptor.h @@ -18,7 +18,7 @@ class BLEDescriptor { public: BLEDescriptor(ESPBTUUID uuid, uint16_t max_len = 100); virtual ~BLEDescriptor(); - void do_create(BLECharacteristic *characteristic); + void do_create(std::shared_ptr characteristic); void set_value(const std::string &value); void set_value(const uint8_t *data, size_t length); @@ -29,7 +29,7 @@ class BLEDescriptor { bool is_failed() { return this->state_ == FAILED; } protected: - BLECharacteristic *characteristic_{nullptr}; + std::shared_ptr characteristic_{nullptr}; ESPBTUUID uuid_; uint16_t handle_{0xFFFF}; diff --git a/esphome/components/esp32_ble_server/ble_server.cpp b/esphome/components/esp32_ble_server/ble_server.cpp index 338413f64e..2b204a1524 100644 --- a/esphome/components/esp32_ble_server/ble_server.cpp +++ b/esphome/components/esp32_ble_server/ble_server.cpp @@ -1,18 +1,18 @@ #include "ble_server.h" #include "esphome/components/esp32_ble/ble.h" -#include "esphome/core/log.h" #include "esphome/core/application.h" +#include "esphome/core/log.h" #include "esphome/core/version.h" #ifdef USE_ESP32 -#include -#include -#include #include -#include +#include #include +#include +#include +#include namespace esphome { namespace esp32_ble_server { @@ -58,9 +58,8 @@ void BLEServer::loop() { pair.second->do_create(this); } if (this->device_information_service_ == nullptr) { - this->create_service(ESPBTUUID::from_uint16(DEVICE_INFORMATION_SERVICE_UUID)); this->device_information_service_ = - this->get_service(ESPBTUUID::from_uint16(DEVICE_INFORMATION_SERVICE_UUID)); + this->create_service(ESPBTUUID::from_uint16(DEVICE_INFORMATION_SERVICE_UUID)); this->create_device_characteristics_(); } this->state_ = STARTING_SERVICE; @@ -94,56 +93,58 @@ void BLEServer::restart_advertising_() { } bool BLEServer::create_device_characteristics_() { + std::shared_ptr model = + this->device_information_service_->create_characteristic(MODEL_UUID, BLECharacteristic::PROPERTY_READ); if (this->model_.has_value()) { - BLECharacteristic *model = - this->device_information_service_->create_characteristic(MODEL_UUID, BLECharacteristic::PROPERTY_READ); model->set_value(this->model_.value()); } else { - BLECharacteristic *model = - this->device_information_service_->create_characteristic(MODEL_UUID, BLECharacteristic::PROPERTY_READ); model->set_value(ESPHOME_BOARD); } - BLECharacteristic *version = + std::shared_ptr version = this->device_information_service_->create_characteristic(VERSION_UUID, BLECharacteristic::PROPERTY_READ); version->set_value("ESPHome " ESPHOME_VERSION); - BLECharacteristic *manufacturer = + std::shared_ptr manufacturer = this->device_information_service_->create_characteristic(MANUFACTURER_UUID, BLECharacteristic::PROPERTY_READ); manufacturer->set_value(this->manufacturer_); return true; } -void BLEServer::create_service(ESPBTUUID uuid, bool advertise, uint16_t num_handles, uint8_t inst_id) { - ESP_LOGV(TAG, "Creating BLE service - %s", uuid.to_string().c_str()); +std::shared_ptr BLEServer::create_service(ESPBTUUID uuid, bool advertise, uint16_t num_handles, + uint8_t inst_id) { + std::string uuid_str = uuid.to_string(); // If the service already exists, do nothing - BLEService *service = this->get_service(uuid); + std::shared_ptr service = this->get_service(uuid); if (service != nullptr) { - ESP_LOGW(TAG, "BLE service %s already exists", uuid.to_string().c_str()); - return; + ESP_LOGW(TAG, "BLE service %s already exists", uuid_str.c_str()); + return service; } - service = new BLEService(uuid, num_handles, inst_id, advertise); // NOLINT(cppcoreguidelines-owning-memory) - this->services_.emplace(uuid.to_string(), service); + ESP_LOGV(TAG, "Creating BLE service - %s", uuid_str.c_str()); + service = std::make_shared(uuid, num_handles, inst_id, advertise); + this->services_.emplace(uuid_str, service); service->do_create(this); + return service; } void BLEServer::remove_service(ESPBTUUID uuid) { - ESP_LOGV(TAG, "Removing BLE service - %s", uuid.to_string().c_str()); - BLEService *service = this->get_service(uuid); + std::string uuid_str = uuid.to_string(); + std::shared_ptr service = this->get_service(uuid); if (service == nullptr) { - ESP_LOGW(TAG, "BLE service %s not found", uuid.to_string().c_str()); + ESP_LOGW(TAG, "BLE service %s not found", uuid_str.c_str()); return; } + ESP_LOGV(TAG, "Removing BLE service - %s", uuid_str.c_str()); service->do_delete(); - delete service; // NOLINT(cppcoreguidelines-owning-memory) - this->services_.erase(uuid.to_string()); + this->services_.erase(uuid_str); } -BLEService *BLEServer::get_service(ESPBTUUID uuid) { - BLEService *service = nullptr; - if (this->services_.count(uuid.to_string()) > 0) { - service = this->services_.at(uuid.to_string()); +std::shared_ptr BLEServer::get_service(ESPBTUUID uuid) { + std::string uuid_str = uuid.to_string(); + std::shared_ptr service = nullptr; + if (this->services_.count(uuid_str) > 0) { + service = this->services_.at(uuid_str); } return service; } diff --git a/esphome/components/esp32_ble_server/ble_server.h b/esphome/components/esp32_ble_server/ble_server.h index e379e67296..0864364c0c 100644 --- a/esphome/components/esp32_ble_server/ble_server.h +++ b/esphome/components/esp32_ble_server/ble_server.h @@ -1,7 +1,7 @@ #pragma once -#include "ble_service.h" #include "ble_characteristic.h" +#include "ble_service.h" #include "esphome/components/esp32_ble/ble.h" #include "esphome/components/esp32_ble/ble_advertising.h" @@ -12,8 +12,8 @@ #include "esphome/core/preferences.h" #include -#include #include +#include #ifdef USE_ESP32 @@ -51,9 +51,10 @@ class BLEServer : public Component, public GATTsEventHandler, public BLEStatusEv this->restart_advertising_(); } - void create_service(ESPBTUUID uuid, bool advertise = false, uint16_t num_handles = 15, uint8_t inst_id = 0); + std::shared_ptr create_service(ESPBTUUID uuid, bool advertise = false, uint16_t num_handles = 15, + uint8_t inst_id = 0); void remove_service(ESPBTUUID uuid); - BLEService *get_service(ESPBTUUID uuid); + std::shared_ptr get_service(ESPBTUUID uuid); esp_gatt_if_t get_gatts_if() { return this->gatts_if_; } uint32_t get_connected_client_count() { return this->connected_clients_; } @@ -81,8 +82,8 @@ class BLEServer : public Component, public GATTsEventHandler, public BLEStatusEv uint32_t connected_clients_{0}; std::unordered_map clients_; - std::unordered_map services_; - BLEService *device_information_service_; + std::unordered_map> services_; + std::shared_ptr device_information_service_; std::vector service_components_; diff --git a/esphome/components/esp32_ble_server/ble_service.cpp b/esphome/components/esp32_ble_server/ble_service.cpp index 368f03fb52..4421cfba94 100644 --- a/esphome/components/esp32_ble_server/ble_service.cpp +++ b/esphome/components/esp32_ble_server/ble_service.cpp @@ -12,31 +12,33 @@ static const char *const TAG = "esp32_ble_server.service"; BLEService::BLEService(ESPBTUUID uuid, uint16_t num_handles, uint8_t inst_id, bool advertise) : uuid_(uuid), num_handles_(num_handles), inst_id_(inst_id), advertise_(advertise) {} -BLEService::~BLEService() { - for (auto &chr : this->characteristics_) - delete chr; // NOLINT(cppcoreguidelines-owning-memory) -} - -BLECharacteristic *BLEService::get_characteristic(ESPBTUUID uuid) { - for (auto *chr : this->characteristics_) { +std::shared_ptr BLEService::get_characteristic(ESPBTUUID uuid) { + for (auto chr : this->characteristics_) { if (chr->get_uuid() == uuid) return chr; } return nullptr; } -BLECharacteristic *BLEService::get_characteristic(uint16_t uuid) { +std::shared_ptr BLEService::get_characteristic(uint16_t uuid) { return this->get_characteristic(ESPBTUUID::from_uint16(uuid)); } -BLECharacteristic *BLEService::create_characteristic(uint16_t uuid, esp_gatt_char_prop_t properties) { +std::shared_ptr BLEService::create_characteristic(uint16_t uuid, esp_gatt_char_prop_t properties) { return create_characteristic(ESPBTUUID::from_uint16(uuid), properties); } -BLECharacteristic *BLEService::create_characteristic(const std::string &uuid, esp_gatt_char_prop_t properties) { +std::shared_ptr BLEService::create_characteristic(const std::string &uuid, + esp_gatt_char_prop_t properties) { return create_characteristic(ESPBTUUID::from_raw(uuid), properties); } -BLECharacteristic *BLEService::create_characteristic(ESPBTUUID uuid, esp_gatt_char_prop_t properties) { - // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) - BLECharacteristic *characteristic = new BLECharacteristic(uuid, properties); +std::shared_ptr BLEService::create_characteristic(const char *uuid, size_t len, + esp_gatt_char_prop_t properties) { + return create_characteristic(ESPBTUUID::from_raw(uuid, len), properties); +} +std::shared_ptr BLEService::create_characteristic(ESPBTUUID uuid, esp_gatt_char_prop_t properties) { + auto characteristic = std::make_shared(uuid, properties); + if (characteristic == nullptr) { + return nullptr; + } this->characteristics_.push_back(characteristic); return characteristic; } @@ -80,9 +82,9 @@ bool BLEService::do_create_characteristics_() { if (this->last_created_characteristic_ != nullptr && !this->last_created_characteristic_->is_created()) return true; // Signifies that the previous characteristic is still being created. - auto *characteristic = this->characteristics_[this->created_characteristic_count_++]; + auto characteristic = this->characteristics_[this->created_characteristic_count_++]; this->last_created_characteristic_ = characteristic; - characteristic->do_create(this); + characteristic->do_create(shared_from_this()); return true; } @@ -124,7 +126,7 @@ bool BLEService::is_failed() { if (this->init_state_ == FAILED) return true; bool failed = false; - for (auto *characteristic : this->characteristics_) + for (auto characteristic : this->characteristics_) failed |= characteristic->is_failed(); if (failed) @@ -166,7 +168,7 @@ void BLEService::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t g break; } - for (auto *characteristic : this->characteristics_) { + for (auto characteristic : this->characteristics_) { characteristic->gatts_event_handler(event, gatts_if, param); } } diff --git a/esphome/components/esp32_ble_server/ble_service.h b/esphome/components/esp32_ble_server/ble_service.h index 5e5883b6bf..a598d0d7bf 100644 --- a/esphome/components/esp32_ble_server/ble_service.h +++ b/esphome/components/esp32_ble_server/ble_service.h @@ -3,6 +3,7 @@ #include "ble_characteristic.h" #include "esphome/components/esp32_ble/ble_uuid.h" +#include #include #ifdef USE_ESP32 @@ -20,19 +21,21 @@ class BLEServer; using namespace esp32_ble; -class BLEService { +class BLEService : public std::enable_shared_from_this { public: BLEService(ESPBTUUID uuid, uint16_t num_handles, uint8_t inst_id, bool advertise); - ~BLEService(); - BLECharacteristic *get_characteristic(ESPBTUUID uuid); - BLECharacteristic *get_characteristic(uint16_t uuid); - BLECharacteristic *create_characteristic(const std::string &uuid, esp_gatt_char_prop_t properties); - BLECharacteristic *create_characteristic(uint16_t uuid, esp_gatt_char_prop_t properties); - BLECharacteristic *create_characteristic(ESPBTUUID uuid, esp_gatt_char_prop_t properties); + std::shared_ptr get_characteristic(ESPBTUUID uuid); + std::shared_ptr get_characteristic(uint16_t uuid); + + std::shared_ptr create_characteristic(const char *uuid, size_t len, + esp_gatt_char_prop_t properties); + std::shared_ptr create_characteristic(const std::string &uuid, esp_gatt_char_prop_t properties); + std::shared_ptr create_characteristic(uint16_t uuid, esp_gatt_char_prop_t properties); + std::shared_ptr create_characteristic(ESPBTUUID uuid, esp_gatt_char_prop_t properties); ESPBTUUID get_uuid() { return this->uuid_; } - BLECharacteristic *get_last_created_characteristic() { return this->last_created_characteristic_; } + std::shared_ptr get_last_created_characteristic() { return this->last_created_characteristic_; } uint16_t get_handle() { return this->handle_; } BLEServer *get_server() { return this->server_; } @@ -52,8 +55,8 @@ class BLEService { bool is_deleted() { return this->init_state_ == DELETED; } protected: - std::vector characteristics_; - BLECharacteristic *last_created_characteristic_{nullptr}; + std::vector> characteristics_; + std::shared_ptr last_created_characteristic_{nullptr}; uint32_t created_characteristic_count_{0}; BLEServer *server_; ESPBTUUID uuid_; diff --git a/esphome/components/esp32_improv/esp32_improv_component.cpp b/esphome/components/esp32_improv/esp32_improv_component.cpp index d90eaac3b6..40fb1d89d0 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.cpp +++ b/esphome/components/esp32_improv/esp32_improv_component.cpp @@ -1,12 +1,13 @@ #include "esp32_improv_component.h" #include "esphome/components/esp32_ble/ble.h" -#include "esphome/components/esp32_ble_server/ble_2902.h" #include "esphome/core/application.h" #include "esphome/core/log.h" #ifdef USE_ESP32 +static constexpr size_t MAX_UUID_LENGTH = 37; + namespace esphome { namespace esp32_improv { @@ -28,35 +29,72 @@ void ESP32ImprovComponent::setup() { #endif } -void ESP32ImprovComponent::setup_characteristics() { - this->status_ = this->service_->create_characteristic( - improv::STATUS_UUID, BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY); - BLEDescriptor *status_descriptor = new BLE2902(); - this->status_->add_descriptor(status_descriptor); +bool ESP32ImprovComponent::setup_characteristics() { + this->status_ = + this->service_->create_characteristic(improv::STATUS_UUID, strnlen(improv::STATUS_UUID, MAX_UUID_LENGTH), + BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY); + if (this->status_ == nullptr) { + ESP_LOGE(TAG, "Failed to create status characteristic"); + return false; + } - this->error_ = this->service_->create_characteristic( - improv::ERROR_UUID, BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY); - BLEDescriptor *error_descriptor = new BLE2902(); - this->error_->add_descriptor(error_descriptor); + if (!this->status_->make_2902_descriptor()) { + ESP_LOGE(TAG, "Failed to create status descriptor"); + return false; + } - this->rpc_ = this->service_->create_characteristic(improv::RPC_COMMAND_UUID, BLECharacteristic::PROPERTY_WRITE); + this->error_ = + this->service_->create_characteristic(improv::ERROR_UUID, strnlen(improv::ERROR_UUID, MAX_UUID_LENGTH), + BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY); + if (this->error_ == nullptr) { + ESP_LOGE(TAG, "Failed to create error characteristic"); + return false; + } + + if (!this->error_->make_2902_descriptor()) { + ESP_LOGE(TAG, "Failed to create error descriptor"); + return false; + } + + this->rpc_ = this->service_->create_characteristic( + improv::RPC_COMMAND_UUID, strnlen(improv::RPC_COMMAND_UUID, MAX_UUID_LENGTH), BLECharacteristic::PROPERTY_WRITE); + if (this->rpc_ == nullptr) { + ESP_LOGE(TAG, "Failed to create rpc characteristic"); + return false; + } this->rpc_->on_write([this](const std::vector &data) { if (!data.empty()) { this->incoming_data_.insert(this->incoming_data_.end(), data.begin(), data.end()); } }); - BLEDescriptor *rpc_descriptor = new BLE2902(); - this->rpc_->add_descriptor(rpc_descriptor); + if (this->rpc_->make_2902_descriptor() == nullptr) { + ESP_LOGE(TAG, "Failed to create rpc descriptor"); + return false; + } - this->rpc_response_ = this->service_->create_characteristic( - improv::RPC_RESULT_UUID, BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY); - BLEDescriptor *rpc_response_descriptor = new BLE2902(); - this->rpc_response_->add_descriptor(rpc_response_descriptor); + this->rpc_response_ = + this->service_->create_characteristic(improv::RPC_RESULT_UUID, strnlen(improv::RPC_RESULT_UUID, MAX_UUID_LENGTH), + BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY); + if (this->rpc_response_ == nullptr) { + ESP_LOGE(TAG, "Failed to create rpc response characteristic"); + return false; + } + if (!this->rpc_response_->make_2902_descriptor()) { + ESP_LOGE(TAG, "Failed to create rpc response descriptor"); + return false; + } + + this->capabilities_ = this->service_->create_characteristic( + improv::CAPABILITIES_UUID, strnlen(improv::CAPABILITIES_UUID, MAX_UUID_LENGTH), BLECharacteristic::PROPERTY_READ); + if (this->capabilities_ == nullptr) { + ESP_LOGE(TAG, "Failed to create capabilities characteristic"); + return false; + } + if (!this->capabilities_->make_2902_descriptor()) { + ESP_LOGE(TAG, "Failed to create capabilities descriptor"); + return false; + } - this->capabilities_ = - this->service_->create_characteristic(improv::CAPABILITIES_UUID, BLECharacteristic::PROPERTY_READ); - BLEDescriptor *capabilities_descriptor = new BLE2902(); - this->capabilities_->add_descriptor(capabilities_descriptor); uint8_t capabilities = 0x00; #ifdef USE_OUTPUT if (this->status_indicator_ != nullptr) @@ -64,6 +102,7 @@ void ESP32ImprovComponent::setup_characteristics() { #endif this->capabilities_->set_value(capabilities); this->setup_complete_ = true; + return true; } void ESP32ImprovComponent::loop() { @@ -75,9 +114,11 @@ void ESP32ImprovComponent::loop() { if (this->service_ == nullptr) { // Setup the service ESP_LOGD(TAG, "Creating Improv service"); - global_ble_server->create_service(ESPBTUUID::from_raw(improv::SERVICE_UUID), true); - this->service_ = global_ble_server->get_service(ESPBTUUID::from_raw(improv::SERVICE_UUID)); - this->setup_characteristics(); + this->service_ = global_ble_server->create_service(ESPBTUUID::from_raw(improv::SERVICE_UUID), true); + if (!this->setup_characteristics()) { + this->mark_failed(); + return; + } } if (!this->incoming_data_.empty()) diff --git a/esphome/components/esp32_improv/esp32_improv_component.h b/esphome/components/esp32_improv/esp32_improv_component.h index 3ed377a6ad..efdd5b9a0e 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.h +++ b/esphome/components/esp32_improv/esp32_improv_component.h @@ -34,7 +34,7 @@ class ESP32ImprovComponent : public Component, public BLEServiceComponent { void dump_config() override; void loop() override; void setup() override; - void setup_characteristics(); + bool setup_characteristics(); void on_client_disconnect() override; float get_setup_priority() const override; @@ -68,12 +68,12 @@ class ESP32ImprovComponent : public Component, public BLEServiceComponent { std::vector incoming_data_; wifi::WiFiAP connecting_sta_; - BLEService *service_ = nullptr; - BLECharacteristic *status_; - BLECharacteristic *error_; - BLECharacteristic *rpc_; - BLECharacteristic *rpc_response_; - BLECharacteristic *capabilities_; + std::shared_ptr service_{nullptr}; + std::shared_ptr status_; + std::shared_ptr error_; + std::shared_ptr rpc_; + std::shared_ptr rpc_response_; + std::shared_ptr capabilities_; #ifdef USE_BINARY_SENSOR binary_sensor::BinarySensor *authorizer_{nullptr};