[bluetooth_connection] Engine follow-ups from the rp2 GATT series (#18159)

esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp

@@ -24,6 +24,8 @@ using ble_device_base::GATT_ERR_NO_MEMORY;
 // disconnect timeout mirrors the esp32 CLOSE_EVT safety net.
 static constexpr uint32_t CONNECT_TIMEOUT_MS = 20000;
 static constexpr uint32_t DISCONNECT_TIMEOUT_MS = 10000;
+// Can-send windows normally open within a connection interval (tens of ms).
+static constexpr uint32_t WRITE_NO_RSP_TIMEOUT_MS = 500;
 
 // HCI "connection timeout" reason, reported when a teardown had to be forced.
 static constexpr uint8_t HCI_REASON_CONNECTION_TIMEOUT = 0x08;
@@ -176,11 +178,11 @@ void RP2GattClient::gatt_packet_handler(uint8_t type, uint16_t channel, uint8_t
     case GATT_EVENT_ALL_CHARACTERISTIC_DESCRIPTORS_QUERY_RESULT:
       con_handle = gatt_event_all_characteristic_descriptors_query_result_get_handle(packet);
       break;
-    case GATT_EVENT_CHARACTERISTIC_VALUE_QUERY_RESULT:
-      con_handle = gatt_event_characteristic_value_query_result_get_handle(packet);
+    case GATT_EVENT_LONG_CHARACTERISTIC_VALUE_QUERY_RESULT:
+      con_handle = gatt_event_long_characteristic_value_query_result_get_handle(packet);
       break;
-    case GATT_EVENT_CHARACTERISTIC_DESCRIPTOR_QUERY_RESULT:
-      con_handle = gatt_event_characteristic_descriptor_query_result_get_handle(packet);
+    case GATT_EVENT_LONG_CHARACTERISTIC_DESCRIPTOR_QUERY_RESULT:
+      con_handle = gatt_event_long_characteristic_descriptor_query_result_get_handle(packet);
       break;
     case GATT_EVENT_NOTIFICATION:
       con_handle = gatt_event_notification_get_handle(packet);
@@ -263,24 +265,17 @@ void RP2GattClient::handle_gatt_event_irq_(uint8_t event_type, const uint8_t *pa
       this->desc_count_++;
       break;
     }
-    case GATT_EVENT_CHARACTERISTIC_VALUE_QUERY_RESULT: {
-      uint16_t len = gatt_event_characteristic_value_query_result_get_value_length(packet);
-      if (len > RP2_GATT_MAX_ATTR_LEN) {
-        len = RP2_GATT_MAX_ATTR_LEN;
-      }
-      memcpy(this->op_buffer_, gatt_event_characteristic_value_query_result_get_value(packet), len);
-      this->op_len_ = len;
+    case GATT_EVENT_LONG_CHARACTERISTIC_VALUE_QUERY_RESULT:
+      // One blob per event at the reported offset; assemble into the op buffer.
+      this->assemble_blob_irq_(gatt_event_long_characteristic_value_query_result_get_value_offset(packet),
+                               gatt_event_long_characteristic_value_query_result_get_value(packet),
+                               gatt_event_long_characteristic_value_query_result_get_value_length(packet));
       break;
-    }
-    case GATT_EVENT_CHARACTERISTIC_DESCRIPTOR_QUERY_RESULT: {
-      uint16_t len = gatt_event_characteristic_descriptor_query_result_get_descriptor_length(packet);
-      if (len > RP2_GATT_MAX_ATTR_LEN) {
-        len = RP2_GATT_MAX_ATTR_LEN;
-      }
-      memcpy(this->op_buffer_, gatt_event_characteristic_descriptor_query_result_get_descriptor(packet), len);
-      this->op_len_ = len;
+    case GATT_EVENT_LONG_CHARACTERISTIC_DESCRIPTOR_QUERY_RESULT:
+      this->assemble_blob_irq_(gatt_event_long_characteristic_descriptor_query_result_get_descriptor_offset(packet),
+                               gatt_event_long_characteristic_descriptor_query_result_get_descriptor(packet),
+                               gatt_event_long_characteristic_descriptor_query_result_get_descriptor_length(packet));
       break;
-    }
     case GATT_EVENT_NOTIFICATION:
       this->enqueue_notify_irq_(gatt_event_notification_get_value_handle(packet),
                                 gatt_event_notification_get_value(packet),
@@ -297,28 +292,45 @@ void RP2GattClient::handle_gatt_event_irq_(uint8_t event_type, const uint8_t *pa
 }
 
 // NOLINTBEGIN(clang-analyzer-unix.Malloc)
+void RP2GattClient::assemble_blob_irq_(uint16_t offset, const uint8_t *data, uint16_t len) {
+  if (offset >= RP2_GATT_MAX_ATTR_LEN) {
+    return;
+  }
+  if (len > RP2_GATT_MAX_ATTR_LEN - offset) {
+    len = RP2_GATT_MAX_ATTR_LEN - offset;
+  }
+  memcpy(this->op_buffer_ + offset, data, len);
+  if (offset + len > this->op_len_) {
+    this->op_len_ = offset + len;
+  }
+}
+
 void RP2GattClient::enqueue_event_irq_(RP2GattEvent::Type type, uint8_t status, uint16_t value) {
   RP2GattEvent *event = this->event_pool_.allocate();
   if (event == nullptr) {
     this->event_queue_.increment_dropped_count();
+    this->enable_loop_soon_any_context();
     return;
   }
   event->type = type;
   event->status = status;
   event->value = value;
   this->event_queue_.push(event);
+  this->enable_loop_soon_any_context();
 }
 
 void RP2GattClient::enqueue_notify_irq_(uint16_t handle, const uint8_t *data, uint16_t len) {
   RP2GattNotifyEvent *event = this->notify_pool_.allocate();
   if (event == nullptr) {
     this->notify_queue_.increment_dropped_count();
+    this->enable_loop_soon_any_context();
     return;
   }
   event->handle = handle;
   event->len = len > RP2_GATT_MAX_ATTR_LEN ? RP2_GATT_MAX_ATTR_LEN : len;
   memcpy(event->data, data, event->len);
   this->notify_queue_.push(event);
+  this->enable_loop_soon_any_context();
 }
 // NOLINTEND(clang-analyzer-unix.Malloc)
 
@@ -384,7 +396,27 @@ void RP2GattClient::loop() {
       ESP_LOGW(TAG, "Disconnect timeout, forcing idle");
       this->handle_disconnected_(HCI_REASON_CONNECTION_TIMEOUT);
     }
-  } else if (this->state_ == EngineState::IDLE) {
+  } else if (this->state_ == EngineState::READY && this->op_type_ == OpType::WRITE_CHAR_NO_RSP &&
+             millis() - this->write_no_rsp_started_ > WRITE_NO_RSP_TIMEOUT_MS) {
+    // The can-send window never opened; report instead of hanging the op slot.
+    bool timed_out = false;
+    {
+      BluetoothLock lock;
+      // The trampoline may have just sent it; its queued result wins.
+      if (this->event_queue_.empty()) {
+        this->op_type_ = OpType::NONE;
+        timed_out = true;
+      }
+    }
+    if (timed_out) {
+      ESP_LOGW(TAG, "Deferred write timeout, handle=0x%04x", this->op_handle_);
+      if (this->listener_ != nullptr) {
+        this->listener_->on_write_result(this->op_handle_, GATT_CLIENT_BUSY);
+      }
+    }
+  } else if (this->state_ == EngineState::IDLE || (this->state_ == EngineState::READY && !this->op_in_flight_() &&
+                                                   this->event_queue_.empty() && this->notify_queue_.empty())) {
+    // Nothing pending: the enqueue path re-arms the loop from any context.
     this->disable_loop();
   }
 }
@@ -412,6 +444,35 @@ void RP2GattClient::handle_event_(const RP2GattEvent &event) {
     case RP2GattEvent::QUERY_COMPLETE:
       this->handle_query_complete_(event.status);
       break;
+    case RP2GattEvent::WRITE_NO_RSP_DONE:
+      this->finish_write_no_rsp_(event.status);
+      break;
+  }
+}
+
+void RP2GattClient::can_write_no_rsp_trampoline(void *context) {
+  // BTstack context: this callback IS the can-send window, so the deferred
+  // write happens here; only the result is enqueued for the main loop.
+  auto *self = static_cast<RP2GattClient *>(context);
+  if (self->op_type_ != OpType::WRITE_CHAR_NO_RSP) {
+    return;
+  }
+  uint8_t status = gatt_client_write_value_of_characteristic_without_response(self->con_handle_, self->op_handle_,
+                                                                              self->op_len_, self->op_buffer_);
+  if ((status == GATT_CLIENT_BUSY || status == BTSTACK_ACL_BUFFERS_FULL) &&
+      gatt_client_request_to_write_without_response(&self->can_write_registration_, self->con_handle_) == 0) {
+    return;  // next window retries; a failed re-arm falls through as an error
+  }
+  self->enqueue_event_irq_(RP2GattEvent::WRITE_NO_RSP_DONE, status, 0);
+}
+
+void RP2GattClient::finish_write_no_rsp_(uint8_t status) {
+  if (this->op_type_ != OpType::WRITE_CHAR_NO_RSP) {
+    return;
+  }
+  this->op_type_ = OpType::NONE;
+  if (this->listener_ != nullptr) {
+    this->listener_->on_write_result(this->op_handle_, status);
   }
 }
 
@@ -514,7 +575,7 @@ void RP2GattClient::handle_query_complete_(uint8_t att_status) {
   // this BTstack emits no QUERY_COMPLETE (the MTU state machine is separate
   // from the query state machine). Completions with nothing in flight are
   // dropped below.
-  if (this->op_type_ != OpType::NONE) {
+  if (this->op_type_ != OpType::NONE && this->op_type_ != OpType::WRITE_CHAR_NO_RSP) {
     OpType op = this->op_type_;
     this->op_type_ = OpType::NONE;
     if (this->listener_ == nullptr) {
@@ -523,6 +584,13 @@ void RP2GattClient::handle_query_complete_(uint8_t att_status) {
     switch (op) {
       case OpType::READ_CHAR:
       case OpType::READ_DESC:
+        // A value that is an exact multiple of MTU - 1 ends with a trailing
+        // blob request some peers refuse with INVALID_OFFSET; the read is
+        // complete, not failed.
+        if ((att_status == ATT_ERROR_INVALID_OFFSET || att_status == ATT_ERROR_ATTRIBUTE_NOT_LONG) &&
+            this->op_len_ > 0) {
+          att_status = 0;
+        }
         this->listener_->on_read_result(this->op_handle_, this->op_buffer_, att_status == 0 ? this->op_len_ : 0,
                                         att_status);
         break;
@@ -822,8 +890,9 @@ int RP2GattClient::read_characteristic(uint16_t handle) {
   this->op_handle_ = handle;
   this->op_len_ = 0;
   BluetoothLock lock;
-  uint8_t status = gatt_client_read_value_of_characteristic_using_value_handle(&RP2GattClient::gatt_packet_handler,
-                                                                               this->con_handle_, handle);
+  // Long variant: plain read first, blob continuations only past MTU - 1.
+  uint8_t status = gatt_client_read_long_value_of_characteristic_using_value_handle(&RP2GattClient::gatt_packet_handler,
+                                                                                    this->con_handle_, handle);
   if (status != 0) {
     this->op_type_ = OpType::NONE;
     return status;
@@ -845,8 +914,38 @@ int RP2GattClient::write_characteristic(uint16_t handle, const uint8_t *data, ui
     uint8_t status;
     {
       BluetoothLock lock;
+      if (this->op_type_ == OpType::WRITE_CHAR_NO_RSP) {
+        // A deferred write is parked; sending now would overtake it.
+        return GATT_CLIENT_BUSY;
+      }
       status = gatt_client_write_value_of_characteristic_without_response(this->con_handle_, handle, len,
                                                                           const_cast<uint8_t *>(data));
+      // BTSTACK_ACL_BUFFERS_FULL is the same transient flow control one layer
+      // down (L2CAP), so it defers identically.
+      if (status == GATT_CLIENT_BUSY || status == BTSTACK_ACL_BUFFERS_FULL) {
+        if (this->op_in_flight_()) {
+          // The op buffer is owned; bounce the busy to the caller as before.
+          return status;
+        }
+        // Stash the payload and send from the can-send callback.
+        memcpy(this->op_buffer_, data, len);
+        this->op_type_ = OpType::WRITE_CHAR_NO_RSP;
+        this->op_handle_ = handle;
+        this->op_len_ = len;
+        this->write_no_rsp_started_ = millis();
+        this->can_write_registration_.callback = &RP2GattClient::can_write_no_rsp_trampoline;
+        this->can_write_registration_.context = this;
+        uint8_t req = gatt_client_request_to_write_without_response(&this->can_write_registration_, this->con_handle_);
+        if (req != 0 && req != ERROR_CODE_COMMAND_DISALLOWED) {
+          this->op_type_ = OpType::NONE;
+          return req;
+        }
+        // COMMAND_DISALLOWED = still armed from a timed-out deferral; that
+        // registration sends the newly parked payload. Keep the loop running
+        // so the deadline below can fire on a stalled link.
+        this->enable_loop();
+        return 0;
+      }
     }
     if (status == 0 && this->listener_ != nullptr) {
       this->listener_->on_write_result(handle, 0);
@@ -888,7 +987,7 @@ int RP2GattClient::read_descriptor(uint16_t handle) {
   this->op_handle_ = handle;
   this->op_len_ = 0;
   BluetoothLock lock;
-  uint8_t status = gatt_client_read_characteristic_descriptor_using_descriptor_handle(
+  uint8_t status = gatt_client_read_long_characteristic_descriptor_using_descriptor_handle(
       &RP2GattClient::gatt_packet_handler, this->con_handle_, handle);
   if (status != 0) {
     this->op_type_ = OpType::NONE;

esphome/components/bluetooth_connection/bluetooth_connection_rp2.h

@@ -46,10 +46,11 @@ static constexpr uint16_t RP2_GATT_MAX_ATTR_LEN = 512;
 // Control events from the BTstack handlers to loop().
 struct RP2GattEvent {
   enum Type : uint8_t {
-    CONNECTED,       // status + con_handle (value)
-    DISCONNECTED,    // status = HCI reason
-    MTU_EXCHANGED,   // value = negotiated MTU
-    QUERY_COMPLETE,  // status = ATT status of the finished query
+    CONNECTED,          // status + con_handle (value)
+    DISCONNECTED,       // status = HCI reason
+    MTU_EXCHANGED,      // value = negotiated MTU
+    QUERY_COMPLETE,     // status = ATT status of the finished query
+    WRITE_NO_RSP_DONE,  // status = result of the deferred write
   };
   Type type;
   uint8_t status;
@@ -106,7 +107,7 @@ class RP2GattClient final : public Component,
 
   enum class DiscoveryPhase : uint8_t { NONE, SERVICES, CHARACTERISTICS, DESCRIPTORS };
 
-  enum class OpType : uint8_t { NONE, READ_CHAR, WRITE_CHAR, READ_DESC, WRITE_DESC };
+  enum class OpType : uint8_t { NONE, READ_CHAR, WRITE_CHAR, WRITE_CHAR_NO_RSP, READ_DESC, WRITE_DESC };
 
   // The whole table in one transient allocation (RAMAllocator, checked),
   // freed after streaming.
@@ -124,6 +125,7 @@ class RP2GattClient final : public Component,
   void handle_gatt_event_irq_(uint8_t event_type, const uint8_t *packet);
   void enqueue_event_irq_(RP2GattEvent::Type type, uint8_t status, uint16_t value);
   void enqueue_notify_irq_(uint16_t handle, const uint8_t *data, uint16_t len);
+  void assemble_blob_irq_(uint16_t offset, const uint8_t *data, uint16_t len);
 
   // Main-loop state machine.
   void handle_event_(const RP2GattEvent &event);
@@ -137,6 +139,8 @@ class RP2GattClient final : public Component,
   void fail_connection_(uint8_t reason);
   void cleanup_link_state_();
   bool notify_subscribed_(uint16_t handle) const;
+  static void can_write_no_rsp_trampoline(void *context);
+  void finish_write_no_rsp_(uint8_t status);
   void release_scan_inhibit_();
   bool op_in_flight_() const {
     return this->op_type_ != OpType::NONE || this->discovery_phase_ != DiscoveryPhase::NONE;
@@ -156,10 +160,12 @@ class RP2GattClient final : public Component,
 
   // BTstack registrations
   gatt_client_notification_t notification_registration_{};
+  btstack_context_callback_registration_t can_write_registration_{};
 
   // Group 3: 4-byte types
   uint32_t connect_started_{0};
   uint32_t disconnecting_started_{0};
+  uint32_t write_no_rsp_started_{0};
 
   // Group 4: 2-byte types (table counters written from the handler during
   // discovery, read from the main loop after the phase's QUERY_COMPLETE)

esphome/components/bluetooth_proxy/bluetooth_proxy.cpp

@@ -380,6 +380,9 @@ void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest
         } else {
           this->send_device_pairing(msg.address, true);
         }
+      } else {
+        // Answer instead of leaving the client to time out.
+        this->send_device_pairing(msg.address, false, GATT_NOT_CONNECTED);
       }
 #else
       // Explicit pairing is not offered (FEATURE_PAIRING is not advertised);