From 3f5b8139f32ffd28722f8544a688584d1b820d05 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" <3060199+jesserockz@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:52:25 -0500 Subject: [PATCH] [bluetooth_proxy] Latch the connection replies and tighten the send paths (#18278) --- .../bluetooth_connection.cpp | 9 +- .../bluetooth_connection.h | 11 +- .../bluetooth_connection_hub.cpp | 61 ++++- .../bluetooth_connection_hub.h | 28 ++- .../bluetooth_proxy/bluetooth_proxy.cpp | 236 ++++++++---------- .../bluetooth_proxy/bluetooth_proxy.h | 24 +- 6 files changed, 220 insertions(+), 149 deletions(-) diff --git a/esphome/components/bluetooth_connection/bluetooth_connection.cpp b/esphome/components/bluetooth_connection/bluetooth_connection.cpp index 94bb119c84..a7e9825e56 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection.cpp @@ -46,12 +46,11 @@ BatchClose close_service_batch(api::BluetoothGATTGetServicesResponse &resp, size #endif // BLUETOOTH_CONNECTION_HAS_GATT -#ifdef USE_ESP32 +#if defined(USE_ESP32) && defined(USE_BLE_GATT_CLIENT) namespace esphome::bluetooth_connection { -// Address-scoped Bluedroid maintenance shared by every esp32 proxy build, -// including advertisement-only ones where no GATT backend (and none of the -// gated surface above) is compiled - so this block sits outside that gate. +// Address-scoped Bluedroid maintenance. Gated with the connection surface: +// the advertisement-only arm no longer dispatches these requests at all. conn_err_t unpair_device(uint64_t address) { esp_bd_addr_t bda; @@ -66,4 +65,4 @@ conn_err_t clear_gatt_cache(uint64_t address) { } } // namespace esphome::bluetooth_connection -#endif // USE_ESP32 +#endif // USE_ESP32 && USE_BLE_GATT_CLIENT diff --git a/esphome/components/bluetooth_connection/bluetooth_connection.h b/esphome/components/bluetooth_connection/bluetooth_connection.h index d200c6b48f..bcfbdaa6cf 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection.h @@ -20,10 +20,9 @@ // wired by codegen (one slot per connection). This is the single spelling of // that predicate - the hub wrapper and the API request handlers gate on it. // The wrapper serves the proxy's API surface, so it compiles only when a -// backend AND the proxy are present; advertisement-only and backend-only -// builds get the clean-error handlers instead. Address-scoped maintenance -// (unpair, cache clear) still works there through the per-platform free -// functions below. +// backend AND the proxy are present. The address-scoped maintenance functions +// below are only reached from that gated surface; their #else stubs just +// keep this header parsing on arms without a backend. #if defined(USE_BLE_GATT_CLIENT) && defined(USE_BLUETOOTH_PROXY) #define BLUETOOTH_CONNECTION_HAS_GATT #endif @@ -68,12 +67,12 @@ static constexpr bool SUPPORTS_CACHE_CLEARING = false; #endif // Address-scoped (not connection-scoped) maintenance requests. -#if defined(USE_ESP32) || (defined(USE_RP2040_BLE) && defined(USE_BLE_GATT_CLIENT)) +#if (defined(USE_ESP32) || defined(USE_RP2040_BLE)) && defined(USE_BLE_GATT_CLIENT) conn_err_t unpair_device(uint64_t address); #else inline conn_err_t unpair_device(uint64_t) { return GATT_NOT_CONNECTED; } #endif -#ifdef USE_ESP32 +#if defined(USE_ESP32) && defined(USE_BLE_GATT_CLIENT) conn_err_t clear_gatt_cache(uint64_t address); #else inline conn_err_t clear_gatt_cache(uint64_t) { return GATT_NOT_CONNECTED; } diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index 3909e16305..8707637e9d 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -38,7 +38,7 @@ void BluetoothConnection::set_address(uint64_t address) { 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->clear_owed_flags_(); this->address_ = address; if (address == 0) { this->address_str_[0] = '\0'; @@ -97,8 +97,7 @@ void BluetoothConnection::reset_connection_(conn_err_t reason) { 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->clear_owed_flags_(); this->backend_->release_services(); this->proxy_->reset_connection_slot_(this, reason); } @@ -142,7 +141,7 @@ void BluetoothConnection::on_connection_state(bool connected, uint16_t mtu, int ESP_LOGW(TAG, "[%d] [%s] conn param update failed, err=%d", this->connection_index_, this->address_str_, param_err); } - this->proxy_->send_device_connection(this->address_, true, mtu); + this->send_connected_reply_(); this->proxy_->send_connections_free(); return; } @@ -180,10 +179,49 @@ void BluetoothConnection::on_service_discovery_done(int error) { this->mtu_); this->state_ = ClientState::ESTABLISHED; this->services_discovered_ = true; - this->proxy_->send_device_connection(this->address_, true, this->mtu_); + this->send_connected_reply_(); this->proxy_->send_connections_free(); } +void BluetoothConnection::flush_owed_replies_() { + // Connected first: the client should never see services-done or an ack for + // a link it has not been told is up. Structural, not size-dependent: a + // still-owed connected reply defers the smaller sends to the next tick. + if (this->connected_reply_owed_) { + this->send_connected_reply_(); + if (this->connected_reply_owed_) { + // The retry limits are wall-clock windows: age the deferred budgets so + // a reply cannot outlive the window it was sized for. + if (this->send_service_ == SERVICES_DONE_PENDING) { + this->age_services_done_(); + } + if (this->has_pending_ack_()) { + this->age_pending_ack_(); + } + return; + } + } + if (this->send_service_ == SERVICES_DONE_PENDING) { + this->send_services_done_(); + } + if (this->has_pending_ack_()) { + this->flush_pending_ack_(); + } +} + +void BluetoothConnection::send_connected_reply_() { + if (this->proxy_->send_device_connection(this->address_, true, this->mtu_)) { + this->connected_reply_owed_ = false; + return; + } + // Warn on the leading edge only, as elsewhere: the drop must be visible but + // must not add traffic to the connection that just refused a frame. + if (!this->connected_reply_owed_) { + ESP_LOGW(TAG, "[%d] [%s] Connected reply deferred, TCP buffer full", this->connection_index_, this->address_str_); + this->connected_reply_owed_ = true; + } +} + void BluetoothConnection::log_gatt_operation_error_(const char *operation, uint16_t handle, int status) { ESP_LOGW(TAG, "[%d] [%s] Error %s for handle 0x%2X, status=%d", this->connection_index_, this->address_str_, operation, handle, status); @@ -245,13 +283,16 @@ void BluetoothConnection::send_ack_(PendingAck kind, uint16_t handle, conn_err_t } 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; } + this->age_pending_ack_(); +} + +void BluetoothConnection::age_pending_ack_() { 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. @@ -401,7 +442,13 @@ void BluetoothConnection::send_services_done_() { ESP_LOGW(TAG, "[%d] [%s] Failed to send services done, retrying", this->connection_index_, this->address_str_); this->services_done_retries_ = 0; this->send_service_ = SERVICES_DONE_PENDING; - } else if (++this->services_done_retries_ >= SERVICES_DONE_RETRY_LIMIT) { + } else { + this->age_services_done_(); + } +} + +void BluetoothConnection::age_services_done_() { + if (++this->services_done_retries_ >= SERVICES_DONE_RETRY_LIMIT) { // Undeliverable (see SERVICES_DONE_RETRY_LIMIT); silence arbitrates. ESP_LOGW(TAG, "[%d] [%s] Services done undeliverable, abandoning", this->connection_index_, this->address_str_); this->send_service_ = DONE_SENDING_SERVICES; diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h index b0d0f5fd46..3553f8bf00 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h @@ -151,6 +151,20 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { /// 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_(); + /// Send the connected=true reply, latching it if the API refuses. Rebuilt + /// from address_ and mtu_, so the latch is one bit; a dropped confirmation + /// leaves the client timing out while this slot holds a live link. No retry + /// bound: the slot's lifetime is the bound (teardown clears the flag). + void send_connected_reply_(); + /// Re-offer everything this slot owes. One entry point so the proxy drain + /// does not have to know which latches exist. + void flush_owed_replies_(); + /// Drop everything this slot owes, in one write to the shared tail byte. + void clear_owed_flags_() { + this->pending_ack_ = PendingAck::PENDING_ACK_NONE; + this->batch_stalled_ = false; + this->connected_reply_owed_ = false; + } /// 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. @@ -162,6 +176,8 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { } /// Re-offer the owed reply; clears on success, stays owed on a refusal. void flush_pending_ack_(); + /// Advance the retry budget and abandon at the limit, without sending. + void age_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 @@ -177,8 +193,6 @@ 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(); @@ -193,6 +207,8 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { /// retries). Callers release the table first; the message needs only the /// address. void send_services_done_(); + /// Advance the retry budget and abandon at the limit, without sending. + void age_services_done_(); void reset_connection_(conn_err_t reason); conn_err_t check_connected_op_(const char *action, const char *type) const; void log_gatt_operation_error_(const char *operation, uint16_t handle, int status); @@ -220,8 +236,10 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { // uses tail slack instead of pushing address_ out by 6 bytes of padding. uint16_t pending_ack_handle_{0}; - // 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. + // Group 5: bit-packed tail. The first two bytes were already full, so the + // first added bit forced a third and took the 8-aligned object 48 -> 56; + // the handle, error and retry counter ride in that padding. Four bitfield + // bits left; another byte-sized member costs 8 per slot. 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"); @@ -238,6 +256,8 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { 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}; + /// An owed connected=true reply; the proxy's paced drain re-offers it. + bool connected_reply_owed_ : 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"); diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp index 0d91b541e8..0cf8483cea 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp @@ -122,6 +122,18 @@ void BluetoothProxy::log_connection_info_(BluetoothConnection *connection, const } #endif // BLUETOOTH_CONNECTION_HAS_GATT +void BluetoothProxy::log_reply_dropped_(const char *what, uint64_t address) { + ESP_LOGW(TAG, "%s reply for %012" PRIX64 " dropped, TCP buffer full", what, address); +} + +void BluetoothProxy::log_reply_deferred_(const char *what, uint64_t address) { + ESP_LOGW(TAG, "%s reply for %012" PRIX64 " deferred, TCP buffer full", what, address); +} + +void BluetoothProxy::log_reply_displaced_(const char *what, uint64_t owed, uint64_t address) { + ESP_LOGW(TAG, "%s reply for %012" PRIX64 " dropped, displaced by %012" PRIX64, what, owed, address); +} + void BluetoothProxy::log_not_connected_gatt_(const char *action, const char *type) { ESP_LOGW(TAG, "Cannot %s GATT %s, not connected", action, type); } @@ -129,7 +141,10 @@ void BluetoothProxy::log_not_connected_gatt_(const char *action, const char *typ void BluetoothProxy::handle_gatt_not_connected_(uint64_t address, uint16_t handle, const char *action, const char *type) { this->log_not_connected_gatt_(action, type); - this->send_gatt_error(address, handle, GATT_NOT_CONNECTED); + if (!this->send_gatt_error(address, handle, GATT_NOT_CONNECTED)) { + // No connection, so nothing to latch against; the client's timeout arbitrates. + this->log_reply_dropped_("Not-connected", address); + } } void BluetoothProxy::log_advertisement_flush_() { @@ -209,12 +224,12 @@ void BluetoothProxy::latch_pending_disconnection_(uint64_t address, conn_err_t e } } if (free_entry != nullptr) { + this->log_reply_deferred_("Disconnect", address); free_entry->set(address, error); return; } // Every entry is owed: evict the first so the newest loss is not silent too. - ESP_LOGW(TAG, "Owed disconnect dropped (0x%llx), retry pool full", - (unsigned long long) this->pending_disconnections_[0].address()); + this->log_reply_displaced_("Disconnect", this->pending_disconnections_[0].address(), address); this->pending_disconnections_[0].set(address, error); } @@ -224,19 +239,39 @@ void BluetoothProxy::clear_pending_disconnection_(uint64_t address) { for (uint8_t i = 0; i < this->connection_count_; i++) { if (this->pending_disconnections_[i].matches(address)) { this->pending_disconnections_[i].clear(); + return; // latch_pending_disconnection_ keeps at most one entry per address } } } -void BluetoothProxy::reset_connection_slot_(BluetoothConnection *connection, conn_err_t reason) { - if (!this->send_device_connection(connection->get_address(), false, 0, reason)) { - // The client has no other way to learn of an unsolicited disconnect; - // latch and let loop()'s paced drain deliver it. V by design: a louder - // level would ride the same congested link this reports on. - ESP_LOGV(TAG, "[%d] [%s] Disconnect notification deferred, TCP buffer full", connection->get_connection_index(), - connection->address_str()); - this->latch_pending_disconnection_(connection->get_address(), reason); +void BluetoothProxy::answer_device_disconnected_(uint64_t address) { + if (this->send_device_connection(address, false)) { + // A landed answer satisfies any owed notification for the address; a + // drained duplicate would follow it otherwise. + this->clear_pending_disconnection_(address); + return; } + // Not latched: the client's own request timeout arbitrates, and pooling + // these would let a request retry loop displace an unsolicited disconnect. + this->log_reply_dropped_("Disconnect", address); +} + +void BluetoothProxy::send_device_disconnected_(uint64_t address, conn_err_t error) { + if (this->send_device_connection(address, false, 0, error)) { + // A later disconnect landing for an address that still has one owed would + // otherwise have the drain repeat it. + this->clear_pending_disconnection_(address); + return; + } + // A dropped disconnect leaves the client believing the link is live, so + // every GATT operation on it times out until something else corrects it. + // latch_pending_disconnection_() reports the leading edge. + this->latch_pending_disconnection_(address, error); +} + +void BluetoothProxy::reset_connection_slot_(BluetoothConnection *connection, conn_err_t reason) { + // The client has no other way to learn of an unsolicited disconnect. + this->send_device_disconnected_(connection->get_address(), reason); connection->set_address(0); connection->send_service_ = INIT_SENDING_SERVICES; this->send_connections_free(); @@ -282,18 +317,18 @@ void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest auto *connection = this->get_connection_(msg.address, true); if (connection == nullptr) { ESP_LOGW(TAG, "No free connections available"); - this->send_device_connection(msg.address, false); + this->answer_device_disconnected_(msg.address); return; } if (!msg.has_address_type) { ESP_LOGE(TAG, "[%d] [%s] Missing address type in connect request", connection->get_connection_index(), connection->address_str()); - this->send_device_connection(msg.address, false); + this->answer_device_disconnected_(msg.address); return; } if (connection->state() == ClientState::CONNECTED || connection->state() == ClientState::ESTABLISHED) { this->log_connection_request_ignored_(connection, connection->state()); - this->send_device_connection(msg.address, true); + connection->send_connected_reply_(); this->send_connections_free(); return; } else if (connection->state() == ClientState::DISCONNECTING && connection->cancel_teardown()) { @@ -320,7 +355,7 @@ void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_DISCONNECT: { auto *connection = this->get_connection_(msg.address, false); if (connection == nullptr) { - this->send_device_connection(msg.address, false); + this->answer_device_disconnected_(msg.address); this->send_connections_free(); return; } @@ -328,7 +363,7 @@ void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest connection->disconnect(); } else { connection->set_address(0); - this->send_device_connection(msg.address, false); + this->answer_device_disconnected_(msg.address); this->send_connections_free(); } break; @@ -372,7 +407,7 @@ void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest } case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_CONNECT: { ESP_LOGE(TAG, "V1 connections removed"); - this->send_device_connection(msg.address, false); + this->answer_device_disconnected_(msg.address); break; } } @@ -484,7 +519,8 @@ void BluetoothProxy::bluetooth_gatt_notify(const api::BluetoothGATTNotifyRequest void BluetoothProxy::bluetooth_set_connection_params(const api::BluetoothSetConnectionParamsRequest &msg) { if (this->api_connection_ == nullptr) return; - // Send results unchecked (esp32 parity): a drop resolves via the client timeout. + // Not latched (esp32 parity): the request is idempotent, so a drop resolves + // via the client timeout and a retry gives the same answer. Still reported. auto *connection = this->get_connection_(msg.address, false); api::BluetoothSetConnectionParamsResponse resp; @@ -495,7 +531,9 @@ void BluetoothProxy::bluetooth_set_connection_params(const api::BluetoothSetConn connection ? static_cast(connection->get_connection_index()) : -1, connection ? connection->address_str() : "unknown"); resp.error = GATT_NOT_CONNECTED; - this->api_connection_->send_message(resp); + if (!this->api_connection_->send_message(resp)) { + this->log_reply_dropped_("Connection-params", msg.address); + } return; } @@ -506,7 +544,9 @@ void BluetoothProxy::bluetooth_set_connection_params(const api::BluetoothSetConn static_cast(std::min(msg.max_interval, max_val)), static_cast(std::min(msg.latency, max_val)), static_cast(std::min(msg.timeout, max_val))); - this->api_connection_->send_message(resp); + if (!this->api_connection_->send_message(resp)) { + this->log_reply_dropped_("Connection-params", msg.address); + } } #endif // BLUETOOTH_CONNECTION_HAS_GATT @@ -568,9 +608,9 @@ void BluetoothProxy::loop() { if (this->connections_free_pending_ && this->api_connection_ != nullptr) { // Resend a dropped slot-state update, paced by the 100 ms gate so the - // retry does not hammer the congestion it exists to survive; the - // advertisement-only arm answers DISCONNECT requests with this message - // too, so the drain compiles on every proxy build. + // retry does not hammer the congestion it exists to survive. Every build + // sends this at subscribe time (api_connection.cpp), so the drain + // compiles on every proxy build. this->connections_free_pending_ = false; this->send_connections_free(this->api_connection_); } @@ -593,17 +633,17 @@ void BluetoothProxy::loop() { // Paced retries of owed per-slot notifications; subscriber swaps clear // stale latches before this runs. for (uint8_t i = 0; i < this->connection_count_; i++) { - auto *connection = this->connections_[i]; - if (connection->send_service_ == SERVICES_DONE_PENDING) { - connection->send_services_done_(); - } - if (connection->has_pending_ack_()) { - connection->flush_pending_ack_(); - } + this->connections_[i]->flush_owed_replies_(); + } + // Address-keyed, not slot-keyed, so it gets its own loop; bounded by + // connection_count_ like the latch and clear helpers. Not pre-cleared: + // the sender clears on success and re-latches on refusal, keeping the + // latch's leading-edge warn honest (same shape as the unpair drain). + for (uint8_t i = 0; i < this->connection_count_; i++) { auto &owed = this->pending_disconnections_[i]; - if (!owed.empty() && this->send_device_connection(owed.address(), false, 0, owed.error())) { - owed.clear(); - } + if (owed.empty()) + continue; + this->send_device_disconnected_(owed.address(), owed.error()); } // An owed unpair reply. Not pre-cleared: the sender clears on success and @@ -632,75 +672,21 @@ void BluetoothProxy::loop() { #ifndef BLUETOOTH_CONNECTION_HAS_GATT -// Advertisement-only proxy. GATT client connections are excluded at compile -// time (no connection backend on this platform, or active: false), so every -// connection-oriented request is answered with a clean error instead of -// silence, and Home Assistant treats the proxy as passive. +// Advertisement-only proxy: no connection backend on this platform, or +// active: false. get_feature_flags() then omits FEATURE_ACTIVE_CONNECTIONS, +// so a client treats the proxy as passive and never sends a connection or +// GATT request. These exist only because the api layer dispatches them +// unconditionally; answering would link response encoders this build has no +// use for. -void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest &msg) { - switch (msg.request_type) { - case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_CONNECT_V3_WITH_CACHE: - case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_CONNECT_V3_WITHOUT_CACHE: - case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_CONNECT: - ESP_LOGW(TAG, "Active connections are not supported on this platform"); - this->send_device_connection(msg.address, false, 0, GATT_NOT_CONNECTED); - break; - case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_DISCONNECT: - // Not an error: the device is already disconnected, which is the requested state. - this->send_device_connection(msg.address, false); - this->send_connections_free(); - break; - case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_PAIR: - this->send_device_pairing(msg.address, false, GATT_NOT_CONNECTED); - break; - case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_UNPAIR: { - // Address-scoped maintenance needs no connection slot: real on esp32 - // (Bluedroid bond table), the stub elsewhere keeps the old error reply. - conn_err_t ret = bluetooth_connection::unpair_device(msg.address); - this->send_device_unpairing(msg.address, ret == CONN_OK, ret); - break; - } - case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_CLEAR_CACHE: { - conn_err_t ret = bluetooth_connection::clear_gatt_cache(msg.address); - this->send_device_clear_cache(msg.address, ret == CONN_OK, ret); - break; - } - } -} - -void BluetoothProxy::bluetooth_gatt_read(const api::BluetoothGATTReadRequest &msg) { - this->handle_gatt_not_connected_(msg.address, msg.handle, "read", "characteristic"); -} - -void BluetoothProxy::bluetooth_gatt_write(const api::BluetoothGATTWriteRequest &msg) { - this->handle_gatt_not_connected_(msg.address, msg.handle, "write", "characteristic"); -} - -void BluetoothProxy::bluetooth_gatt_read_descriptor(const api::BluetoothGATTReadDescriptorRequest &msg) { - this->handle_gatt_not_connected_(msg.address, msg.handle, "read", "descriptor"); -} - -void BluetoothProxy::bluetooth_gatt_write_descriptor(const api::BluetoothGATTWriteDescriptorRequest &msg) { - this->handle_gatt_not_connected_(msg.address, msg.handle, "write", "descriptor"); -} - -void BluetoothProxy::bluetooth_gatt_send_services(const api::BluetoothGATTGetServicesRequest &msg) { - this->handle_gatt_not_connected_(msg.address, 0, "get", "services"); -} - -void BluetoothProxy::bluetooth_gatt_notify(const api::BluetoothGATTNotifyRequest &msg) { - this->handle_gatt_not_connected_(msg.address, msg.handle, "notify", "characteristic"); -} - -void BluetoothProxy::bluetooth_set_connection_params(const api::BluetoothSetConnectionParamsRequest &msg) { - if (this->api_connection_ == nullptr) - return; - // Send results unchecked (esp32 parity): a drop resolves via the client timeout. - api::BluetoothSetConnectionParamsResponse resp; - resp.address = msg.address; - resp.error = GATT_NOT_CONNECTED; - this->api_connection_->send_message(resp); -} +void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest &msg) {} +void BluetoothProxy::bluetooth_gatt_read(const api::BluetoothGATTReadRequest &msg) {} +void BluetoothProxy::bluetooth_gatt_write(const api::BluetoothGATTWriteRequest &msg) {} +void BluetoothProxy::bluetooth_gatt_read_descriptor(const api::BluetoothGATTReadDescriptorRequest &msg) {} +void BluetoothProxy::bluetooth_gatt_write_descriptor(const api::BluetoothGATTWriteDescriptorRequest &msg) {} +void BluetoothProxy::bluetooth_gatt_send_services(const api::BluetoothGATTGetServicesRequest &msg) {} +void BluetoothProxy::bluetooth_gatt_notify(const api::BluetoothGATTNotifyRequest &msg) {} +void BluetoothProxy::bluetooth_set_connection_params(const api::BluetoothSetConnectionParamsRequest &msg) {} #endif // !BLUETOOTH_CONNECTION_HAS_GATT @@ -723,8 +709,9 @@ void BluetoothProxy::reset_owed_replies_() { for (uint8_t i = 0; i < this->connection_count_; i++) { // Neither a partial stream's tail nor an owed done belongs to the next // session; silence (the client's timeout) arbitrates. - this->connections_[i]->park_service_stream_(); - this->connections_[i]->clear_pending_ack_(); + auto *connection = this->connections_[i]; + connection->park_service_stream_(); + connection->clear_owed_flags_(); } #endif } @@ -810,6 +797,7 @@ bool BluetoothProxy::send_gatt_error(uint64_t address, uint16_t handle, conn_err return this->api_connection_->send_message(call); } +#ifdef BLUETOOTH_CONNECTION_HAS_GATT void BluetoothProxy::send_device_pairing(uint64_t address, bool paired, conn_err_t error) { if (this->api_connection_ == nullptr) return; @@ -818,13 +806,16 @@ void BluetoothProxy::send_device_pairing(uint64_t address, bool paired, conn_err call.paired = paired; call.error = error; - this->api_connection_->send_message(call); + if (!this->api_connection_->send_message(call)) { + // Not latched: a retried PAIR is answered from is_paired(), so the client + // recovers on its own. Still worth saying it happened. + this->log_reply_dropped_("Pairing", address); + } } void BluetoothProxy::send_device_unpairing(uint64_t address, bool success, conn_err_t error) { if (this->api_connection_ == nullptr) return; -#ifdef BLUETOOTH_CONNECTION_HAS_GATT // An owed success is the authoritative answer: a later attempt for the // same address fails only because the first already removed the bond. if (!this->pending_unpairing_.empty() && this->pending_unpairing_.matches(address) && @@ -832,38 +823,29 @@ void BluetoothProxy::send_device_unpairing(uint64_t address, bool success, conn_ success = true; error = CONN_OK; } -#endif api::BluetoothDeviceUnpairingResponse call; call.address = address; call.success = success; call.error = error; - // Advertisement-only builds answer this with a canned reply and keep no - // retry state, so only the latch is conditional, not the send. - [[maybe_unused]] bool sent = this->api_connection_->send_message(call); -#ifdef BLUETOOTH_CONNECTION_HAS_GATT - if (sent) { + if (this->api_connection_->send_message(call)) { // A later unpair landing for an address that still has one owed would // otherwise have the drain repeat it. if (this->pending_unpairing_.matches(address)) { this->pending_unpairing_.clear(); } - } else { - // Warn on the leading edge and on displacement (that one loses a reply); - // the drain's re-refusals of the same reply stay quiet. - if (this->pending_unpairing_.empty()) { - ESP_LOGW(TAG, "Unpair reply for %012" PRIX64 " deferred, TCP buffer full", address); - } else if (!this->pending_unpairing_.matches(address)) { - ESP_LOGW(TAG, "Owed unpair reply for %012" PRIX64 " dropped, displaced by %012" PRIX64, - this->pending_unpairing_.address(), address); - } - this->pending_unpairing_.set(address, error); + return; } -#endif + if (this->pending_unpairing_.empty()) { + this->log_reply_deferred_("Unpair", address); + } else if (!this->pending_unpairing_.matches(address)) { + this->log_reply_displaced_("Unpair", this->pending_unpairing_.address(), address); + } + this->pending_unpairing_.set(address, error); } -// Shared by both platform paths: the neutral bluetooth_device_request() uses it to -// answer a clear-cache request with a clean error, so it must not be esp32-guarded. +// GATT arm only: the advertisement-only arm no longer dispatches CLEAR_CACHE, +// so its response encoder would be dead weight there. void BluetoothProxy::send_device_clear_cache(uint64_t address, bool success, conn_err_t error) { if (this->api_connection_ == nullptr) return; @@ -872,8 +854,12 @@ void BluetoothProxy::send_device_clear_cache(uint64_t address, bool success, con call.success = success; call.error = error; - this->api_connection_->send_message(call); + if (!this->api_connection_->send_message(call)) { + // Not latched: clear-cache is idempotent, so a retry gives the same answer. + this->log_reply_dropped_("Clear-cache", address); + } } +#endif BluetoothProxy *global_bluetooth_proxy = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.h b/esphome/components/bluetooth_proxy/bluetooth_proxy.h index 16b153625f..de70b35aaf 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.h +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.h @@ -138,8 +138,8 @@ class BluetoothProxy final : public Component { } /// False only when a subscriber refused the frame; true = delivered or - /// nobody subscribed. Request-answer callers ignore the result (client - /// timeouts cover those); only reset_connection_slot_ latches for retry. + /// nobody subscribed. Refusals latch in send_device_disconnected_() and + /// send_connected_reply_(); other callers report via log_reply_dropped_(). bool send_device_connection(uint64_t address, bool connected, uint16_t mtu = 0, conn_err_t error = CONN_OK); void send_connections_free(); void send_connections_free(api::APIConnection *api_connection); @@ -147,11 +147,13 @@ class BluetoothProxy final : public Component { bool send_gatt_services_done(uint64_t address); /// 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); +#ifdef BLUETOOTH_CONNECTION_HAS_GATT void send_device_pairing(uint64_t address, bool paired, conn_err_t error = CONN_OK); /// No default error: the drain rebuilds success as (error == CONN_OK), so a /// caller that omitted it would have a reported failure resent as a success. void send_device_unpairing(uint64_t address, bool success, conn_err_t error); void send_device_clear_cache(uint64_t address, bool success, conn_err_t error = CONN_OK); +#endif void bluetooth_scanner_set_mode(bool active); @@ -230,6 +232,9 @@ class BluetoothProxy final : public Component { void flush_pending_advertisements_() { if (this->response_.advertisements_len == 0) return; + // The one deliberately ignored result: advertisements are perishable and + // this is the highest-frequency send here, so reporting each drop would be + // the flood the batch pacing exists to avoid. this->api_connection_->send_message(this->response_); #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE this->log_advertisement_flush_(); @@ -282,6 +287,15 @@ class BluetoothProxy final : public Component { void reset_connection_slot_(BluetoothConnection *connection, conn_err_t reason); /// Drop any owed freed-slot notification for this address (client reconnected). void clear_pending_disconnection_(uint64_t address); + /// Send connected=false and pool it for the paced drain if refused. A + /// dropped disconnect desynchronises the proxy: the client keeps a link it + /// believes is live and every operation on it times out. Unsolicited and + /// drained notifications only; request answers use the variant below. + void send_device_disconnected_(uint64_t address, conn_err_t error = CONN_OK); + /// Answer a request with connected=false. Never pools: a refusal falls back + /// to the client's request timeout, keeping the pool for the unsolicited + /// notifications the client cannot recover on its own. + void answer_device_disconnected_(uint64_t address); /// Pool a refused freed-slot notification for the paced drain. void latch_pending_disconnection_(uint64_t address, conn_err_t error); #endif @@ -291,6 +305,12 @@ class BluetoothProxy final : public Component { /// Drops state only, never sends: api_connection_ is the departing /// subscriber on subscribe and nullptr on unsubscribe. void reset_owed_replies_(); + /// Report a reply we deliberately do not latch, so no drop is silent. + void log_reply_dropped_(const char *what, uint64_t address); + /// A latched reply's leading edge; the drain's re-refusals stay quiet. + void log_reply_deferred_(const char *what, uint64_t address); + /// A latched reply lost to a newer one for a different address. + void log_reply_displaced_(const char *what, uint64_t owed, uint64_t address); // Memory optimized layout for 32-bit systems // Group 1: Pointers (4 bytes each, naturally aligned)