Skip to content

Commit fa013a1

Browse files
DerekSeamanclaude
andcommitted
fix(threading): add 200ms timeout to BleOpGuard on ESPHome main loop callers
Prevents NimBLE task starvation when the ESPHome main loop holds ble_op_mutex_ during slow BLE stack operations. If the ESPHome loop cannot acquire the lock within 200ms it logs a warning and returns early; loop() will retry on the next iteration. NimBLE GAP callback callers (handle_gap_enc_change, poll_irk_if_due, retry_security_if_needed, handle_late_enc_timer) retain portMAX_DELAY since dropping those events would lose IRK captures. Changes: - BleOpGuard gains optional timeout parameter (default: portMAX_DELAY) - BleOpGuard::acquired() method for callers to check lock success - start_advertising(), stop_advertising(), update_ble_name(), refresh_mac() terminate, MAC rotation in loop(), and pairing timeout terminate all use pdMS_TO_TICKS(200) with early-return on failure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c8694f7 commit fa013a1

File tree

1 file changed

+45
-9
lines changed

1 file changed

+45
-9
lines changed

components/irk_capture/irk_capture.cpp

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -373,25 +373,37 @@ class MutexGuard {
373373

374374
// Serialize BLE host control operations across ESPHome and NimBLE task
375375
// contexts.
376+
//
377+
// Usage:
378+
// BleOpGuard g(mutex_); // portMAX_DELAY (NimBLE callbacks)
379+
// BleOpGuard g(mutex_, pdMS_TO_TICKS(200)); // timeout (ESPHome main loop)
380+
// if (!g.acquired()) { ESP_LOGW(...); return; } // bail on timeout
376381
class BleOpGuard {
377382
public:
378-
explicit BleOpGuard(SemaphoreHandle_t mutex) : mutex_(mutex) {
383+
explicit BleOpGuard(SemaphoreHandle_t mutex, TickType_t timeout = portMAX_DELAY)
384+
: mutex_(mutex), acquired_(false) {
379385
if (mutex_) {
380-
xSemaphoreTake(mutex_, portMAX_DELAY);
386+
acquired_ = (xSemaphoreTake(mutex_, timeout) == pdTRUE);
381387
}
382388
}
383389

384390
~BleOpGuard() {
385-
if (mutex_) {
391+
if (mutex_ && acquired_) {
386392
xSemaphoreGive(mutex_);
387393
}
388394
}
389395

396+
// Returns false if the mutex was not acquired (timed out).
397+
bool acquired() const {
398+
return acquired_;
399+
}
400+
390401
BleOpGuard(const BleOpGuard&) = delete;
391402
BleOpGuard& operator=(const BleOpGuard&) = delete;
392403

393404
private:
394405
SemaphoreHandle_t mutex_;
406+
bool acquired_;
395407
};
396408

397409
static void hr_measurement_sample(uint8_t* buf, size_t* len) {
@@ -1555,7 +1567,11 @@ void IRKCaptureComponent::loop() {
15551567
uint8_t own_addr_type = BLE_OWN_ADDR_PUBLIC;
15561568
int rc2 = 0;
15571569
{
1558-
BleOpGuard ble_lock(ble_op_mutex_);
1570+
BleOpGuard ble_lock(ble_op_mutex_, pdMS_TO_TICKS(200));
1571+
if (!ble_lock.acquired()) {
1572+
ESP_LOGW(TAG, "MAC rotation: ble_op_mutex timeout, will retry next loop()");
1573+
return;
1574+
}
15591575
rc = ble_hs_id_set_rnd(mac_copy);
15601576
if (rc == 0) {
15611577
rc2 = ble_hs_id_infer_auto(0, &own_addr_type);
@@ -1676,7 +1692,11 @@ void IRKCaptureComponent::loop() {
16761692
ble_store_util_delete_peer(&d.peer_id_addr);
16771693
int term_rc;
16781694
{
1679-
BleOpGuard ble_lock(ble_op_mutex_);
1695+
BleOpGuard ble_lock(ble_op_mutex_, pdMS_TO_TICKS(200));
1696+
if (!ble_lock.acquired()) {
1697+
ESP_LOGW(TAG, "Pairing timeout terminate: ble_op_mutex timeout, will retry next loop()");
1698+
return;
1699+
}
16801700
term_rc = ble_gap_terminate(timeout_conn_handle, BLE_ERR_REM_USER_CONN_TERM);
16811701
}
16821702
if (term_rc != 0) {
@@ -2036,7 +2056,11 @@ void IRKCaptureComponent::start_advertising() {
20362056

20372057
int rc = 0;
20382058
{
2039-
BleOpGuard ble_lock(ble_op_mutex_);
2059+
BleOpGuard ble_lock(ble_op_mutex_, pdMS_TO_TICKS(200));
2060+
if (!ble_lock.acquired()) {
2061+
ESP_LOGW(TAG, "start_advertising: ble_op_mutex timeout, will retry next loop()");
2062+
return;
2063+
}
20402064

20412065
// Defensive stop - ensure clean GAP state before starting.
20422066
rc = ble_gap_adv_stop();
@@ -2108,7 +2132,11 @@ void IRKCaptureComponent::start_advertising() {
21082132
void IRKCaptureComponent::stop_advertising() {
21092133
int rc;
21102134
{
2111-
BleOpGuard ble_lock(ble_op_mutex_);
2135+
BleOpGuard ble_lock(ble_op_mutex_, pdMS_TO_TICKS(200));
2136+
if (!ble_lock.acquired()) {
2137+
ESP_LOGW(TAG, "stop_advertising: ble_op_mutex timeout, skipping");
2138+
return;
2139+
}
21122140
rc = ble_gap_adv_stop();
21132141
}
21142142
if (rc != 0 && rc != BLE_HS_EALREADY && rc != BLE_HS_EINVAL) {
@@ -2192,7 +2220,11 @@ void IRKCaptureComponent::refresh_mac() {
21922220
ESP_LOGI(TAG, "Terminating connection for MAC rotation");
21932221
int rc;
21942222
{
2195-
BleOpGuard ble_lock(ble_op_mutex_);
2223+
BleOpGuard ble_lock(ble_op_mutex_, pdMS_TO_TICKS(200));
2224+
if (!ble_lock.acquired()) {
2225+
ESP_LOGW(TAG, "MAC rotation terminate: ble_op_mutex timeout, will retry next loop()");
2226+
return;
2227+
}
21962228
rc = ble_gap_terminate(conn_handle_copy, BLE_ERR_REM_USER_CONN_TERM);
21972229
}
21982230
if (rc != 0) {
@@ -2221,7 +2253,11 @@ void IRKCaptureComponent::update_ble_name(const std::string& name) {
22212253
// Update GAP device name
22222254
int rc;
22232255
{
2224-
BleOpGuard ble_lock(ble_op_mutex_);
2256+
BleOpGuard ble_lock(ble_op_mutex_, pdMS_TO_TICKS(200));
2257+
if (!ble_lock.acquired()) {
2258+
ESP_LOGW(TAG, "update_ble_name: ble_op_mutex timeout, name update skipped");
2259+
return;
2260+
}
22252261
rc = ble_svc_gap_device_name_set(name.c_str());
22262262
}
22272263
if (rc != 0) {

0 commit comments

Comments
 (0)