diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index 6e6fb0e30d..fc95760cf8 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -100,21 +100,38 @@ void ESP32BLE::disable() { #ifdef USE_ESP32_BLE_ADVERTISING void ESP32BLE::advertising_start() { this->advertising_init_(); - if (!this->is_active()) + this->advertising_ref_count_++; + this->advertising_refresh(); +} + +void ESP32BLE::advertising_stop() { + if (this->advertising_ref_count_ == 0) return; - this->advertising_->start(); + this->advertising_ref_count_--; + this->advertising_refresh(); +} + +void ESP32BLE::advertising_refresh() { + if (this->advertising_ == nullptr || !this->is_active()) + return; + // Advertise while any component still needs it, otherwise stop + if (this->advertising_ref_count_ == 0) { + this->advertising_->stop(); + } else { + this->advertising_->start(); + } } void ESP32BLE::advertising_set_service_data(const std::vector &data) { this->advertising_init_(); this->advertising_->set_service_data(data); - this->advertising_start(); + this->advertising_refresh(); } void ESP32BLE::advertising_set_manufacturer_data(const std::vector &data) { this->advertising_init_(); this->advertising_->set_manufacturer_data(data); - this->advertising_start(); + this->advertising_refresh(); } void ESP32BLE::advertising_set_service_data_and_name(std::span data, bool include_name) { @@ -136,7 +153,7 @@ void ESP32BLE::advertising_set_service_data_and_name(std::span da this->advertising_->set_service_data(data); } - this->advertising_start(); + this->advertising_refresh(); } void ESP32BLE::advertising_register_raw_advertisement_callback(std::function &&callback) { @@ -147,13 +164,13 @@ void ESP32BLE::advertising_register_raw_advertisement_callback(std::functionadvertising_init_(); this->advertising_->add_service_uuid(uuid); - this->advertising_start(); + this->advertising_refresh(); } void ESP32BLE::advertising_remove_service_uuid(ESPBTUUID uuid) { this->advertising_init_(); this->advertising_->remove_service_uuid(uuid); - this->advertising_start(); + this->advertising_refresh(); } #endif @@ -575,6 +592,10 @@ void ESP32BLE::loop_handle_state_transition_not_active_() { } this->state_ = BLE_COMPONENT_STATE_ACTIVE; +#ifdef USE_ESP32_BLE_ADVERTISING + // Requests made before the stack was up (or before it was re-enabled) take effect now + this->advertising_refresh(); +#endif } } diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index 2a355a6c8b..7d2d0438a4 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -114,7 +114,17 @@ class ESP32BLE final : public Component { void set_name(const char *name) { this->name_ = name; } #ifdef USE_ESP32_BLE_ADVERTISING + /** Request advertising on behalf of a component. + * + * Requests are reference counted: advertising runs until every component that called + * advertising_start() has released it again with advertising_stop(). Each component must + * pair its calls, so nothing advertises until something actually asks for it. + */ void advertising_start(); + /// Release a request made with advertising_start(); advertising stops at the last release. + void advertising_stop(); + /// Apply the current payload and request count: advertise while requested, otherwise stop. + void advertising_refresh(); void advertising_set_service_data(const std::vector &data); void advertising_set_manufacturer_data(const std::vector &data); void advertising_set_appearance(uint16_t appearance) { this->appearance_ = appearance; } @@ -226,6 +236,9 @@ class ESP32BLE final : public Component { // 1-byte aligned members (grouped together to minimize padding) BLEComponentState state_{BLE_COMPONENT_STATE_OFF}; // 1 byte (uint8_t enum) bool enable_on_boot_{}; // 1 byte +#ifdef USE_ESP32_BLE_ADVERTISING + uint8_t advertising_ref_count_{0}; // 1 byte, number of components requesting advertising +#endif #ifdef ESPHOME_ESP32_BLE_EXTENDED_AUTH_PARAMS optional auth_req_mode_; diff --git a/esphome/components/esp32_ble_beacon/esp32_ble_beacon.cpp b/esphome/components/esp32_ble_beacon/esp32_ble_beacon.cpp index 9f1723430b..ab728f9f6f 100644 --- a/esphome/components/esp32_ble_beacon/esp32_ble_beacon.cpp +++ b/esphome/components/esp32_ble_beacon/esp32_ble_beacon.cpp @@ -67,6 +67,8 @@ void ESP32BLEBeacon::setup() { this->on_advertise_(); } }); + // A beacon always needs the device to advertise, and never releases the request + global_ble->advertising_start(); } void ESP32BLEBeacon::on_advertise_() { diff --git a/esphome/components/esp32_ble_server/__init__.py b/esphome/components/esp32_ble_server/__init__.py index 855a3be29b..d8095cd702 100644 --- a/esphome/components/esp32_ble_server/__init__.py +++ b/esphome/components/esp32_ble_server/__init__.py @@ -596,6 +596,18 @@ async def to_code(config): cg.add(var.set_parent(parent)) cg.add(parent.advertising_set_appearance(config[CONF_APPEARANCE])) cg.add(var.set_max_clients(config[CONF_MAX_CLIENTS])) + # Only advertise for the server itself when the configuration gives clients something to + # find. A server that is auto-loaded purely to host a runtime service (esp32_improv) stays + # silent until that service asks for advertising. + cg.add( + var.set_advertising_required( + CONF_MANUFACTURER_DATA in config + or any( + not uuid_is(service_config[CONF_UUID], DEVICE_INFORMATION_SERVICE_UUID) + for service_config in config[CONF_SERVICES] + ) + ) + ) if CONF_MANUFACTURER_DATA in config: cg.add(var.set_manufacturer_data(config[CONF_MANUFACTURER_DATA])) for service_config in config[CONF_SERVICES]: diff --git a/esphome/components/esp32_ble_server/ble_server.cpp b/esphome/components/esp32_ble_server/ble_server.cpp index 2dea1666bb..45679b9b98 100644 --- a/esphome/components/esp32_ble_server/ble_server.cpp +++ b/esphome/components/esp32_ble_server/ble_server.cpp @@ -81,6 +81,7 @@ void BLEServer::loop() { if (this->device_information_service_->is_running()) { this->state_ = RUNNING; this->restart_advertising_(); + this->request_advertising_(); ESP_LOGD(TAG, "BLE server setup successfully"); } else if (this->device_information_service_->is_created()) { this->device_information_service_->start(); @@ -98,6 +99,20 @@ void BLEServer::restart_advertising_() { } } +void BLEServer::request_advertising_() { + if (!this->advertising_required_ || this->advertising_requested_) + return; + this->advertising_requested_ = true; + this->parent_->advertising_start(); +} + +void BLEServer::release_advertising_() { + if (!this->advertising_requested_) + return; + this->advertising_requested_ = false; + this->parent_->advertising_stop(); +} + BLEService *BLEServer::create_service(ESPBTUUID uuid, bool advertise, uint16_t num_handles) { #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE char uuid_buf[esp32_ble::UUID_STR_LEN]; @@ -170,7 +185,7 @@ void BLEServer::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t ga this->add_client_(param->connect.conn_id); // Resume advertising so additional clients can discover and connect if (this->client_count_ < this->max_clients_) { - this->parent_->advertising_start(); + this->parent_->advertising_refresh(); } this->dispatch_callbacks_(CallbackType::ON_CONNECT, param->connect.conn_id); break; @@ -178,7 +193,7 @@ void BLEServer::gatts_event_handler(esp_gatts_cb_event_t event, esp_gatt_if_t ga case ESP_GATTS_DISCONNECT_EVT: { ESP_LOGD(TAG, "BLE Client disconnected"); this->remove_client_(param->disconnect.conn_id); - this->parent_->advertising_start(); + this->parent_->advertising_refresh(); this->dispatch_callbacks_(CallbackType::ON_DISCONNECT, param->disconnect.conn_id); break; } @@ -226,6 +241,8 @@ void BLEServer::remove_client_(uint16_t conn_id) { } void BLEServer::ble_before_disabled_event_handler() { + // Advertising is re-requested once the server is running again after BLE is re-enabled + this->release_advertising_(); // Delete all clients this->client_count_ = 0; // Delete all services diff --git a/esphome/components/esp32_ble_server/ble_server.h b/esphome/components/esp32_ble_server/ble_server.h index fdd92812cd..7869c73cc5 100644 --- a/esphome/components/esp32_ble_server/ble_server.h +++ b/esphome/components/esp32_ble_server/ble_server.h @@ -38,6 +38,13 @@ class BLEServer final : public Component, public Parented { this->restart_advertising_(); } + /** Whether this server needs the device to advertise so clients can find and connect to it. + * + * False for a server that only hosts services created at runtime (e.g. esp32_improv), which + * request advertising themselves for as long as they need it. + */ + void set_advertising_required(bool required) { this->advertising_required_ = required; } + void set_max_clients(uint8_t max_clients) { this->max_clients_ = max_clients; } uint8_t get_max_clients() const { return this->max_clients_; } @@ -82,6 +89,8 @@ class BLEServer final : public Component, public Parented { }; void restart_advertising_(); + void request_advertising_(); + void release_advertising_(); int8_t find_client_index_(uint16_t conn_id) const; void add_client_(uint16_t conn_id); @@ -93,6 +102,8 @@ class BLEServer final : public Component, public Parented { std::vector manufacturer_data_{}; esp_gatt_if_t gatts_if_{0}; bool registered_{false}; + bool advertising_required_{true}; + bool advertising_requested_{false}; uint16_t clients_[USE_ESP32_BLE_MAX_CONNECTIONS]{}; uint8_t client_count_{0}; diff --git a/esphome/components/esp32_improv/esp32_improv_component.cpp b/esphome/components/esp32_improv/esp32_improv_component.cpp index 4756fba637..9ec6eb7bab 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.cpp +++ b/esphome/components/esp32_improv/esp32_improv_component.cpp @@ -112,6 +112,7 @@ void ESP32ImprovComponent::loop() { this->state_callback_.call(this->state_, this->error_state_); #endif } + this->release_advertising_(); this->incoming_data_.clear(); return; } @@ -143,8 +144,9 @@ void ESP32ImprovComponent::loop() { ESP_LOGV(TAG, "Starting with device name advertising"); this->advertising_device_name_ = true; this->last_name_adv_time_ = App.get_loop_component_start_time(); + // Set the payload before requesting, so advertising starts exactly once esp32_ble::global_ble->advertising_set_service_data_and_name(std::span{}, true); - esp32_ble::global_ble->advertising_start(); + this->request_advertising_(); // Set initial state based on whether we have an authorizer this->set_state_(this->get_initial_state_(), false); @@ -326,6 +328,8 @@ void ESP32ImprovComponent::stop() { this->set_timeout("end-service", STOP_ADVERTISING_DELAY, [this] { if (this->state_ == improv::STATE_STOPPED || this->service_ == nullptr) return; + // Release first so removing the service UUID does not restart advertising on the way out + this->release_advertising_(); this->service_->stop(); this->set_state_(improv::STATE_STOPPED); }); @@ -520,6 +524,20 @@ void ESP32ImprovComponent::update_advertising_type_() { } } +void ESP32ImprovComponent::request_advertising_() { + if (this->advertising_requested_) + return; + this->advertising_requested_ = true; + esp32_ble::global_ble->advertising_start(); +} + +void ESP32ImprovComponent::release_advertising_() { + if (!this->advertising_requested_) + return; + this->advertising_requested_ = false; + esp32_ble::global_ble->advertising_stop(); +} + improv::State ESP32ImprovComponent::get_initial_state_() const { #ifdef USE_BINARY_SENSOR // If we have an authorizer, start in awaiting authorization state diff --git a/esphome/components/esp32_improv/esp32_improv_component.h b/esphome/components/esp32_improv/esp32_improv_component.h index 414948c977..a40d60552a 100644 --- a/esphome/components/esp32_improv/esp32_improv_component.h +++ b/esphome/components/esp32_improv/esp32_improv_component.h @@ -104,8 +104,11 @@ class ESP32ImprovComponent final : public Component, public improv_base::ImprovB bool status_indicator_state_{false}; uint32_t last_name_adv_time_{0}; bool advertising_device_name_{false}; + bool advertising_requested_{false}; void set_status_indicator_state_(bool state); void update_advertising_type_(); + void request_advertising_(); + void release_advertising_(); void set_state_(improv::State state, bool update_advertising = true); void set_error_(improv::Error error); diff --git a/tests/component_tests/esp32_ble_server/config/improv_only.yaml b/tests/component_tests/esp32_ble_server/config/improv_only.yaml new file mode 100644 index 0000000000..8a5c3ba638 --- /dev/null +++ b/tests/component_tests/esp32_ble_server/config/improv_only.yaml @@ -0,0 +1,13 @@ +esphome: + name: test + +esp32: + variant: esp32 + +wifi: + ssid: MySSID + password: password1 + +# esp32_ble_server is only auto-loaded here, so it has no services of its own. +esp32_improv: + authorizer: none diff --git a/tests/component_tests/esp32_ble_server/config/manufacturer_data_only.yaml b/tests/component_tests/esp32_ble_server/config/manufacturer_data_only.yaml new file mode 100644 index 0000000000..b7bdae4af7 --- /dev/null +++ b/tests/component_tests/esp32_ble_server/config/manufacturer_data_only.yaml @@ -0,0 +1,9 @@ +esphome: + name: test + +esp32: + variant: esp32 + +esp32_ble_server: + id: ble_server + manufacturer_data: [0x72, 0x04, 0x00, 0x23] diff --git a/tests/component_tests/esp32_ble_server/config/own_service.yaml b/tests/component_tests/esp32_ble_server/config/own_service.yaml new file mode 100644 index 0000000000..c7ef0287b0 --- /dev/null +++ b/tests/component_tests/esp32_ble_server/config/own_service.yaml @@ -0,0 +1,14 @@ +esphome: + name: test + +esp32: + variant: esp32 + +esp32_ble_server: + id: ble_server + services: + - uuid: 2a24b789-7aab-4535-af3e-ee76a35cc12d + characteristics: + - uuid: cad48e28-7fbe-41cf-bae9-d77a6c233423 + read: true + value: [1, 2, 3, 4] diff --git a/tests/component_tests/esp32_ble_server/test_esp32_ble_server.py b/tests/component_tests/esp32_ble_server/test_esp32_ble_server.py index 88307d0dcf..4b7ab79a81 100644 --- a/tests/component_tests/esp32_ble_server/test_esp32_ble_server.py +++ b/tests/component_tests/esp32_ble_server/test_esp32_ble_server.py @@ -1,5 +1,10 @@ """Tests for esp32_ble_server configuration helpers.""" +from __future__ import annotations + +from collections.abc import Callable +from pathlib import Path + import pytest from esphome.components.esp32_ble_server import ( @@ -45,3 +50,26 @@ def test_uuid_is_matches_descriptor_short_strings(uuid16) -> None: assert uuid_is(uuid16, uuid16) assert uuid_is(f"{uuid16:04X}", uuid16) assert uuid_is(f"{uuid16:08X}", uuid16) + + +@pytest.mark.parametrize( + ("config_file", "required"), + [ + # Auto-loaded by esp32_improv only: nothing to find until Improv asks for it + ("improv_only.yaml", False), + # The configuration defines a service clients are meant to connect to + ("own_service.yaml", True), + # Manufacturer data is only useful if it is actually broadcast + ("manufacturer_data_only.yaml", True), + ], +) +def test_advertising_required( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], + config_file: str, + required: bool, +) -> None: + """The server only requests advertising when the configuration needs it.""" + main_cpp = generate_main(component_config_path(config_file)) + + assert f"set_advertising_required({str(required).lower()})" in main_cpp