github/esphome
2
0
Fork 0
mirror of https://github.com/esphome/esphome.git synced 2026-08-24 07:06:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

Apply review feedback: owner-matched release, retry throttle, wrap link guards, slot-tagged logs

Browse Source
This commit is contained in:
J. Nick Koston
2026-08-10 15:31:58 -05:00
parent d5497f3c43
commit 4f98bc4baf
3 changed files with 64 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp
+36 -23
View File
@@ -25,6 +25,9 @@ using ble_device_base::GATT_ERR_NO_MEMORY;
// and keeps the scan inhibited, so the engine cancels after 20 s. The
// disconnect timeout mirrors the esp32 CLOSE_EVT safety net.
static constexpr uint32_t CONNECT_TIMEOUT_MS = 20000;
// Pending engines re-attempt gap_connect on this cadence instead of every
// loop pass: the DISALLOWED path (teardown overlap) takes BluetoothLock.
static constexpr uint32_t CONNECT_RETRY_INTERVAL_MS = 50;
// Can-send windows normally open within a connection interval (tens of ms).
static constexpr uint32_t WRITE_NO_RSP_TIMEOUT_MS = 500;
@@ -84,6 +87,7 @@ void RP2GattClient::setup() {
// One locked section: the slot store lands before the count bump, and a
// live HCI handler (N > 1 builds) cannot read a half-written registry.
BluetoothLock lock;
this->engine_index_ = instance_count;
instances[instance_count] = this;
instance_count++;
// One HCI event handler for all engine instances (BTstack supports
@@ -129,8 +133,13 @@ void RP2GattClient::hci_packet_handler(uint8_t type, uint16_t channel, uint8_t *
uint8_t status = gap_subevent_le_connection_complete_get_status(packet);
hci_con_handle_t con_handle = gap_subevent_le_connection_complete_get_connection_handle(packet);
// The stack-wide create-connection resolved either way (success, failure
// or cancel completion); a pending engine's loop() may issue the next.
connect_owner = nullptr;
// or cancel completion; all carry the peer address). Matched against the
// owner so a stale late completion cannot strip a live owner that has
// since re-issued for another peer — that would skip its cancel path and
// leave the stack initiating forever.
if (connect_owner != nullptr && memcmp(connect_owner->peer_addr_, peer, sizeof(bd_addr_t)) == 0) {
connect_owner = nullptr;
}
// Route to the engine that is waiting for this peer.
for (uint8_t i = 0; i < instance_count; i++) {
RP2GattClient *inst = instances[i];
@@ -392,26 +401,30 @@ void RP2GattClient::loop() {
if (dropped > 0) {
// Control events must not be lost; the connection state is no longer
// trustworthy — recover with a forced teardown.
ESP_LOGE(TAG, "Dropped %u GATT control events, disconnecting", dropped);
ESP_LOGE(TAG, "[%u] Dropped %u GATT control events, disconnecting", this->engine_index_, dropped);
this->gatt_disconnect();
}
uint16_t notify_dropped = this->notify_queue_.get_and_reset_dropped_count();
if (notify_dropped > 0) {
ESP_LOGW(TAG, "Dropped %u GATT notifications (queue full)", notify_dropped);
ESP_LOGW(TAG, "[%u] Dropped %u GATT notifications (queue full)", this->engine_index_, notify_dropped);
}
if (this->state_ == EngineState::CONNECT_PENDING) {
if (millis() - this->connect_started_ > CONNECT_TIMEOUT_MS) {
uint32_t now = millis();
if (now - this->connect_started_ > CONNECT_TIMEOUT_MS) {
// Never reached the radio; nothing stack-side to cancel.
ESP_LOGW(TAG, "Connect timeout (queued)");
ESP_LOGW(TAG, "[%u] Connect timeout (queued)", this->engine_index_);
this->fail_connection_(HCI_REASON_CONNECTION_TIMEOUT);
} else if (int err = this->try_gap_connect_(); err != 0) {
this->fail_connection_(static_cast<uint8_t>(err));
} else if (now - this->connect_retry_ms_ >= CONNECT_RETRY_INTERVAL_MS) {
this->connect_retry_ms_ = now;
if (int err = this->try_gap_connect_(); err != 0) {
this->fail_connection_(static_cast<uint8_t>(err));
}
}
} else if (this->state_ == EngineState::CONNECTING || this->state_ == EngineState::MTU_EXCHANGE) {
uint32_t now = millis();
if (now - this->connect_started_ > CONNECT_TIMEOUT_MS) {
ESP_LOGW(TAG, "Connect timeout");
ESP_LOGW(TAG, "[%u] Connect timeout", this->engine_index_);
if (this->state_ == EngineState::CONNECTING && this->con_handle_ == HCI_CON_HANDLE_INVALID) {
if (!this->connect_cancel_attempted_) {
this->connect_cancel_attempted_ = true;
@@ -442,7 +455,7 @@ void RP2GattClient::loop() {
}
} else if (this->state_ == EngineState::DISCONNECTING) {
if (millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) {
ESP_LOGW(TAG, "Disconnect timeout, forcing idle");
ESP_LOGW(TAG, "[%u] Disconnect timeout, forcing idle", this->engine_index_);
this->handle_disconnected_(HCI_REASON_CONNECTION_TIMEOUT);
}
} else if (this->state_ == EngineState::READY && this->op_type_ == OpType::WRITE_CHAR_NO_RSP &&
@@ -458,7 +471,7 @@ void RP2GattClient::loop() {
}
}
if (timed_out) {
ESP_LOGW(TAG, "Deferred write timeout, handle=0x%04x", this->op_handle_);
ESP_LOGW(TAG, "[%u] Deferred write timeout, handle=0x%04x", this->engine_index_, this->op_handle_);
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_() &&
@@ -479,7 +492,7 @@ void RP2GattClient::handle_event_(const RP2GattEvent &event) {
case RP2GattEvent::MTU_EXCHANGED:
if (this->state_ == EngineState::MTU_EXCHANGE) {
this->mtu_ = event.value;
ESP_LOGD(TAG, "MTU %u", this->mtu_);
ESP_LOGD(TAG, "[%u] MTU %u", this->engine_index_, this->mtu_);
this->state_ = EngineState::READY;
// Scanning resumes and runs alongside the established connection.
this->release_scan_inhibit_();
@@ -527,7 +540,7 @@ void RP2GattClient::handle_connected_(uint8_t status, uint16_t con_handle) {
return;
}
if (status != 0) {
ESP_LOGW(TAG, "Connect failed, status=0x%02x", status);
ESP_LOGW(TAG, "[%u] Connect failed, status=0x%02x", this->engine_index_, status);
this->fail_connection_(status);
return;
}
@@ -551,7 +564,7 @@ void RP2GattClient::handle_connected_(uint8_t status, uint16_t con_handle) {
}
this->con_handle_ = con_handle;
this->state_ = EngineState::MTU_EXCHANGE;
ESP_LOGD(TAG, "Link up, handle=0x%04x, negotiating MTU", con_handle);
ESP_LOGD(TAG, "[%u] Link up, handle=0x%04x, negotiating MTU", this->engine_index_, con_handle);
BluetoothLock lock;
// One wildcard listener covers notifications/indications for every
// characteristic on this connection; the CCCD writes come from the API
@@ -598,9 +611,9 @@ void RP2GattClient::cleanup_link_state_() {
while ((stale = this->notify_queue_.pop()) != nullptr) {
this->notify_pool_.release(stale);
}
// The wildcard listener is registered on the normal connect path right
// after con_handle_ is recorded; the cancel branch tears down before
// registering, where stop_listening on an unregistered entry is a no-op.
// con_handle_ may be stamped in the BTstack context before the main loop
// registers the listener, so a valid handle does not imply a registration;
// stop_listening on an unregistered entry is a benign no-op.
if (this->con_handle_ != HCI_CON_HANDLE_INVALID) {
BluetoothLock lock;
gatt_client_stop_listening_for_characteristic_value_updates(&this->notification_registration_);
@@ -617,7 +630,7 @@ void RP2GattClient::handle_disconnected_(uint8_t reason) {
if (this->state_ == EngineState::IDLE) {
return;
}
ESP_LOGD(TAG, "Disconnected, reason=0x%02x", reason);
ESP_LOGD(TAG, "[%u] Disconnected, reason=0x%02x", this->engine_index_, reason);
this->fail_connection_(reason);
}
@@ -675,7 +688,7 @@ int RP2GattClient::discover_services() {
RAMAllocator<ServiceArena> allocator(RAMAllocator<ServiceArena>::ALLOC_INTERNAL);
this->arena_ = allocator.allocate(1);
if (this->arena_ == nullptr) {
ESP_LOGE(TAG, "Service table allocation failed");
ESP_LOGE(TAG, "[%u] Service table allocation failed", this->engine_index_);
return ble_device_base::GATT_ERR_NO_MEMORY;
}
new (this->arena_) ServiceArena();
@@ -781,8 +794,8 @@ void RP2GattClient::advance_discovery_(uint8_t att_status) {
void RP2GattClient::finish_discovery_(int error) {
this->discovery_phase_ = DiscoveryPhase::NONE;
ESP_LOGD(TAG, "Discovery done (err=%d): %u services, %u characteristics, %u descriptors", error, this->service_count_,
this->char_count_, this->desc_count_);
ESP_LOGD(TAG, "[%u] Discovery done (err=%d): %u services, %u characteristics, %u descriptors", this->engine_index_,
error, this->service_count_, this->char_count_, this->desc_count_);
if (error == 0 && this->truncated_) {
// A partial table must not stream: V3 clients cache the database
// permanently, so an incomplete one would be wrong forever.
@@ -907,7 +920,7 @@ int RP2GattClient::try_gap_connect_() {
this->state_ = EngineState::CONNECT_PENDING;
return 0;
}
ESP_LOGW(TAG, "gap_connect failed, status=0x%02x", status);
ESP_LOGW(TAG, "[%u] gap_connect failed, status=0x%02x", this->engine_index_, status);
return status;
}
@@ -948,7 +961,7 @@ int RP2GattClient::gatt_disconnect() {
BluetoothLock lock;
status = gap_disconnect(this->con_handle_);
if (status != 0) {
ESP_LOGW(TAG, "gap_disconnect failed, status=0x%02x", status);
ESP_LOGW(TAG, "[%u] gap_disconnect failed, status=0x%02x", this->engine_index_, status);
}
}
if (status != 0) {
esphome/components/bluetooth_connection/bluetooth_connection_rp2.h
+3 -1
View File
@@ -173,6 +173,7 @@ class RP2GattClient final : public Component, public Parented<rp2040_ble::RP2040
// Group 3: 4-byte types
uint32_t connect_started_{0};
uint32_t connect_retry_ms_{0}; // last CONNECT_PENDING gap_connect attempt
uint32_t disconnecting_started_{0};
uint32_t write_no_rsp_started_{0};
@@ -193,7 +194,8 @@ class RP2GattClient final : public Component, public Parented<rp2040_ble::RP2040
// listener's deliveries on this list (esp32 parity for enable=false).
std::array<uint16_t, RP2_GATT_MAX_NOTIFY_SUBSCRIPTIONS> notify_subscriptions_{};
uint8_t notify_subscription_count_{0};
bd_addr_t peer_addr_{}; // MSB-first, as gap_connect expects
uint8_t engine_index_{0}; // position in instances[]; tags log lines per slot
bd_addr_t peer_addr_{}; // MSB-first, as gap_connect expects
bd_addr_type_t peer_addr_type_{BD_ADDR_TYPE_LE_PUBLIC};
EngineState state_{EngineState::IDLE};
DiscoveryPhase discovery_phase_{DiscoveryPhase::NONE};
esphome/components/bluetooth_connection/btstack_memory_rp2.cpp
+25
View File
@@ -19,6 +19,20 @@
namespace esphome::bluetooth_connection {
namespace {
// Pinned against arduino-pico 6.0.0's prebuilt archives: a framework bump (or
// a changed ENABLE_* macro) shifting the struct layout must fail the build
// here, not overrun the pool blocks at runtime. Sizes differ per core
// architecture (measured from each archive's own storage symbols). GCC only:
// the clang-tidy frontend lays these structs out differently, and the guard
// targets the real link.
#ifndef __clang__
#ifdef __riscv
static_assert(sizeof(gatt_client_t) == 140 && sizeof(hci_connection_t) == 3740, "BTstack layout changed");
#else
static_assert(sizeof(gatt_client_t) == 128 && sizeof(hci_connection_t) == 3688, "BTstack layout changed");
#endif
#endif // __clang__
// One gatt_client_t per configured connection slot. An hci_connection_t is
// held from gap_connect() to DISCONNECTION_COMPLETE (scanning holds none);
// +1 mirrors the prebuilt library's own headroom (2 connections for 1 GATT
@@ -48,6 +62,17 @@ struct PoolInit {
// Exact semantics of btstack_memory.c's static-pool arm: zeroed block on
// success, NULL when exhausted; free returns the block to the pool.
// NOLINTBEGIN(bugprone-reserved-identifier,cert-dcl37-c,cert-dcl51-cpp,readability-identifier-naming)
extern "C" gatt_client_t *__real_btstack_memory_gatt_client_get(void);
namespace {
// Never called: the linker silently ignores --wrap for an unreferenced
// symbol, and __real_* only resolves while --wrap is in effect — so this
// reference turns "pools compiled but flags not emitted" into a link error
// instead of a silent fallback to the prebuilt one-client pool.
// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
[[gnu::used]] gatt_client_t *(*const WRAP_ACTIVE_CHECK)() = &__real_btstack_memory_gatt_client_get;
} // namespace
extern "C" {
gatt_client_t *__wrap_btstack_memory_gatt_client_get(void) {