Fixes for BLE/improv (#1878)

esphome/components/esp32_ble/ble.cpp

@@ -119,24 +119,24 @@ bool ESP32BLE::ble_setup_() {
   return true;
 }
 
-void ESP32BLE::loop() {
+// void ESP32BLE::loop() {
-  BLEEvent *ble_event = this->ble_events_.pop();
+//   BLEEvent *ble_event = this->ble_events_.pop();
-  while (ble_event != nullptr) {
+//   while (ble_event != nullptr) {
-    switch (ble_event->type_) {
+//     switch (ble_event->type_) {
-      case ble_event->GATTS:
+//       case ble_event->GATTS:
-        this->real_gatts_event_handler_(ble_event->event_.gatts.gatts_event, ble_event->event_.gatts.gatts_if,
+//         this->real_gatts_event_handler_(ble_event->event_.gatts.gatts_event, ble_event->event_.gatts.gatts_if,
-                                        &ble_event->event_.gatts.gatts_param);
+//                                         &ble_event->event_.gatts.gatts_param);
-        break;
+//         break;
-      case ble_event->GAP:
+//       case ble_event->GAP:
-        this->real_gap_event_handler_(ble_event->event_.gap.gap_event, &ble_event->event_.gap.gap_param);
+//         this->real_gap_event_handler_(ble_event->event_.gap.gap_event, &ble_event->event_.gap.gap_param);
-        break;
+//         break;
-      default:
+//       default:
-        break;
+//         break;
-    }
+//     }
-    delete ble_event;
+//     delete ble_event;
-    ble_event = this->ble_events_.pop();
+//     ble_event = this->ble_events_.pop();
-  }
+//   }
-}
+// }
 
 void ESP32BLE::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) {
   global_ble->real_gap_event_handler_(event, param);

esphome/components/esp32_ble/ble.h

@@ -23,7 +23,7 @@ typedef struct {
 class ESP32BLE : public Component {
  public:
   void setup() override;
-  void loop() override;
+  // void loop() override;
   void dump_config() override;
   float get_setup_priority() const override;
   void mark_failed() override;

esphome/components/esp32_ble/ble_2901.cpp

@@ -6,8 +6,8 @@
 namespace esphome {
 namespace esp32_ble {
 
-BLE2901::BLE2901(const std::string value) : BLE2901((uint8_t *) value.data(), value.length()) {}
+BLE2901::BLE2901(const std::string &value) : BLE2901((uint8_t *) value.data(), value.length()) {}
-BLE2901::BLE2901(uint8_t *data, size_t length) : BLEDescriptor(ESPBTUUID::from_uint16(0x2901)) {
+BLE2901::BLE2901(const uint8_t *data, size_t length) : BLEDescriptor(ESPBTUUID::from_uint16(0x2901)) {
   this->set_value(data, length);
   this->permissions_ = ESP_GATT_PERM_READ;
 }

esphome/components/esp32_ble/ble_2901.h

@@ -9,8 +9,8 @@ namespace esp32_ble {
 
 class BLE2901 : public BLEDescriptor {
  public:
-  BLE2901(const std::string value);
+  BLE2901(const std::string &value);
-  BLE2901(uint8_t *data, size_t length);
+  BLE2901(const uint8_t *data, size_t length);
 };
 
 }  // namespace esp32_ble

esphome/components/esp32_ble/ble_characteristic.cpp

@@ -32,10 +32,10 @@ void BLECharacteristic::set_value(std::vector<uint8_t> value) {
   this->value_ = value;
   xSemaphoreGive(this->set_value_lock_);
 }
-void BLECharacteristic::set_value(std::string value) {
+void BLECharacteristic::set_value(const std::string &value) {
   this->set_value(std::vector<uint8_t>(value.begin(), value.end()));
 }
-void BLECharacteristic::set_value(uint8_t *data, size_t length) {
+void BLECharacteristic::set_value(const uint8_t *data, size_t length) {
   this->set_value(std::vector<uint8_t>(data, data + length));
 }
 void BLECharacteristic::set_value(uint8_t &data) {
@@ -105,7 +105,7 @@ bool BLECharacteristic::do_create(BLEService *service) {
   esp_attr_control_t control;
   control.auto_rsp = ESP_GATT_RSP_BY_APP;
 
-  xSemaphoreTake(this->create_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreTake(this->create_lock_, portMAX_DELAY);
   ESP_LOGV(TAG, "Creating characteristic - %s", this->uuid_.to_string().c_str());
 
   esp_bt_uuid_t uuid = this->uuid_.get_uuid();
@@ -117,7 +117,7 @@ bool BLECharacteristic::do_create(BLEService *service) {
     return false;
   }
 
-  xSemaphoreWait(this->create_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreWait(this->create_lock_, portMAX_DELAY);
 
   for (auto *descriptor : this->descriptors_) {
     descriptor->do_create(this);

esphome/components/esp32_ble/ble_characteristic.h

@@ -22,9 +22,9 @@ class BLECharacteristic {
  public:
   BLECharacteristic(const ESPBTUUID uuid, uint32_t properties);
 
-  void set_value(uint8_t *data, size_t length);
+  void set_value(const uint8_t *data, size_t length);
   void set_value(std::vector<uint8_t> value);
-  void set_value(std::string value);
+  void set_value(const std::string &value);
   void set_value(uint8_t &data);
   void set_value(uint16_t &data);
   void set_value(uint32_t &data);
@@ -45,7 +45,7 @@ class BLECharacteristic {
   bool do_create(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(std::vector<uint8_t> &)> &func) { this->on_write_ = func; }
+  void on_write(const std::function<void(const std::vector<uint8_t> &)> &func) { this->on_write_ = func; }
 
   void add_descriptor(BLEDescriptor *descriptor);
 
@@ -74,7 +74,7 @@ class BLECharacteristic {
 
   std::vector<BLEDescriptor *> descriptors_;
 
-  std::function<void(std::vector<uint8_t> &)> on_write_;
+  std::function<void(const std::vector<uint8_t> &)> on_write_;
 
   esp_gatt_perm_t permissions_ = ESP_GATT_PERM_READ | ESP_GATT_PERM_WRITE;
 };

esphome/components/esp32_ble/ble_descriptor.cpp

@@ -28,7 +28,7 @@ bool BLEDescriptor::do_create(BLECharacteristic *characteristic) {
   esp_attr_control_t control;
   control.auto_rsp = ESP_GATT_AUTO_RSP;
 
-  xSemaphoreTake(this->create_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreTake(this->create_lock_, portMAX_DELAY);
   ESP_LOGV(TAG, "Creating descriptor - %s", this->uuid_.to_string().c_str());
   esp_bt_uuid_t uuid = this->uuid_.get_uuid();
   esp_err_t err = esp_ble_gatts_add_char_descr(this->characteristic_->get_service()->get_handle(), &uuid,
@@ -37,13 +37,13 @@ bool BLEDescriptor::do_create(BLECharacteristic *characteristic) {
     ESP_LOGE(TAG, "esp_ble_gatts_add_char_descr failed: %d", err);
     return false;
   }
-  xSemaphoreWait(this->create_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreWait(this->create_lock_, portMAX_DELAY);
 
   return true;
 }
 
-void BLEDescriptor::set_value(const std::string value) { this->set_value((uint8_t *) value.data(), value.length()); }
+void BLEDescriptor::set_value(const std::string &value) { this->set_value((uint8_t *) value.data(), value.length()); }
-void BLEDescriptor::set_value(uint8_t *data, size_t length) {
+void BLEDescriptor::set_value(const uint8_t *data, size_t length) {
   if (length > this->value_.attr_max_len) {
     ESP_LOGE(TAG, "Size %d too large, must be no bigger than %d", length, this->value_.attr_max_len);
     return;

esphome/components/esp32_ble/ble_descriptor.h

@@ -18,8 +18,8 @@ class BLEDescriptor {
   virtual ~BLEDescriptor();
   bool do_create(BLECharacteristic *characteristic);
 
-  void set_value(const std::string value);
+  void set_value(const std::string &value);
-  void set_value(uint8_t *data, size_t length);
+  void set_value(const uint8_t *data, size_t length);
 
   void gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param);
 

esphome/components/esp32_ble/ble_server.cpp

@@ -46,14 +46,14 @@ void BLEServer::setup() {
 }
 
 void BLEServer::setup_server_() {
-  xSemaphoreTake(this->register_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreTake(this->register_lock_, portMAX_DELAY);
   esp_err_t err = esp_ble_gatts_app_register(0);
   if (err != ESP_OK) {
     ESP_LOGE(TAG, "esp_ble_gatts_app_register failed: %d", err);
     this->mark_failed();
     return;
   }
-  xSemaphoreWait(this->register_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreWait(this->register_lock_, portMAX_DELAY);
 
   this->device_information_service = this->create_service(DEVICE_INFORMATION_SERVICE_UUID);
 
@@ -96,7 +96,7 @@ BLEService *BLEServer::create_service(const uint8_t *uuid, bool advertise) {
 BLEService *BLEServer::create_service(uint16_t uuid, bool advertise) {
   return this->create_service(ESPBTUUID::from_uint16(uuid), advertise);
 }
-BLEService *BLEServer::create_service(const std::string uuid, bool advertise) {
+BLEService *BLEServer::create_service(const std::string &uuid, bool advertise) {
   return this->create_service(ESPBTUUID::from_raw(uuid), advertise);
 }
 BLEService *BLEServer::create_service(ESPBTUUID uuid, bool advertise, uint16_t num_handles, uint8_t inst_id) {

esphome/components/esp32_ble/ble_server.h

@@ -34,17 +34,17 @@ class BLEServer : public Component {
 
   void teardown();
 
-  void set_manufacturer(const std::string manufacturer) { this->manufacturer_ = manufacturer; }
+  void set_manufacturer(const std::string &manufacturer) { this->manufacturer_ = manufacturer; }
-  void set_model(const std::string model) { this->model_ = model; }
+  void set_model(const std::string &model) { this->model_ = model; }
 
   BLEService *create_service(const uint8_t *uuid, bool advertise = false);
   BLEService *create_service(uint16_t uuid, bool advertise = false);
-  BLEService *create_service(const std::string uuid, bool advertise = false);
+  BLEService *create_service(const std::string &uuid, bool advertise = false);
   BLEService *create_service(ESPBTUUID uuid, bool advertise = false, uint16_t num_handles = 15, uint8_t inst_id = 0);
 
   esp_gatt_if_t get_gatts_if() { return this->gatts_if_; }
   uint32_t get_connected_client_count() { return this->connected_clients_; }
-  std::map<uint16_t, void *> get_clients() { return this->clients_; }
+  const std::map<uint16_t, void *> &get_clients() { return this->clients_; }
   BLEAdvertising *get_advertising() { return this->advertising_; }
 
   void gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param);

esphome/components/esp32_ble/ble_service.cpp

@@ -38,7 +38,7 @@ BLECharacteristic *BLEService::get_characteristic(uint16_t uuid) {
 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) {
+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) {
@@ -50,7 +50,7 @@ BLECharacteristic *BLEService::create_characteristic(ESPBTUUID uuid, esp_gatt_ch
 bool BLEService::do_create(BLEServer *server) {
   this->server_ = server;
 
-  xSemaphoreTake(this->create_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreTake(this->create_lock_, portMAX_DELAY);
   esp_gatt_srvc_id_t srvc_id;
   srvc_id.is_primary = true;
   srvc_id.id.inst_id = this->inst_id_;
@@ -61,7 +61,7 @@ bool BLEService::do_create(BLEServer *server) {
     ESP_LOGE(TAG, "esp_ble_gatts_create_service failed: %d", err);
     return false;
   }
-  xSemaphoreWait(this->create_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreWait(this->create_lock_, portMAX_DELAY);
 
   return true;
 }
@@ -72,23 +72,23 @@ void BLEService::start() {
     characteristic->do_create(this);
   }
 
-  xSemaphoreTake(this->start_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreTake(this->start_lock_, portMAX_DELAY);
   esp_err_t err = esp_ble_gatts_start_service(this->handle_);
   if (err != ESP_OK) {
     ESP_LOGE(TAG, "esp_ble_gatts_start_service failed: %d", err);
     return;
   }
-  xSemaphoreWait(this->start_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreWait(this->start_lock_, portMAX_DELAY);
 }
 
 void BLEService::stop() {
-  xSemaphoreTake(this->stop_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreTake(this->stop_lock_, portMAX_DELAY);
   esp_err_t err = esp_ble_gatts_stop_service(this->handle_);
   if (err != ESP_OK) {
     ESP_LOGE(TAG, "esp_ble_gatts_stop_service failed: %d", err);
     return;
   }
-  xSemaphoreWait(this->stop_lock_, SEMAPHORE_MAX_DELAY);
+  xSemaphoreWait(this->stop_lock_, portMAX_DELAY);
 }
 
 void BLEService::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if,

esphome/components/esp32_ble/ble_service.h

@@ -23,7 +23,7 @@ class 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(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);
 

esphome/components/esp32_ble/ble_uuid.cpp

@@ -25,7 +25,7 @@ ESPBTUUID ESPBTUUID::from_raw(const uint8_t *data) {
     ret.uuid_.uuid.uuid128[i] = data[i];
   return ret;
 }
-ESPBTUUID ESPBTUUID::from_raw(const std::string data) {
+ESPBTUUID ESPBTUUID::from_raw(const std::string &data) {
   ESPBTUUID ret;
   if (data.length() == 4) {
     ret.uuid_.len = ESP_UUID_LEN_16;

esphome/components/esp32_ble/ble_uuid.h

@@ -20,7 +20,7 @@ class ESPBTUUID {
 
   static ESPBTUUID from_raw(const uint8_t *data);
 
-  static ESPBTUUID from_raw(const std::string data);
+  static ESPBTUUID from_raw(const std::string &data);
 
   static ESPBTUUID from_uuid(esp_bt_uuid_t uuid);
 

esphome/components/esp32_improv/esp32_improv_component.cpp

@@ -27,7 +27,7 @@ void ESP32ImprovComponent::setup_service() {
 
   this->rpc_ =
       this->service_->create_characteristic(improv::RPC_COMMAND_UUID, esp32_ble::BLECharacteristic::PROPERTY_WRITE);
-  this->rpc_->on_write([this](std::vector<uint8_t> &data) {
+  this->rpc_->on_write([this](const std::vector<uint8_t> &data) {
     if (data.size() > 0) {
       this->incoming_data_.insert(this->incoming_data_.end(), data.begin(), data.end());
     }
@@ -123,13 +123,17 @@ void ESP32ImprovComponent::loop() {
         std::string url = "https://my.home-assistant.io/redirect/config_flow_start?domain=esphome";
         std::vector<uint8_t> data = improv::build_rpc_response(improv::WIFI_SETTINGS, {url});
         this->send_response(data);
+        this->set_timeout("end-service", 1000, [this] {
+          this->service_->stop();
+          this->set_state_(improv::STATE_STOPPED);
+        });
       }
       break;
     }
     case improv::STATE_PROVISIONED: {
       this->incoming_data_.clear();
       if (this->status_indicator_ != nullptr)
-        this->status_indicator_->turn_on();
+        this->status_indicator_->turn_off();
       break;
     }
   }
@@ -188,8 +192,10 @@ void ESP32ImprovComponent::start() {
 }
 
 void ESP32ImprovComponent::end() {
-  this->set_state_(improv::STATE_STOPPED);
+  this->set_timeout("end-service", 1000, [this] {
     this->service_->stop();
+    this->set_state_(improv::STATE_STOPPED);
+  });
 }
 
 float ESP32ImprovComponent::get_setup_priority() const {

esphome/components/esp32_improv/esp32_improv_component.h

@@ -26,7 +26,7 @@ class ESP32ImprovComponent : public Component, public esp32_ble::BLEServiceCompo
   float get_setup_priority() const override;
   void start();
   void end();
-  bool is_active() const { return this->state_ == improv::STATE_AUTHORIZED; }
+  bool is_active() const { return this->state_ != improv::STATE_STOPPED; }
 
   void set_authorizer(binary_sensor::BinarySensor *authorizer) { this->authorizer_ = authorizer; }
   void set_status_indicator(output::BinaryOutput *status_indicator) { this->status_indicator_ = status_indicator; }

esphome/components/improv/improv.cpp

@@ -2,7 +2,9 @@
 
 namespace improv {
 
-ImprovCommand parse_improv_data(std::vector<uint8_t> &data) { return parse_improv_data(data.data(), data.size()); }
+ImprovCommand parse_improv_data(const std::vector<uint8_t> &data) {
+  return parse_improv_data(data.data(), data.size());
+}
 
 ImprovCommand parse_improv_data(const uint8_t *data, size_t length) {
   Command command = (Command) data[0];
@@ -42,7 +44,28 @@ ImprovCommand parse_improv_data(const uint8_t *data, size_t length) {
   };
 }
 
-std::vector<uint8_t> build_rpc_response(Command command, std::vector<std::string> datum) {
+std::vector<uint8_t> build_rpc_response(Command command, const std::vector<std::string> &datum) {
+  std::vector<uint8_t> out;
+  uint32_t length = 0;
+  out.push_back(command);
+  for (auto str : datum) {
+    uint8_t len = str.length();
+    length += len;
+    out.push_back(len);
+    out.insert(out.end(), str.begin(), str.end());
+  }
+  out.insert(out.begin() + 1, length);
+
+  uint32_t calculated_checksum = 0;
+
+  for (uint8_t byte : out) {
+    calculated_checksum += byte;
+  }
+  out.push_back(calculated_checksum);
+  return out;
+}
+
+std::vector<uint8_t> build_rpc_response(Command command, const std::vector<String> &datum) {
   std::vector<uint8_t> out;
   uint32_t length = 0;
   out.push_back(command);

esphome/components/improv/improv.h

@@ -2,6 +2,7 @@
 
 #include <stdint.h>
 #include <string>
+#include "WString.h"
 #include <vector>
 
 namespace improv {
@@ -45,9 +46,10 @@ struct ImprovCommand {
   std::string password;
 };
 
-ImprovCommand parse_improv_data(std::vector<uint8_t> &data);
+ImprovCommand parse_improv_data(const std::vector<uint8_t> &data);
 ImprovCommand parse_improv_data(const uint8_t *data, size_t length);
 
-std::vector<uint8_t> build_rpc_response(Command command, std::vector<std::string> datum);
+std::vector<uint8_t> build_rpc_response(Command command, const std::vector<std::string> &datum);
+std::vector<uint8_t> build_rpc_response(Command command, const std::vector<String> &datum);
 
 }  // namespace improv

esphome/core/defines.h

@@ -19,6 +19,7 @@
 #define USE_JSON
 #ifdef ARDUINO_ARCH_ESP32
 #define USE_ESP32_CAMERA
+#define USE_IMPROV
 #endif
 #define USE_TIME
 #define USE_DEEP_SLEEP

esphome/core/helpers.h

@@ -27,8 +27,6 @@
 
 namespace esphome {
 
-static const uint32_t SEMAPHORE_MAX_DELAY = 4294967295UL;
-
 /// The characters that are allowed in a hostname.
 extern const char *HOSTNAME_CHARACTER_ALLOWLIST;