Apply review: refusal resolves waiters, cancel skips backoff, wrap-safe hold-off

esphome/components/ble_client/__init__.py

@@ -263,15 +263,6 @@ async def register_ble_node(var, config):
     cg.add(parent.register_ble_node(var))
 
 
-def _esp32_only_action(name: str):
-    def validator(config: ConfigType) -> ConfigType:
-        if not CORE.is_esp32:
-            raise cv.Invalid(f"{name} is only supported on esp32")
-        return config
-
-    return validator
-
-
 BLE_WRITE_ACTION_SCHEMA = cv.Schema(
     {
         cv.GenerateID(CONF_ID): cv.use_id(BLEClient),
@@ -288,33 +279,33 @@ BLE_CONNECT_ACTION_SCHEMA = maybe_simple_id(
 )
 
 BLE_NUMERIC_COMPARISON_REPLY_ACTION_SCHEMA = cv.All(
+    cv.only_on_esp32,
     cv.Schema(
         {
             cv.GenerateID(CONF_ID): cv.use_id(BLEClient),
             cv.Required(CONF_ACCEPT): cv.templatable(cv.boolean),
         }
     ),
-    _esp32_only_action("ble_client.numeric_comparison_reply"),
 )
 
 BLE_PASSKEY_REPLY_ACTION_SCHEMA = cv.All(
+    cv.only_on_esp32,
     cv.Schema(
         {
             cv.GenerateID(CONF_ID): cv.use_id(BLEClient),
             cv.Required(CONF_PASSKEY): cv.templatable(cv.int_range(min=0, max=999999)),
         }
     ),
-    _esp32_only_action("ble_client.passkey_reply"),
 )
 
 
 BLE_REMOVE_BOND_ACTION_SCHEMA = cv.All(
+    cv.only_on_esp32,
     cv.Schema(
         {
             cv.GenerateID(CONF_ID): cv.use_id(BLEClient),
         }
     ),
-    _esp32_only_action("ble_client.remove_bond"),
 )
 
 

esphome/components/ble_client/automation_gatt.h

@@ -20,6 +20,9 @@
 #include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
+// Maximum bytes to log in hex format for BLE writes (many logging buffers are 256 chars)
+static constexpr size_t BLE_WRITE_MAX_LOG_BYTES = 64;
+
 namespace esphome::ble_client {
 
 // placeholder class for static TAG (shared with automation.cpp).
@@ -98,6 +101,10 @@ template<typename... Ts> class BLEClientWriteAction final : public Action<Ts...>
       esph_log_w(Automation::TAG, "Cannot write to BLE characteristic - not connected");
       return false;
     }
+#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERY_VERBOSE
+    char hex_buf[format_hex_pretty_size(BLE_WRITE_MAX_LOG_BYTES)];
+    esph_log_vv(Automation::TAG, "Will write %d bytes: %s", len, format_hex_pretty_to(hex_buf, data, len));
+#endif
     int err = this->ble_client_->write_characteristic(this->char_handle_, data, len, this->write_response_);
     if (err != 0) {
       esph_log_e(Automation::TAG, "Error writing to characteristic: %d!", err);
@@ -135,12 +142,13 @@ template<typename... Ts> class BLEClientWriteAction final : public Action<Ts...>
 
   void on_disconnected() override {
     this->resolved_ = false;
+    this->char_handle_ = 0;
     if (this->num_running_ != 0)
       this->stop_complex();
   }
 
   void on_write_result(uint16_t handle, int error) override {
-    if (handle == this->char_handle_ && this->num_running_ != 0) {
+    if (this->resolved_ && handle == this->char_handle_ && this->num_running_ != 0) {
       this->ble_client_->run_later([this]() { this->play_next_tuple_(this->var_); });
     }
   }

esphome/components/ble_client/ble_client_gatt.cpp

@@ -39,9 +39,9 @@ bool BLEClient::parse_device(const ble_device_base::ESPBTDevice &device) {
   this->address_type_ = device.get_address_type();
   if (!this->enabled || !this->auto_connect_ || this->state_ != State::IDLE)
     return true;
-  if (this->hold_off_until_ != 0 && millis() < this->hold_off_until_)
+  if (this->hold_off_ms_ != 0 && millis() - this->hold_off_start_ < this->hold_off_ms_)
     return true;
-  this->attempt_connect_(false);
+  this->attempt_connect_();
   return true;
 }
 
@@ -51,15 +51,18 @@ void BLEClient::connect() {
   // Without a fresh sighting each attempt can inhibit scanning for the
   // backend's full connect timeout (20 s on rp2) if the peer is absent.
   ESP_LOGW(TAG, "[%s] Connecting without a recent advertisement", this->address_str_);
-  this->attempt_connect_(true);
+  this->attempt_connect_();
 }
 
-void BLEClient::attempt_connect_(bool from_action) {
+void BLEClient::attempt_connect_() {
   int err = this->backend_->connect(this->address_, this->address_type_);
   if (err != 0) {
-    // A refused connect never produces a callback; stay idle and let the
-    // next sighting (or action) retry.
+    // A refused connect never produces a callback: stay idle, charge the
+    // backoff, and resolve any waiting connect action through the failure
+    // path so its chain terminates.
     ESP_LOGW(TAG, "[%s] Connect refused, err=%d", this->address_str_, err);
+    this->register_failure_();
+    this->defer([this]() { this->connect_failed_callbacks_.call(); });
     return;
   }
   ESP_LOGD(TAG, "[%s] Connecting", this->address_str_);
@@ -69,38 +72,16 @@ void BLEClient::attempt_connect_(bool from_action) {
 void BLEClient::disconnect() {
   if (this->state_ == State::IDLE)
     return;
+  // A deliberate teardown's failure report must not feed the backoff.
+  this->cancel_requested_ = true;
   this->backend_->disconnect();
 }
 
-void BLEClient::request_notify(uint16_t char_handle, uint16_t cccd_handle, uint8_t properties) {
-  if (this->cccd_count_ >= BLE_CLIENT_MAX_NOTIFY_REQUESTS) {
-    ESP_LOGW(TAG, "[%s] Too many notify requests, dropping handle 0x%04x", this->address_str_, char_handle);
-    return;
-  }
-  // Local registration is free (no operation slot); the CCCD write is
-  // serialized through the client's queue because the backend accepts one
-  // outstanding operation at a time.
-  this->backend_->notify_characteristic(char_handle, true);
-  uint8_t value = (properties & ble_device_base::GATT_CHAR_PROP_NOTIFY) ? 1 : 2;
-  this->pending_cccd_[this->cccd_count_++] = {cccd_handle, value};
-}
-
-void BLEClient::issue_next_cccd_() {
-  while (this->cccd_head_ < this->cccd_count_) {
-    const auto &pending = this->pending_cccd_[this->cccd_head_];
-    const uint8_t value[2] = {pending.value, 0x00};
-    if (this->backend_->write_descriptor(pending.handle, value, sizeof(value)) == 0) {
-      return;  // advance on the matching on_write_result
-    }
-    ESP_LOGW(TAG, "[%s] CCCD write failed for handle 0x%04x", this->address_str_, pending.handle);
-    this->cccd_head_++;
-  }
-}
-
 void BLEClient::register_failure_() {
   if (this->consecutive_failures_ < FAILURE_HOLD_OFF_MAX_STEPS)
     this->consecutive_failures_++;
-  this->hold_off_until_ = millis() + this->consecutive_failures_ * FAILURE_HOLD_OFF_STEP_MS;
+  this->hold_off_start_ = millis();
+  this->hold_off_ms_ = this->consecutive_failures_ * FAILURE_HOLD_OFF_STEP_MS;
   ESP_LOGW(TAG, "[%s] Holding off reconnect for %u s", this->address_str_,
            this->consecutive_failures_ * (FAILURE_HOLD_OFF_STEP_MS / 1000));
 }
@@ -115,8 +96,9 @@ void BLEClient::on_connection_state(bool connected, uint16_t mtu, int error) {
     return;
   }
   bool was_connected = this->state_ == State::CONNECTED;
+  bool cancelled = this->cancel_requested_;
+  this->cancel_requested_ = false;
   this->state_ = State::IDLE;
-  this->cccd_head_ = this->cccd_count_ = 0;
   if (was_connected) {
     ESP_LOGI(TAG, "[%s] Disconnected, status=%d", this->address_str_, error);
     for (auto *node : this->nodes_) {
@@ -125,8 +107,12 @@ void BLEClient::on_connection_state(bool connected, uint16_t mtu, int error) {
     // Continuations leave the backend's event-drain stack first.
     this->defer([this]() { this->disconnect_callbacks_.call(); });
   } else {
-    ESP_LOGW(TAG, "[%s] Connect failed, status=%d", this->address_str_, error);
-    this->register_failure_();
+    if (cancelled) {
+      ESP_LOGD(TAG, "[%s] Connect attempt cancelled", this->address_str_);
+    } else {
+      ESP_LOGW(TAG, "[%s] Connect failed, status=%d", this->address_str_, error);
+      this->register_failure_();
+    }
     this->defer([this]() { this->connect_failed_callbacks_.call(); });
   }
 }
@@ -145,23 +131,12 @@ void BLEClient::on_service_discovery_done(int error) {
   this->backend_->release_services();
   this->state_ = State::CONNECTED;
   this->consecutive_failures_ = 0;
-  this->hold_off_until_ = 0;
+  this->hold_off_ms_ = 0;
   ESP_LOGI(TAG, "[%s] Connected", this->address_str_);
   this->defer([this]() { this->connect_callbacks_.call(); });
-  this->issue_next_cccd_();
 }
 
 void BLEClient::on_write_result(uint16_t handle, int error) {
-  // The client's CCCD queue claims its own completion before the node
-  // fan-out sees it.
-  if (this->cccd_head_ < this->cccd_count_ && this->pending_cccd_[this->cccd_head_].handle == handle) {
-    if (error != 0) {
-      ESP_LOGW(TAG, "[%s] CCCD write error %d on handle 0x%04x", this->address_str_, error, handle);
-    }
-    this->cccd_head_++;
-    this->issue_next_cccd_();
-    return;
-  }
   for (auto *node : this->nodes_) {
     node->on_write_result(handle, error);
   }

esphome/components/ble_client/ble_client_gatt.h

@@ -22,7 +22,6 @@
 #include "esphome/core/component.h"
 #include "esphome/core/helpers.h"
 
-#include <array>
 #include <cstdint>
 #include <functional>
 #include <vector>
@@ -31,10 +30,6 @@ namespace esphome::ble_client {
 
 class BLEClient;
 
-// Concurrent notify subscriptions per client (matches the smallest backend's
-// own cap; a node requesting more logs and is dropped).
-static constexpr uint8_t BLE_CLIENT_MAX_NOTIFY_REQUESTS = 16;
-
 /// The neutral node interface. Nodes resolve their handles from the service
 /// table during on_connected() and MUST copy what they need: the table is
 /// borrowed backend storage, valid only for the duration of that call.
@@ -57,7 +52,6 @@ class BLEClient : public Component,
                   public ble_device_base::ESPBTDeviceListener,
                   public ble_device_base::GattClientListener {
  public:
-  void setup() override { this->enabled = true; }
   void dump_config() override;
 
   // Public field for legacy parity (the switch platform republishes it).
@@ -90,11 +84,6 @@ class BLEClient : public Component,
   /// continuations must leave that stack first.
   void run_later(std::function<void()> &&f) { this->defer(std::move(f)); }  // NOLINT
 
-  /// Called by nodes during on_connected(): registers the notification with
-  /// the backend and queues the CCCD write (the backend accepts one
-  /// outstanding operation, so the client serializes them).
-  void request_notify(uint16_t char_handle, uint16_t cccd_handle, uint8_t properties);
-
   // Backend ops for nodes and actions.
   int write_characteristic(uint16_t handle, const uint8_t *data, uint16_t len, bool response) {
     return this->backend_->write_characteristic(handle, data, len, response);
@@ -130,13 +119,7 @@ class BLEClient : public Component,
  protected:
   enum class State : uint8_t { IDLE, CONNECTING, DISCOVERING, CONNECTED };
 
-  struct PendingCccd {
-    uint16_t handle;
-    uint8_t value;  // 1 = notify, 2 = indicate
-  };
-
-  void attempt_connect_(bool from_action);
-  void issue_next_cccd_();
+  void attempt_connect_();
   void register_failure_();
 
   // Group 1: pointers / containers
@@ -153,19 +136,21 @@ class BLEClient : public Component,
 
   // Group 4: 4-byte types
   // Backoff after repeated failures so an undiscoverable database or a
-  // dead peer cannot produce a battery-draining connect loop.
-  uint32_t hold_off_until_{0};
+  // dead peer cannot produce a battery-draining connect loop (wrap-safe
+  // start+duration pair).
+  uint32_t hold_off_start_{0};
+  uint32_t hold_off_ms_{0};
 
   // Group 5: arrays
   char address_str_[MAC_ADDRESS_PRETTY_BUFFER_SIZE]{};
-  std::array<PendingCccd, BLE_CLIENT_MAX_NOTIFY_REQUESTS> pending_cccd_{};
 
   // Group 6: 1-byte types
   State state_{State::IDLE};
   uint8_t address_type_{0};  // BLE_ADDR_TYPE_*, captured from the sighting
   bool auto_connect_{true};
-  uint8_t cccd_head_{0};
-  uint8_t cccd_count_{0};
+  // A user-initiated teardown in flight; its failure report is not a
+  // connect failure and must not feed the backoff.
+  bool cancel_requested_{false};
   uint8_t consecutive_failures_{0};
 };
 

esphome/components/ble_client/output/__init__.py

@@ -13,7 +13,7 @@ BLEBinaryOutput = ble_client_ns.class_(
     "BLEBinaryOutput", output.BinaryOutput, ble_client.BLEClientNode, cg.Component
 )
 
-_CONFIG_SCHEMA = cv.All(
+CONFIG_SCHEMA = cv.All(
     output.BINARY_OUTPUT_SCHEMA.extend(
         {
             cv.Required(CONF_ID): cv.declare_id(BLEBinaryOutput),
@@ -66,7 +66,3 @@ async def to_code(config):
     await output.register_output(var, config)
     await ble_client.register_ble_node(var, config)
     await cg.register_component(var, config)
-
-
-# Raw-gattc node platform: not yet migrated to the neutral ble_client engine.
-CONFIG_SCHEMA = cv.All(cv.only_on_esp32, _CONFIG_SCHEMA)

esphome/components/ble_client/sensor/__init__.py

@@ -50,7 +50,7 @@ def checkType(value):
     return value
 
 
-_CONFIG_SCHEMA = cv.All(
+CONFIG_SCHEMA = cv.All(
     checkType,
     cv.typed_schema(
         {
@@ -164,7 +164,3 @@ async def to_code(config):
         await rssi_sensor_to_code(config)
     elif config[CONF_TYPE] == TYPE_CHARACTERISTIC:
         await characteristic_sensor_to_code(config)
-
-
-# Raw-gattc node platform: not yet migrated to the neutral ble_client engine.
-CONFIG_SCHEMA = cv.All(cv.only_on_esp32, _CONFIG_SCHEMA)

esphome/components/ble_client/switch/__init__.py

@@ -9,7 +9,7 @@ BLEClientSwitch = ble_client_ns.class_(
     "BLEClientSwitch", switch.Switch, cg.Component, ble_client.BLEClientNode
 )
 
-_CONFIG_SCHEMA = (
+CONFIG_SCHEMA = (
     switch.switch_schema(BLEClientSwitch, icon=ICON_BLUETOOTH, block_inverted=True)
     .extend(ble_client.BLE_CLIENT_SCHEMA)
     .extend(cv.COMPONENT_SCHEMA)
@@ -20,7 +20,3 @@ async def to_code(config):
     var = await switch.new_switch(config)
     await cg.register_component(var, config)
     await ble_client.register_ble_node(var, config)
-
-
-# Raw-gattc node platform: not yet migrated to the neutral ble_client engine.
-CONFIG_SCHEMA = cv.All(cv.only_on_esp32, _CONFIG_SCHEMA)

esphome/components/ble_client/text_sensor/__init__.py

@@ -33,7 +33,7 @@ BLETextSensorNotifyTrigger = ble_client_ns.class_(
     "BLETextSensorNotifyTrigger", automation.Trigger.template(cg.std_string)
 )
 
-_CONFIG_SCHEMA = cv.All(
+CONFIG_SCHEMA = cv.All(
     text_sensor.text_sensor_schema(BLETextSensor)
     .extend(
         {
@@ -109,7 +109,3 @@ async def to_code(config):
         trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var)
         await ble_client.register_ble_node(trigger, config)
         await automation.build_automation(trigger, [(cg.std_string, "x")], conf)
-
-
-# Raw-gattc node platform: not yet migrated to the neutral ble_client engine.
-CONFIG_SCHEMA = cv.All(cv.only_on_esp32, _CONFIG_SCHEMA)