From 37a59a07bce1947604324e875bcd3da3bb15f54a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 11 Aug 2026 18:40:47 -0500 Subject: [PATCH] [bluetooth_proxy] Retry dropped GATT acks and stop the retry log spam (#18259) --- .../bluetooth_connection.h | 4 + .../bluetooth_connection_bluedroid.cpp | 6 +- .../bluetooth_connection_hub.cpp | 137 +++++++++++++++--- .../bluetooth_connection_hub.h | 76 +++++++++- .../bluetooth_proxy/bluetooth_proxy.cpp | 21 ++- .../bluetooth_proxy/bluetooth_proxy.h | 3 +- 6 files changed, 212 insertions(+), 35 deletions(-) diff --git a/esphome/components/bluetooth_connection/bluetooth_connection.h b/esphome/components/bluetooth_connection/bluetooth_connection.h index 53e319e369..d200c6b48f 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection.h @@ -92,6 +92,10 @@ static_assert(DONE_SENDING_SERVICES != GATT_NOT_CONNECTED && INIT_SENDING_SERVIC // delivered near the client's 30 s timeout could land on a fresh request's // empty accumulator and cache as an empty database. static constexpr uint8_t SERVICES_DONE_RETRY_LIMIT = 30; +// Owed-ack retries stop after ~25 s of subscribed drain time from the first +// refusal, keeping most of the client's 30 s GATT window for congestion to +// clear while still bounding how stale a delivered reply can be. +static constexpr uint16_t PENDING_ACK_RETRY_LIMIT = 250; // ---- Service-streaming size budget, shared by every platform's streamer ---- diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index d6b815fc2e..99a6a312ec 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -397,6 +397,8 @@ void BluedroidGattClient::deliver_pending_search_() { // which proxy builds compile without a materializer. static_assert(requires(BluedroidGattClient c, BluetoothConnection &conn) { c.stream_service_batch(conn); }); +// Bound by the SERVICE STREAMING HAZARD note at the top of +// bluetooth_connection_hub.cpp: never skip a batch, never send done early. void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { if (this->services_released_) { // Released under the stream: park without services-done so a partial @@ -527,9 +529,11 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { // On a failed send, rewind the cursor so the batch is retried instead of // silently skipped. if (!api_conn->send_message(resp)) { - ESP_LOGW(TAG, "[%d] [%s] Failed to send service batch, retrying", conn.connection_index_, conn.address_str_); + conn.note_batch_stalled_(); conn.send_service_ = batch_start; + return; } + conn.batch_stalled_ = false; } #endif // USE_BLUETOOTH_PROXY diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index c43b2a6f7c..3909e16305 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -1,4 +1,24 @@ // The proxy's per-slot connection wrapper, shared by every platform. +// +// SERVICE STREAMING HAZARD - read before touching the streaming code here or +// in the platform streamers (bluetooth_connection_bluedroid.cpp). +// +// A V3 client caches the service list it receives as the device's complete, +// permanent database. Nothing on the wire marks a list as partial, so a +// stream that is truncated, has a skipped batch, or is terminated early +// would be cached whole and poison every later session with the device. +// +// The rule: it is always better to send nothing and let the client time out +// than to let services-done follow an incomplete stream. Concretely: +// - a refused batch rewinds the cursor and is retried, never skipped; +// - services-done is sent only after every batch was accepted; +// - every interruption (subscriber lost or swapped, backend abort, +// bounds-check failure) parks or aborts WITHOUT services-done and drops +// any owed done; +// - a new GetServices supersedes an owed done, so a stale done can never +// land on a fresh request's empty accumulator and cache it as empty. +// The client only caches a list terminated by services-done within the same +// request; timeouts, disconnects and errors raise instead of caching. #include "bluetooth_connection_hub.h" #ifdef BLUETOOTH_CONNECTION_HAS_GATT @@ -16,6 +36,9 @@ static const char *const TAG = "bluetooth_connection"; void BluetoothConnection::set_address(uint64_t address) { // Keep the proxy's pre-allocated connections-free message in step this->proxy_->update_address_slot_(this->address_, address); + // Slot changing hands: anything owed belonged to the old address. The + // choke point for every reassignment, not just reset_connection_()'s path. + this->clear_pending_ack_(); this->address_ = address; if (address == 0) { this->address_str_[0] = '\0'; @@ -73,6 +96,9 @@ void BluetoothConnection::reset_connection_(conn_err_t reason) { this->state_ = ClientState::IDLE; this->services_discovered_ = false; this->paired_ = false; + // Link gone: the slot may hold a different device before the drain runs. + this->clear_pending_ack_(); + this->batch_stalled_ = false; this->backend_->release_services(); this->proxy_->reset_connection_slot_(this, reason); } @@ -163,13 +189,85 @@ void BluetoothConnection::log_gatt_operation_error_(const char *operation, uint1 operation, handle, status); } +void BluetoothConnection::note_batch_stalled_() { + if (this->batch_stalled_) + return; + this->batch_stalled_ = true; + ESP_LOGW(TAG, "[%d] [%s] Service batch deferred, TCP buffer full; retrying", this->connection_index_, + this->address_str_); +} + +/// Both payload-free acks are just (address, handle); only the type differs. +template +static bool send_handle_reply(api::APIConnection *api_connection, uint64_t address, uint16_t handle) { + Response resp; + resp.address = address; + resp.handle = handle; + return api_connection->send_message(resp); +} + +/// Sole construction site, so a re-offer cannot drift from the original. +bool BluetoothConnection::try_send_ack_(PendingAck kind, uint16_t handle, conn_err_t error) { + if (kind == PendingAck::PENDING_ACK_ERROR) { + // Proxy owns the error reply and reports a refusal the same way. + return this->proxy_->send_gatt_error(this->address_, handle, error); + } + auto *api_connection = this->proxy_->get_api_connection(); + if (api_connection == nullptr) + return true; // Nobody subscribed: nothing is owed + switch (kind) { + case PendingAck::PENDING_ACK_WRITE: + return send_handle_reply(api_connection, this->address_, handle); + case PendingAck::PENDING_ACK_NOTIFY: + return send_handle_reply(api_connection, this->address_, handle); + case PendingAck::PENDING_ACK_NONE: + case PendingAck::PENDING_ACK_ERROR: // returned above + return true; + } + // No default label above, so a new enumerator is a -Wswitch warning rather + // than a silent notify reply. This return only satisfies -Wreturn-type. + return true; +} + +void BluetoothConnection::send_ack_(PendingAck kind, uint16_t handle, conn_err_t error) { + if (this->try_send_ack_(kind, handle, error)) + return; + // Report a newly owed reply and a displaced one; displacing is the case + // that loses a reply. Re-refusing the same one stays quiet. + if (!this->has_pending_ack_()) { + ESP_LOGW(TAG, "[%d] [%s] GATT reply for handle 0x%04X deferred, TCP buffer full", this->connection_index_, + this->address_str_, handle); + } else if (this->pending_ack_handle_ != handle || this->pending_ack_ != kind) { + ESP_LOGW(TAG, "[%d] [%s] GATT reply for handle 0x%04X dropped for handle 0x%04X", this->connection_index_, + this->address_str_, this->pending_ack_handle_, handle); + } + this->latch_pending_ack_(kind, handle, error); +} + +void BluetoothConnection::flush_pending_ack_() { + // No-op on its own rather than relying on the proxy drain's pre-check. + if (!this->has_pending_ack_()) + return; + if (this->try_send_ack_(this->pending_ack_, this->pending_ack_handle_, this->pending_ack_error_)) { + this->clear_pending_ack_(); + return; + } + if (++this->pending_ack_retries_ >= PENDING_ACK_RETRY_LIMIT) { + // Undeliverable: past here the client has given up and may have re-asked, + // and a late reply would answer the new request instead of this one. + ESP_LOGW(TAG, "[%d] [%s] GATT reply for handle 0x%04X undeliverable, abandoning", this->connection_index_, + this->address_str_, this->pending_ack_handle_); + this->clear_pending_ack_(); + } +} + void BluetoothConnection::on_read_result(uint16_t handle, const uint8_t *data, uint16_t len, int error) { // Late completion for a freed slot; nothing to report. if (this->address_ == 0) return; if (error != 0) { this->log_gatt_operation_error_("reading char/descriptor", handle, error); - this->proxy_->send_gatt_error(this->address_, handle, error); + this->send_gatt_error_(handle, error); return; } auto *api_connection = this->proxy_->get_api_connection(); @@ -180,6 +278,8 @@ void BluetoothConnection::on_read_result(uint16_t handle, const uint8_t *data, u resp.handle = handle; resp.set_data(data, len); if (!api_connection->send_message(resp)) { + // Not latched: would mean holding the payload through the congestion + // that refused it. The client's read timeout arbitrates. ESP_LOGW(TAG, "[%d] [%s] Failed to send read response", this->connection_index_, this->address_str_); } } @@ -189,18 +289,10 @@ void BluetoothConnection::on_write_result(uint16_t handle, int error) { return; if (error != 0) { this->log_gatt_operation_error_("writing char/descriptor", handle, error); - this->proxy_->send_gatt_error(this->address_, handle, error); + this->send_gatt_error_(handle, error); return; } - auto *api_connection = this->proxy_->get_api_connection(); - if (api_connection == nullptr) - return; - api::BluetoothGATTWriteResponse resp; - resp.address = this->address_; - resp.handle = handle; - if (!api_connection->send_message(resp)) { - ESP_LOGW(TAG, "[%d] [%s] Failed to send write response", this->connection_index_, this->address_str_); - } + this->send_ack_(PendingAck::PENDING_ACK_WRITE, handle); } void BluetoothConnection::on_notify_state(uint16_t handle, bool enabled, int error) { @@ -209,18 +301,10 @@ void BluetoothConnection::on_notify_state(uint16_t handle, bool enabled, int err if (error != 0) { this->log_gatt_operation_error_(enabled ? "registering notifications" : "unregistering notifications", handle, error); - this->proxy_->send_gatt_error(this->address_, handle, error); + this->send_gatt_error_(handle, error); return; } - auto *api_connection = this->proxy_->get_api_connection(); - if (api_connection == nullptr) - return; - api::BluetoothGATTNotifyResponse resp; - resp.address = this->address_; - resp.handle = handle; - if (!api_connection->send_message(resp)) { - ESP_LOGW(TAG, "[%d] [%s] Failed to send notify state response", this->connection_index_, this->address_str_); - } + this->send_ack_(PendingAck::PENDING_ACK_NOTIFY, handle); } void BluetoothConnection::on_notify_data(uint16_t handle, const uint8_t *data, uint16_t len) { @@ -235,6 +319,8 @@ void BluetoothConnection::on_notify_data(uint16_t handle, const uint8_t *data, u resp.handle = handle; resp.set_data(data, len); if (!api_connection->send_message(resp)) { + // Not latched, same reason as the read reply. Notify data is lossy: the + // peripheral will not resend it. ESP_LOGW(TAG, "[%d] [%s] Failed to send notify data response", this->connection_index_, this->address_str_); } } @@ -251,6 +337,7 @@ conn_err_t BluetoothConnection::check_connected_op_(const char *action, const ch } conn_err_t BluetoothConnection::read_characteristic(uint16_t handle) { + this->supersede_pending_ack_(handle, PendingAck::PENDING_ACK_NONE); if (conn_err_t err = this->check_connected_op_("read", "characteristic"); err != CONN_OK) return err; ESP_LOGV(TAG, "[%d] [%s] Reading GATT characteristic handle %d", this->connection_index_, this->address_str_, handle); @@ -259,6 +346,7 @@ conn_err_t BluetoothConnection::read_characteristic(uint16_t handle) { conn_err_t BluetoothConnection::write_characteristic(uint16_t handle, const uint8_t *data, size_t length, bool response) { + this->supersede_pending_ack_(handle, PendingAck::PENDING_ACK_WRITE); if (conn_err_t err = this->check_connected_op_("write", "characteristic"); err != CONN_OK) return err; ESP_LOGV(TAG, "[%d] [%s] Writing GATT characteristic handle %d", this->connection_index_, this->address_str_, handle); @@ -266,6 +354,7 @@ conn_err_t BluetoothConnection::write_characteristic(uint16_t handle, const uint } conn_err_t BluetoothConnection::read_descriptor(uint16_t handle) { + this->supersede_pending_ack_(handle, PendingAck::PENDING_ACK_NONE); if (conn_err_t err = this->check_connected_op_("read", "descriptor"); err != CONN_OK) return err; ESP_LOGV(TAG, "[%d] [%s] Reading GATT descriptor handle %d", this->connection_index_, this->address_str_, handle); @@ -276,6 +365,7 @@ conn_err_t BluetoothConnection::read_descriptor(uint16_t handle) { // the response flag is intentionally ignored (esp32 maps it to RSP/NO_RSP). conn_err_t BluetoothConnection::write_descriptor(uint16_t handle, const uint8_t *data, size_t length, bool /*response*/) { + this->supersede_pending_ack_(handle, PendingAck::PENDING_ACK_WRITE); if (conn_err_t err = this->check_connected_op_("write", "descriptor"); err != CONN_OK) return err; ESP_LOGV(TAG, "[%d] [%s] Writing GATT descriptor handle %d", this->connection_index_, this->address_str_, handle); @@ -283,6 +373,7 @@ conn_err_t BluetoothConnection::write_descriptor(uint16_t handle, const uint8_t } conn_err_t BluetoothConnection::notify_characteristic(uint16_t handle, bool enable) { + this->supersede_pending_ack_(handle, PendingAck::PENDING_ACK_NOTIFY); if (conn_err_t err = this->check_connected_op_("notify", "characteristic"); err != CONN_OK) return err; ESP_LOGV(TAG, "[%d] [%s] %s GATT characteristic notifications handle %d", this->connection_index_, this->address_str_, @@ -413,9 +504,11 @@ void BluetoothConnection::send_service_for_discovery_() { // (bounded: a subscriber that stays gone ends streaming via the api-lost // rewind above). if (!api_conn->send_message(resp)) { - ESP_LOGW(TAG, "[%d] [%s] Failed to send service batch, retrying", this->connection_index_, this->address_str_); + this->note_batch_stalled_(); this->send_service_ = batch_start; + return; } + this->batch_stalled_ = false; } } // namespace esphome::bluetooth_connection diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h index d964af5530..b0d0f5fd46 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h @@ -25,6 +25,16 @@ namespace esphome::bluetooth_connection { using ClientState = ble_device_base::ClientState; using ConnectionType = ble_device_base::ConnectionType; +/// A refused GATT reply owed to the current subscriber. Payload-free only: +/// these rebuild from address + handle + error, so a retry costs no buffered +/// data. Read and notify-data carry payloads and are deliberately absent. +enum class PendingAck : uint8_t { + PENDING_ACK_NONE = 0, + PENDING_ACK_WRITE, + PENDING_ACK_NOTIFY, + PENDING_ACK_ERROR, +}; + class BluetoothConnection final : public ble_device_base::GattClientListener { public: /// Wire the platform backend. Called from codegen before setup. @@ -116,6 +126,42 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { this->pending_error_ = err; } } + + /// Latch a refused reply for the proxy drain. One slot per connection, + /// newest wins: a GATT client works one request at a time, and a discarded + /// reply falls back to the timeout it would have hit anyway. + void latch_pending_ack_(PendingAck kind, uint16_t handle, conn_err_t error = 0) { + this->pending_ack_retries_ = 0; + this->pending_ack_ = kind; + this->pending_ack_handle_ = handle; + this->pending_ack_error_ = error; + } + void clear_pending_ack_() { this->pending_ack_ = PendingAck::PENDING_ACK_NONE; } + /// Drop an owed reply this re-ask makes stale. Clients match futures on + /// response type as well as handle, so an owed error (which resolves any op + /// on the handle) is cleared by any re-ask, other kinds only by their own. + void supersede_pending_ack_(uint16_t handle, PendingAck kind) { + if (this->has_pending_ack_() && this->pending_ack_handle_ == handle && + (this->pending_ack_ == PendingAck::PENDING_ACK_ERROR || this->pending_ack_ == kind)) { + this->clear_pending_ack_(); + } + } + bool has_pending_ack_() const { return this->pending_ack_ != PendingAck::PENDING_ACK_NONE; } + /// Warn on the stall's leading edge only. The batch is never lost (the + /// caller rewinds the cursor), and a warning per attempt would add traffic + /// to the connection already refusing frames. Both streamers route here. + void note_batch_stalled_(); + /// Sole construction site for these replies, shared by send and retry. + bool try_send_ack_(PendingAck kind, uint16_t handle, conn_err_t error); + /// First attempt: send, and latch it for the drain if the API refuses. + void send_ack_(PendingAck kind, uint16_t handle, conn_err_t error = 0); + /// Report a rejected request. Latched like a completion reply, so a + /// refused frame does not strand the client for its whole timeout. + void send_gatt_error_(uint16_t handle, conn_err_t error) { + this->send_ack_(PendingAck::PENDING_ACK_ERROR, handle, error); + } + /// Re-offer the owed reply; clears on success, stays owed on a refusal. + void flush_pending_ack_(); // A backend providing its own streamer (see the contract doc) builds the // response in place from its stack cache; the rest use the table streamer. // Template so the discarded branch is not odr-checked against backends @@ -131,6 +177,9 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { /// interrupted stream must never be declared complete (the client's /// timeout arbitrates), and an owed done is dropped with it. void park_service_stream_() { + // Agree with reset_connection_(): a stall flag left set would swallow the + // next session's leading-edge warning. + this->batch_stalled_ = false; if (this->send_service_ >= 0) { this->backend_->release_services(); this->send_service_ = DONE_SENDING_SERVICES; @@ -153,23 +202,31 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { bluetooth_proxy::BluetoothProxy *proxy_{nullptr}; ble_device_base::BLEGattConnection *backend_{nullptr}; - // Group 2: 2-byte types + // Group 2: 2-byte types. Exactly 4 bytes, so address_ below stays + // 8-aligned with no padding (the vptr makes Group 1 12 bytes, not 8). int16_t send_service_{INIT_SENDING_SERVICES}; uint16_t mtu_{ble_device_base::DEFAULT_ATT_MTU}; // Group 3: 8-byte and 4-byte types uint64_t address_{0}; conn_err_t pending_error_{0}; + // Full width: the GATT error domain is open-ended (ble_gatt_client.h) and + // forwarded untranslated, so narrowing would corrupt platform codes. + conn_err_t pending_ack_error_{0}; // Group 4: Arrays char address_str_[MAC_ADDRESS_PRETTY_BUFFER_SIZE]{}; + // Parked here rather than in Group 2: address_str_ ends 2-aligned, so this + // uses tail slack instead of pushing address_ out by 6 bytes of padding. + uint16_t pending_ack_handle_{0}; - // Group 5: bit-packed tail; within 2 bytes the 8-aligned object stays 48. + // Group 5: bit-packed tail. pending_ack_error_ takes the 8-aligned object + // from 48 to 56, so the third tail byte is free; first two stay packed. static_assert(static_cast(ClientState::ESTABLISHED) < (1 << 3), "state_ bitfield too narrow"); static_assert(static_cast(ConnectionType::V3_WITHOUT_CACHE) < (1 << 2), "connection_type_ bitfield too narrow"); // Ordered so neither byte's fields straddle a storage unit: 3+5 and - // 4+2+1+1 fill the two tail bytes exactly. + // 4+2+1+1 fill the first two tail bytes exactly. ClientState state_ : 3 {ClientState::IDLE}; static_assert(SERVICES_DONE_RETRY_LIMIT < (1 << 5), "counter bitfield too narrow"); uint8_t services_done_retries_ : 5 {0}; @@ -177,8 +234,21 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { ConnectionType connection_type_ : 2 {ConnectionType::V1}; bool paired_ : 1 {false}; bool services_discovered_ : 1 {false}; + static_assert(static_cast(PendingAck::PENDING_ACK_ERROR) < (1 << 2), "pending_ack_ bitfield too narrow"); + PendingAck pending_ack_ : 2 {PendingAck::PENDING_ACK_NONE}; + /// Set while a refused batch is retrying, so only the first one warns. + bool batch_stalled_ : 1 {false}; + // Plain byte after the bitfields: takes the padding byte instead of + // straddling pending_ack_'s storage unit and growing the object. + static_assert(PENDING_ACK_RETRY_LIMIT <= 0xFF, "retry counter too narrow"); + uint8_t pending_ack_retries_{0}; }; +// Pins the grouping above: pending_ack_handle_ in Group 2 instead would pad +// address_ out and reach 64. 32-bit only; the host unit tests build 64-bit. +static_assert(sizeof(void *) != 4 || sizeof(BluetoothConnection) <= 56, + "BluetoothConnection layout regressed on a 32-bit target"); + } // namespace esphome::bluetooth_connection #endif // BLUETOOTH_CONNECTION_HAS_GATT diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp index 13c84b86d1..ff5b7bc5cb 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp @@ -387,7 +387,7 @@ void BluetoothProxy::bluetooth_gatt_read(const api::BluetoothGATTReadRequest &ms auto err = connection->read_characteristic(msg.handle); if (err != CONN_OK) { - this->send_gatt_error(msg.address, msg.handle, err); + connection->send_gatt_error_(msg.handle, err); } } @@ -400,7 +400,7 @@ void BluetoothProxy::bluetooth_gatt_write(const api::BluetoothGATTWriteRequest & auto err = connection->write_characteristic(msg.handle, msg.data, msg.data_len, msg.response); if (err != CONN_OK) { - this->send_gatt_error(msg.address, msg.handle, err); + connection->send_gatt_error_(msg.handle, err); } } @@ -413,7 +413,7 @@ void BluetoothProxy::bluetooth_gatt_read_descriptor(const api::BluetoothGATTRead auto err = connection->read_descriptor(msg.handle); if (err != CONN_OK) { - this->send_gatt_error(msg.address, msg.handle, err); + connection->send_gatt_error_(msg.handle, err); } } @@ -426,7 +426,7 @@ void BluetoothProxy::bluetooth_gatt_write_descriptor(const api::BluetoothGATTWri auto err = connection->write_descriptor(msg.handle, msg.data, msg.data_len, true); if (err != CONN_OK) { - this->send_gatt_error(msg.address, msg.handle, err); + connection->send_gatt_error_(msg.handle, err); } } @@ -477,7 +477,7 @@ void BluetoothProxy::bluetooth_gatt_notify(const api::BluetoothGATTNotifyRequest auto err = connection->notify_characteristic(msg.handle, msg.enable); if (err != CONN_OK) { - this->send_gatt_error(msg.address, msg.handle, err); + connection->send_gatt_error_(msg.handle, err); } } @@ -597,6 +597,9 @@ void BluetoothProxy::loop() { if (connection->send_service_ == SERVICES_DONE_PENDING) { connection->send_services_done_(); } + if (connection->has_pending_ack_()) { + connection->flush_pending_ack_(); + } auto &owed = this->pending_disconnections_[i]; if (!owed.empty() && this->send_device_connection(owed.address(), false, 0, owed.error())) { owed.clear(); @@ -716,6 +719,8 @@ void BluetoothProxy::subscribe_api_connection(api::APIConnection *api_connection // Neither a partial stream's tail nor an owed done belongs to the new // session; silence (the client's timeout) arbitrates. this->connections_[i]->park_service_stream_(); + // An ack owed to the previous subscriber means nothing to the new one. + this->connections_[i]->clear_pending_ack_(); } this->pending_disconnections_.fill({}); #endif @@ -776,14 +781,14 @@ bool BluetoothProxy::send_gatt_services_done(uint64_t address) { return this->api_connection_->send_message(call); } -void BluetoothProxy::send_gatt_error(uint64_t address, uint16_t handle, conn_err_t error) { +bool BluetoothProxy::send_gatt_error(uint64_t address, uint16_t handle, conn_err_t error) { if (this->api_connection_ == nullptr) - return; + return true; // Nobody subscribed: nothing is owed, only a refused frame reports false api::BluetoothGATTErrorResponse call; call.address = address; call.handle = handle; call.error = error; - this->api_connection_->send_message(call); + return this->api_connection_->send_message(call); } void BluetoothProxy::send_device_pairing(uint64_t address, bool paired, conn_err_t error) { diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.h b/esphome/components/bluetooth_proxy/bluetooth_proxy.h index cf7a09a7e5..0b51d61c60 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.h +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.h @@ -145,7 +145,8 @@ class BluetoothProxy final : public Component { void send_connections_free(api::APIConnection *api_connection); /// Same convention as send_device_connection: false only on a refused frame. bool send_gatt_services_done(uint64_t address); - void send_gatt_error(uint64_t address, uint16_t handle, conn_err_t error); + /// False only when the API refused the frame, so the reply is still owed. + bool send_gatt_error(uint64_t address, uint16_t handle, conn_err_t error); void send_device_pairing(uint64_t address, bool paired, conn_err_t error = CONN_OK); void send_device_unpairing(uint64_t address, bool success, conn_err_t error = CONN_OK); void send_device_clear_cache(uint64_t address, bool success, conn_err_t error = CONN_OK);