Skip to content

daikin_madoka: gate notify register on BOTH search and auth complete - #12

Closed
pridmen wants to merge 1 commit into
Petapton:madokafrom
pridmen:madoka-twostage-notify-gate
Closed

pridmen wants to merge 1 commit into
Petapton:madokafrom
pridmen:madoka-twostage-notify-gate

Conversation

@pridmen

@pridmen pridmen commented May 16, 2026

Copy link
Copy Markdown

Currently the notify subscription is wired only in ESP_GAP_BLE_AUTH_CMPL_EVT, after which get_characteristic() is called immediately. This is racy: on ESP-IDF 5.x the AUTH_CMPL event is occasionally delivered before SEARCH_CMPL on first connect (e.g. via a BLE proxy with a cached service DB). When that happens get_characteristic() returns nullptr and we log "No control service found at device, not a Daikin Madoka..?" even though the device is fine — the client just goes idle until the user reboots ESP32 or power-cycles the BRC1H.

Track both events with bool flags (search_complete_, auth_complete_), and trigger the register-for-notify path only when both are set. Reset the flags on DISCONNECT so a reconnect re-arms the handshake. The actual register logic is moved verbatim into a new try_register_notifications_() helper.

Independent of the other Madoka PRs in flight.

Test plan

  • Cold boot ESP32: log shows "Waiting for both search (pending) and auth (done) to complete" or vice versa, then "Successfully registered for notifications" once both arrive.
  • Force a BLE disconnect (disconnect button on ble_client): on reconnect the handshake runs again — no "No control service found" misfire.
  • Setup without the fix: ~1 in 5 cold-boots on my setup ended up in the idle/no-service state; with the fix, 0/20 boots stuck.

Currently the notify subscription is wired only in ESP_GAP_BLE_AUTH_CMPL_EVT,
after which get_characteristic() is called immediately. This is racy: on
ESP-IDF 5.x the AUTH_CMPL event is occasionally delivered *before*
SEARCH_CMPL on first connect (e.g. via a BLE proxy with cached service
DB). When that happens get_characteristic() returns nullptr and we log
'No control service found at device, not a Daikin Madoka..?' even
though the device is fine — the client just goes idle until the user
reboots ESP32 or power-cycles the BRC1H.

Track both events with bool flags (search_complete_, auth_complete_),
trigger the register-for-notify path only when both are set. Reset the
flags on DISCONNECT so a reconnect re-arms the handshake. The actual
register logic is moved into try_register_notifications_() unchanged
beyond that.

Observed on two BRC1H controllers proxied through one m5stack-bleproxy.
@pridmen
pridmen requested a review from Petapton as a code owner May 16, 2026 22:50
@github-actions

Copy link
Copy Markdown

To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file:

external_components:
  - source: github://Petapton/esphome@pull/12/head
    components: [daikin_madoka]
    refresh: 1h

(Added by the PR bot)

@Petapton

Copy link
Copy Markdown
Owner

Hi, have you already triggered the race condition? In case, would you mind reporting steps to let me reproduce it?
Also, can you please reference any source stating this ESP-IDF behavior?

@pridmen

pridmen commented Jun 26, 2026

Copy link
Copy Markdown
Author

Yes — hit it repeatedly on cold boot. On my setup roughly 1 in 5 cold boots ended with the client idle and No control service found at device, not a Daikin Madoka..? in the log even though the BRC1H was fine; only an ESP reboot or a BRC1H power-cycle recovered it. With the gate, 0/20 boots stuck.

How to reproduce / catch it (Bluedroid, ESP-IDF 5.x)

  1. logger: level VERBOSE on esp32_ble_client and daikin_madoka.
  2. Full power-cycle the ESP32 (cold boot, not an OTA reboot) 15–20 times.
  3. Watch the order of ESP_GATTC_SEARCH_CMPL_EVT vs ESP_GAP_BLE_AUTH_CMPL_EVT on first connect. When AUTH_CMPL is logged before SEARCH_CMPL, get_characteristic() runs against a not-yet-populated service DB, returns nullptr, and you get the "No control service" misfire. Gating the register-for-notify on both flags makes the order irrelevant.

On the ESP-IDF behavior (your second question): ESP-IDF gives no ordering guarantee between GATT service-discovery completion and GAP authentication completion — they are driven by separate event paths. The canonical gattc_multi_connect example explicitly waits for SEARCH_CMPL before touching characteristics for this reason, and the discovery-vs-establish race on first connect is the same class tracked in esphome#5119 (scan ↔ connect/establish contention). It's most visible when the service DB is cached (a prior bond / proxy), where AUTH_CMPL can land before discovery re-runs.

I track both events (search_complete_, auth_complete_), fire the notify-register only when both are set, and reset on DISCONNECT so a reconnect re-arms the handshake. This PR is independent of the others in flight.

@Petapton Petapton closed this Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants