From c17e38e58f70fc116ed6e2bd313ff8eada07cb54 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 27 Sep 2025 13:39:03 -0500 Subject: [PATCH 1/3] order matters --- esphome/components/esp32_ble/ble.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 2108ded472..64cef70de2 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -81,8 +81,17 @@ void ESP32BLE::advertising_set_service_data_and_name(std::span da // when changing both properties, avoiding the brief gap that would occur with separate calls. this->advertising_init_(); - this->advertising_->set_service_data(data); - this->advertising_->set_include_name(include_name); + + if (include_name) { + // When including name, clear service data first to avoid packet overflow + this->advertising_->set_service_data(std::span{}); + this->advertising_->set_include_name(true); + } else { + // When including service data, clear name first to avoid packet overflow + this->advertising_->set_include_name(false); + this->advertising_->set_service_data(data); + } + this->advertising_start(); } From 7e4cfe369d701ce3321b95735a9ed6d1ed27d8c1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 27 Sep 2025 13:45:12 -0500 Subject: [PATCH 2/3] make bot happy --- .../esp32_improv/esp32_improv_component.cpp | 39 +++++++++++-------- .../esp32_improv/esp32_improv_component.h | 1 + 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/esphome/components/esp32_improv/esp32_improv_component.cpp b/esphome/components/esp32_improv/esp32_improv_component.cpp index 1c3ea538f3..6f193c0c51 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.cpp +++ b/esphome/components/esp32_improv/esp32_improv_component.cpp @@ -20,6 +20,11 @@ static constexpr uint16_t STOP_ADVERTISING_DELAY = static constexpr uint16_t NAME_ADVERTISING_INTERVAL = 60000; // Advertise name every 60 seconds static constexpr uint16_t NAME_ADVERTISING_DURATION = 1000; // Advertise name for 1 second +// Improv service data constants +static constexpr uint8_t IMPROV_SERVICE_DATA_SIZE = 8; +static constexpr uint8_t IMPROV_PROTOCOL_ID_1 = 0x77; // 'P' << 1 | 'R' >> 7 +static constexpr uint8_t IMPROV_PROTOCOL_ID_2 = 0x46; // 'I' << 1 | 'M' >> 7 + ESP32ImprovComponent::ESP32ImprovComponent() { global_improv_component = this; } void ESP32ImprovComponent::setup() { @@ -122,14 +127,7 @@ void ESP32ImprovComponent::loop() { esp32_ble::global_ble->advertising_start(); // Set initial state based on whether we have an authorizer - // authorizer_ member only exists when USE_BINARY_SENSOR is defined -#ifdef USE_BINARY_SENSOR - this->set_state_( - this->authorizer_ == nullptr ? improv::STATE_AUTHORIZED : improv::STATE_AWAITING_AUTHORIZATION, false); -#else - // No binary_sensor support = no authorizer possible, start as authorized - this->set_state_(improv::STATE_AUTHORIZED, false); -#endif + this->set_state_(this->get_initial_state_(), false); this->set_error_(improv::ERROR_NONE); ESP_LOGD(TAG, "Service started!"); } @@ -140,14 +138,13 @@ void ESP32ImprovComponent::loop() { if (this->authorizer_ == nullptr || (this->authorized_start_ != 0 && ((now - this->authorized_start_) < this->authorized_duration_))) { this->set_state_(improv::STATE_AUTHORIZED); - } else -#else - { this->set_state_(improv::STATE_AUTHORIZED); } -#endif - { + } else { if (!this->check_identify_()) this->set_status_indicator_state_(true); } +#else + this->set_state_(improv::STATE_AUTHORIZED); +#endif break; } case improv::STATE_AUTHORIZED: { @@ -400,9 +397,9 @@ void ESP32ImprovComponent::on_wifi_connect_timeout_() { } void ESP32ImprovComponent::advertise_service_data_() { - uint8_t service_data[8] = {}; - service_data[0] = 0x77; // PR - service_data[1] = 0x46; // IM + uint8_t service_data[IMPROV_SERVICE_DATA_SIZE] = {}; + service_data[0] = IMPROV_PROTOCOL_ID_1; // PR + service_data[1] = IMPROV_PROTOCOL_ID_2; // IM service_data[2] = static_cast(this->state_); uint8_t capabilities = 0x00; @@ -443,6 +440,16 @@ void ESP32ImprovComponent::update_advertising_type_() { } } +improv::State ESP32ImprovComponent::get_initial_state_() const { +#ifdef USE_BINARY_SENSOR + // If we have an authorizer, start in awaiting authorization state + return this->authorizer_ == nullptr ? improv::STATE_AUTHORIZED : improv::STATE_AWAITING_AUTHORIZATION; +#else + // No binary_sensor support = no authorizer possible, start as authorized + return improv::STATE_AUTHORIZED; +#endif +} + ESP32ImprovComponent *global_improv_component = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) } // namespace esp32_improv diff --git a/esphome/components/esp32_improv/esp32_improv_component.h b/esphome/components/esp32_improv/esp32_improv_component.h index ea51f64d4b..eb07e09dce 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.h +++ b/esphome/components/esp32_improv/esp32_improv_component.h @@ -107,6 +107,7 @@ class ESP32ImprovComponent : public Component { void set_state_(improv::State state, bool update_advertising = true); void set_error_(improv::Error error); + improv::State get_initial_state_() const; void send_response_(std::vector &response); void process_incoming_data_(); void on_wifi_connect_timeout_(); From f387e7690c949557baf3e511a53c030525567336 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 27 Sep 2025 13:46:12 -0500 Subject: [PATCH 3/3] nesting --- .../components/esp32_improv/esp32_improv_component.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/esphome/components/esp32_improv/esp32_improv_component.cpp b/esphome/components/esp32_improv/esp32_improv_component.cpp index 6f193c0c51..ca08ff0cca 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.cpp +++ b/esphome/components/esp32_improv/esp32_improv_component.cpp @@ -149,12 +149,10 @@ void ESP32ImprovComponent::loop() { } case improv::STATE_AUTHORIZED: { #ifdef USE_BINARY_SENSOR - if (this->authorizer_ != nullptr) { - if (now - this->authorized_start_ > this->authorized_duration_) { - ESP_LOGD(TAG, "Authorization timeout"); - this->set_state_(improv::STATE_AWAITING_AUTHORIZATION); - return; - } + if (this->authorizer_ != nullptr && now - this->authorized_start_ > this->authorized_duration_) { + ESP_LOGD(TAG, "Authorization timeout"); + this->set_state_(improv::STATE_AWAITING_AUTHORIZATION); + return; } #endif if (!this->check_identify_()) {