[esp32_ble] Attempt to manage memory better with shared_ptr

esphome/components/esp32_ble/ble_uuid.cpp

@@ -2,9 +2,9 @@
 
 #ifdef USE_ESP32
 
-#include <cstring>
-#include <cstdio>
 #include <cinttypes>
+#include <cstdio>
+#include <cstring>
 #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();) {
+    for (int i = 0; i < len;) {
-      uint8_t msb = data.c_str()[i];
+      uint8_t msb = data[i];
-      uint8_t lsb = data.c_str()[i + 1];
+      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();) {
+    for (int i = 0; i < len;) {
-      uint8_t msb = data.c_str()[i];
+      uint8_t msb = data[i];
-      uint8_t lsb = data.c_str()[i + 1];
+      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
+  } 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);
+    memcpy(ret.uuid_.uuid.uuid128, (uint8_t *) data, 16);
-  } else if (data.length() == 36) {
+  } 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();) {
+    for (int i = 0; i < len;) {
-      if (data.c_str()[i] == '-')
+      if (data[i] == '-')
         i++;
-      uint8_t msb = data.c_str()[i];
+      uint8_t msb = data[i];
-      uint8_t lsb = data.c_str()[i + 1];
+      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->string_ = str_snprintf("0x%02" PRIX32 "%02" PRIX32 "%02" PRIX32 "%02" PRIX32, 10,
-                          (this->uuid_.uuid.uuid32 >> 16 & 0xff), (this->uuid_.uuid.uuid32 >> 8 & 0xff),
+                                   (this->uuid_.uuid.uuid32 >> 24), (this->uuid_.uuid.uuid32 >> 16 & 0xff),
-                          this->uuid_.uuid.uuid32 & 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 "";

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 <string>
 #include <esp_bt_defs.h>
+#include <string>
 
 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

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;
 }

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 <cstring>
@@ -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);

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_) {
+  this->descriptors_.clear();
-    delete descriptor;  // NOLINT(cppcoreguidelines-owning-memory)
-  }
   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<BLEDescriptor> descriptor) {
+  this->descriptors_.push_back(descriptor);
+}
 
-void BLECharacteristic::remove_descriptor(BLEDescriptor *descriptor) {
+void BLECharacteristic::remove_descriptor(std::shared_ptr<BLEDescriptor> 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<BLE2901> BLECharacteristic::make_2901_descriptor(const std::string &value) {
+  auto descriptor = std::make_shared<BLE2901>(value);
+  if (descriptor == nullptr) {
+    ESP_LOGE(TAG, "Failed to allocate BLE2901 descriptor");
+    return nullptr;
+  }
+  this->add_descriptor(descriptor);
+  return descriptor;
+}
+
+std::shared_ptr<BLE2902> BLECharacteristic::make_2902_descriptor() {
+  auto descriptor = std::make_shared<BLE2902>();
+  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<BLEService> 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_) {
+        for (auto descriptor : this->descriptors_) {
-          descriptor->do_create(this);
+          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);
   }
 }

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 <memory>
 #include <vector>
 
 #ifdef USE_ESP32
 
+#include <esp_bt_defs.h>
 #include <esp_gap_ble_api.h>
 #include <esp_gatt_defs.h>
 #include <esp_gattc_api.h>
 #include <esp_gatts_api.h>
-#include <esp_bt_defs.h>
 #include <freertos/FreeRTOS.h>
 #include <freertos/semphr.h>
 
@@ -22,7 +26,7 @@ using namespace esp32_ble;
 
 class BLEService;
 
-class BLECharacteristic {
+class BLECharacteristic : public std::enable_shared_from_this<BLECharacteristic> {
  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<BLEService> 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<void(const std::vector<uint8_t> &)> &&func) { this->on_write_ = func; }
 
-  void add_descriptor(BLEDescriptor *descriptor);
+  void add_descriptor(std::shared_ptr<BLEDescriptor> descriptor);
-  void remove_descriptor(BLEDescriptor *descriptor);
+  void remove_descriptor(std::shared_ptr<BLEDescriptor> descriptor);
 
-  BLEService *get_service() { return this->service_; }
+  std::shared_ptr<BLE2901> make_2901_descriptor(const std::string &value);
+  std::shared_ptr<BLE2902> make_2902_descriptor();
+
+  std::shared_ptr<BLEService> get_service() { return this->service_; }
   ESPBTUUID get_uuid() { return this->uuid_; }
   std::vector<uint8_t> &get_value() { return this->value_; }
 
@@ -71,7 +78,7 @@ class BLECharacteristic {
 
  protected:
   bool write_event_{false};
-  BLEService *service_;
+  std::shared_ptr<BLEService> service_;
   ESPBTUUID uuid_;
   esp_gatt_char_prop_t properties_;
   uint16_t handle_{0xFFFF};
@@ -80,7 +87,7 @@ class BLECharacteristic {
   std::vector<uint8_t> value_;
   SemaphoreHandle_t set_value_lock_;
 
-  std::vector<BLEDescriptor *> descriptors_;
+  std::vector<std::shared_ptr<BLEDescriptor>> descriptors_;
 
   std::function<void(const std::vector<uint8_t> &)> on_write_;
 

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 <cstring>
 
 #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<uint8_t> allocator(ExternalRAMAllocator<uint8_t>::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<uint8_t> deallocator(ExternalRAMAllocator<uint8_t>::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<BLECharacteristic> characteristic) {
   this->characteristic_ = characteristic;
   esp_attr_control_t control;
   control.auto_rsp = ESP_GATT_AUTO_RSP;

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<BLECharacteristic> 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<BLECharacteristic> characteristic_{nullptr};
   ESPBTUUID uuid_;
   uint16_t handle_{0xFFFF};
 

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 <nvs_flash.h>
-#include <freertos/FreeRTOSConfig.h>
-#include <esp_bt_main.h>
 #include <esp_bt.h>
-#include <freertos/task.h>
+#include <esp_bt_main.h>
 #include <esp_gap_ble_api.h>
+#include <freertos/FreeRTOSConfig.h>
+#include <freertos/task.h>
+#include <nvs_flash.h>
 
 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_() {
-  if (this->model_.has_value()) {
+  std::shared_ptr<BLECharacteristic> model =
-    BLECharacteristic *model =
       this->device_information_service_->create_characteristic(MODEL_UUID, BLECharacteristic::PROPERTY_READ);
+  if (this->model_.has_value()) {
     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<BLECharacteristic> version =
       this->device_information_service_->create_characteristic(VERSION_UUID, BLECharacteristic::PROPERTY_READ);
   version->set_value("ESPHome " ESPHOME_VERSION);
 
-  BLECharacteristic *manufacturer =
+  std::shared_ptr<BLECharacteristic> 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) {
+std::shared_ptr<BLEService> BLEServer::create_service(ESPBTUUID uuid, bool advertise, uint16_t num_handles,
-  ESP_LOGV(TAG, "Creating BLE service - %s", uuid.to_string().c_str());
+                                                      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<BLEService> service = this->get_service(uuid);
   if (service != nullptr) {
-    ESP_LOGW(TAG, "BLE service %s already exists", uuid.to_string().c_str());
+    ESP_LOGW(TAG, "BLE service %s already exists", uuid_str.c_str());
-    return;
+    return service;
   }
-  service = new BLEService(uuid, num_handles, inst_id, advertise);  // NOLINT(cppcoreguidelines-owning-memory)
+  ESP_LOGV(TAG, "Creating BLE service - %s", uuid_str.c_str());
-  this->services_.emplace(uuid.to_string(), service);
+  service = std::make_shared<BLEService>(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());
+  std::string uuid_str = uuid.to_string();
-  BLEService *service = this->get_service(uuid);
+  std::shared_ptr<BLEService> 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_str);
-  this->services_.erase(uuid.to_string());
 }
 
-BLEService *BLEServer::get_service(ESPBTUUID uuid) {
+std::shared_ptr<BLEService> BLEServer::get_service(ESPBTUUID uuid) {
-  BLEService *service = nullptr;
+  std::string uuid_str = uuid.to_string();
-  if (this->services_.count(uuid.to_string()) > 0) {
+  std::shared_ptr<BLEService> service = nullptr;
-    service = this->services_.at(uuid.to_string());
+  if (this->services_.count(uuid_str) > 0) {
+    service = this->services_.at(uuid_str);
   }
   return service;
 }

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 <memory>
-#include <vector>
 #include <unordered_map>
+#include <vector>
 
 #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<BLEService> 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<BLEService> 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<uint16_t, void *> clients_;
-  std::unordered_map<std::string, BLEService *> services_;
+  std::unordered_map<std::string, std::shared_ptr<BLEService>> services_;
-  BLEService *device_information_service_;
+  std::shared_ptr<BLEService> device_information_service_;
 
   std::vector<BLEServiceComponent *> service_components_;
 

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() {
+std::shared_ptr<BLECharacteristic> BLEService::get_characteristic(ESPBTUUID uuid) {
-  for (auto &chr : this->characteristics_)
+  for (auto chr : this->characteristics_) {
-    delete chr;  // NOLINT(cppcoreguidelines-owning-memory)
-}
-
-BLECharacteristic *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<BLECharacteristic> 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<BLECharacteristic> 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<BLECharacteristic> 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) {
+std::shared_ptr<BLECharacteristic> BLEService::create_characteristic(const char *uuid, size_t len,
-  // NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
+                                                                     esp_gatt_char_prop_t properties) {
-  BLECharacteristic *characteristic = new BLECharacteristic(uuid, properties);
+  return create_characteristic(ESPBTUUID::from_raw(uuid, len), properties);
+}
+std::shared_ptr<BLECharacteristic> BLEService::create_characteristic(ESPBTUUID uuid, esp_gatt_char_prop_t properties) {
+  auto characteristic = std::make_shared<BLECharacteristic>(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);
   }
 }

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 <memory>
 #include <vector>
 
 #ifdef USE_ESP32
@@ -20,19 +21,21 @@ class BLEServer;
 
 using namespace esp32_ble;
 
-class BLEService {
+class BLEService : public std::enable_shared_from_this<BLEService> {
  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);
+  std::shared_ptr<BLECharacteristic> get_characteristic(ESPBTUUID uuid);
-  BLECharacteristic *create_characteristic(uint16_t uuid, esp_gatt_char_prop_t properties);
+  std::shared_ptr<BLECharacteristic> get_characteristic(uint16_t uuid);
-  BLECharacteristic *create_characteristic(ESPBTUUID uuid, esp_gatt_char_prop_t properties);
+
+  std::shared_ptr<BLECharacteristic> create_characteristic(const char *uuid, size_t len,
+                                                           esp_gatt_char_prop_t properties);
+  std::shared_ptr<BLECharacteristic> create_characteristic(const std::string &uuid, esp_gatt_char_prop_t properties);
+  std::shared_ptr<BLECharacteristic> create_characteristic(uint16_t uuid, esp_gatt_char_prop_t properties);
+  std::shared_ptr<BLECharacteristic> 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<BLECharacteristic> 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<BLECharacteristic *> characteristics_;
+  std::vector<std::shared_ptr<BLECharacteristic>> characteristics_;
-  BLECharacteristic *last_created_characteristic_{nullptr};
+  std::shared_ptr<BLECharacteristic> last_created_characteristic_{nullptr};
   uint32_t created_characteristic_count_{0};
   BLEServer *server_;
   ESPBTUUID uuid_;

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() {
+bool ESP32ImprovComponent::setup_characteristics() {
-  this->status_ = this->service_->create_characteristic(
+  this->status_ =
-      improv::STATUS_UUID, BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY);
+      this->service_->create_characteristic(improv::STATUS_UUID, strnlen(improv::STATUS_UUID, MAX_UUID_LENGTH),
-  BLEDescriptor *status_descriptor = new BLE2902();
+                                            BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY);
-  this->status_->add_descriptor(status_descriptor);
+  if (this->status_ == nullptr) {
+    ESP_LOGE(TAG, "Failed to create status characteristic");
+    return false;
+  }
 
-  this->error_ = this->service_->create_characteristic(
+  if (!this->status_->make_2902_descriptor()) {
-      improv::ERROR_UUID, BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY);
+    ESP_LOGE(TAG, "Failed to create status descriptor");
-  BLEDescriptor *error_descriptor = new BLE2902();
+    return false;
-  this->error_->add_descriptor(error_descriptor);
+  }
 
-  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<uint8_t> &data) {
     if (!data.empty()) {
       this->incoming_data_.insert(this->incoming_data_.end(), data.begin(), data.end());
     }
   });
-  BLEDescriptor *rpc_descriptor = new BLE2902();
+  if (this->rpc_->make_2902_descriptor() == nullptr) {
-  this->rpc_->add_descriptor(rpc_descriptor);
+    ESP_LOGE(TAG, "Failed to create rpc descriptor");
+    return false;
+  }
 
-  this->rpc_response_ = this->service_->create_characteristic(
+  this->rpc_response_ =
-      improv::RPC_RESULT_UUID, BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY);
+      this->service_->create_characteristic(improv::RPC_RESULT_UUID, strnlen(improv::RPC_RESULT_UUID, MAX_UUID_LENGTH),
-  BLEDescriptor *rpc_response_descriptor = new BLE2902();
+                                            BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY);
-  this->rpc_response_->add_descriptor(rpc_response_descriptor);
+  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->create_service(ESPBTUUID::from_raw(improv::SERVICE_UUID), true);
-    this->service_ = global_ble_server->get_service(ESPBTUUID::from_raw(improv::SERVICE_UUID));
+    if (!this->setup_characteristics()) {
-    this->setup_characteristics();
+      this->mark_failed();
+      return;
+    }
   }
 
   if (!this->incoming_data_.empty())

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<uint8_t> incoming_data_;
   wifi::WiFiAP connecting_sta_;
 
-  BLEService *service_ = nullptr;
+  std::shared_ptr<BLEService> service_{nullptr};
-  BLECharacteristic *status_;
+  std::shared_ptr<BLECharacteristic> status_;
-  BLECharacteristic *error_;
+  std::shared_ptr<BLECharacteristic> error_;
-  BLECharacteristic *rpc_;
+  std::shared_ptr<BLECharacteristic> rpc_;
-  BLECharacteristic *rpc_response_;
+  std::shared_ptr<BLECharacteristic> rpc_response_;
-  BLECharacteristic *capabilities_;
+  std::shared_ptr<BLECharacteristic> capabilities_;
 
 #ifdef USE_BINARY_SENSOR
   binary_sensor::BinarySensor *authorizer_{nullptr};