From b3fb2f21c7c570c7aeac8cf9797de313acb90ff7 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sat, 22 Aug 2026 16:27:55 +1200 Subject: [PATCH 01/12] fix(crow_alarm_panel): pace arm/disarm retries, match ARMED_STATE intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HA log 2026-08-19 17:09 shows two consecutive disarm calls exhausting all 5 back-to-back retries (12 failed attempts in ~21 s) during a controller degradation episode that outlasted the ~7 s retry envelope — pings stopped and both physical keypads re-registered ~90 s later. A manual call ~8 s after the second abort succeeded first try. - Pace retries with a growing backoff (1/2/3/5/8/13 s) and raise ARM_DISARM_MAX_RETRIES to 6, stretching one call's envelope to ~40 s; ignore KEYPAD_COMMANDs while a backoff wait is pending so a stray periodic command can't advance the dead sequence. - Resolve on ARMED_STATE broadcasts from any non-IDLE state, but only when they match the request's intent (Disarmed for disarm, Arming or Armed Away for arm): a late Disarmed mid-retry could otherwise let the retry re-arm the panel via code+ENTER-while-disarmed, and a registration re-announce of Armed Away could falsely complete a disarm. Compiles against crow_alarm_panel_test.yaml; not yet hardware-validated. Co-Authored-By: Claude Fable 5 --- .../crow_alarm_panel/crow_alarm_panel.cpp | 189 +++++++++++------- .../crow_alarm_panel/crow_alarm_panel.h | 21 +- .../docs/arm_disarm_state_machine.md | 87 +++++++- 3 files changed, 215 insertions(+), 82 deletions(-) diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index e059ee6..8ff3237 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -458,16 +458,39 @@ void CrowAlarmPanel::loop() { format_hex_pretty(data).c_str()); } // ARMED_STATE is the controller's own authoritative state broadcast, independent of the - // arm/disarm state machine below — this is the only signal CODE_ENTER_PENDING treats as - // proof of success (see the enum comment in crow_alarm_panel.h for why the KEYPAD_COMMAND - // byte[1] value below isn't trusted for this). Any recognized ARMED_STATE broadcast while - // a terminal key is outstanding settles it, regardless of which of the three it is — - // "Arming" confirms an arm-with-code request as much as "Armed Away"/"Disarmed" would. - if (this->arm_disarm_state_ == ArmDisarmState::CODE_ENTER_PENDING && - ((data[0] == 0x00 && data[1] == 0x01) || (data[0] == 0x01 && data[1] == 0x00) || - (data[0] == 0x00 && data[1] == 0x00))) { - ESP_LOGD(TAG, "Code sequence: complete (confirmed via ARMED_STATE broadcast)"); - this->arm_disarm_state_ = ArmDisarmState::IDLE; + // arm/disarm state machine — this is the only signal treated as proof of success (see the + // enum comment in crow_alarm_panel.h for why KEYPAD_COMMAND byte[1] isn't trusted for + // this). Resolution is intent-matched and covers every non-IDLE state, including a retry + // backoff wait, for two reasons (see arm_disarm_state_machine.md, 2026-08-22 entry): + // - A slow controller can broadcast the outcome of a previous attempt after the watchdog + // already restarted the sequence (an 825 ms ack has been observed, right at the 1 s + // boundary). Without resolving from CODE_DIGIT_PENDING, the retry would keep typing + // the remaining digits + ENTER into an already-disarmed panel — and code+ENTER while + // disarmed is exactly the "arm with code" gesture, so the retry could re-arm it. + // - The registration handshake re-broadcasts the CURRENT state (logs-46: repeated "Armed + // Away" lines while armed). Accepting any recognized broadcast as success would let an + // "Armed Away" re-announce falsely complete a disarm, so only the broadcast matching + // the request's intent resolves it; non-matching ones are ignored. + if (this->arm_disarm_state_ != ArmDisarmState::IDLE) { + const bool is_arming = (data[0] == 0x00 && data[1] == 0x01); + const bool is_armed_away = (data[0] == 0x01 && data[1] == 0x00); + const bool is_disarmed = (data[0] == 0x00 && data[1] == 0x00); + if (is_arming || is_armed_away || is_disarmed) { + const bool is_disarm_request = (this->arm_disarm_state_ == ArmDisarmState::CODE_DIGIT_PENDING || + this->arm_disarm_state_ == ArmDisarmState::CODE_ENTER_PENDING) && + this->arm_disarm_terminal_key_ == KEY_ENTER; + const bool matches_intent = is_disarm_request ? is_disarmed : (is_arming || is_armed_away); + if (matches_intent) { + ESP_LOGD(TAG, "Code sequence: complete (confirmed via ARMED_STATE broadcast)"); + this->arm_disarm_state_ = ArmDisarmState::IDLE; + this->arm_disarm_code_digits_.clear(); + this->arm_disarm_code_idx_ = 0; + this->arm_disarm_retry_count_ = 0; + this->arm_disarm_retry_pending_ = false; + } else { + ESP_LOGD(TAG, "Arm/disarm: ignoring ARMED_STATE broadcast not matching request intent"); + } + } } break; } @@ -632,62 +655,67 @@ void CrowAlarmPanel::loop() { default: break; } - // Drive arm/disarm state machine forward. - switch (this->arm_disarm_state_) { - case ArmDisarmState::ARM_AWAY_PENDING: - case ArmDisarmState::ARM_STAY_PENDING: - ESP_LOGD(TAG, "Arm/stay: CMD received, sequence complete"); - this->arm_disarm_state_ = ArmDisarmState::IDLE; - break; - case ArmDisarmState::CODE_DIGIT_PENDING: { - uint8_t cmd_byte = (data.size() > 1) ? data[1] : 0; - if (!this->arm_disarm_digit_ack_byte_set_) { - // Learn this sequence's "digit accepted, more expected" display_code from the - // first digit's response, for diagnostics only. Trace: logs-7/logs-33 (2026-07-08) - // show address 0x05 consistently getting 0x07 here — never 0x01 — across four - // separate arm and disarm attempts with a code independently confirmed correct via - // the physical IP keypad (0x07), while address 0x07 consistently got 0x01. The - // controller does not validate code correctness per digit (only at ENTER), so this - // is a per-keypad-type display quirk, not a rejection. A mid-sequence change isn't - // treated as an abort-worthy anomaly either (see docs/arm_disarm_state_machine.md): - // its meaning was never established, and logs-18 showed the abort itself causing a - // stuck alarm_control_panel entity for a sequence that may well have succeeded. - // Success/failure comes solely from the ARMED_STATE broadcast or the shared 1s - // watchdog, same as CODE_ENTER_PENDING. - this->arm_disarm_digit_ack_byte_ = cmd_byte; - this->arm_disarm_digit_ack_byte_set_ = true; - } else if (cmd_byte != this->arm_disarm_digit_ack_byte_) { - ESP_LOGD(TAG, "Arm/disarm: CMD byte changed from 0x%02X to 0x%02X in CODE_DIGIT_PENDING, continuing", - this->arm_disarm_digit_ack_byte_, cmd_byte); + // Drive arm/disarm state machine forward. Skipped while a retry backoff is pending: + // the timed-out attempt is dead and will restart from the first digit, so a stray + // periodic KEYPAD_COMMAND arriving mid-wait must not advance the stale sequence + // (it would type leftover digits into the panel outside any attempt). + if (!this->arm_disarm_retry_pending_) { + switch (this->arm_disarm_state_) { + case ArmDisarmState::ARM_AWAY_PENDING: + case ArmDisarmState::ARM_STAY_PENDING: + ESP_LOGD(TAG, "Arm/stay: CMD received, sequence complete"); + this->arm_disarm_state_ = ArmDisarmState::IDLE; + break; + case ArmDisarmState::CODE_DIGIT_PENDING: { + uint8_t cmd_byte = (data.size() > 1) ? data[1] : 0; + if (!this->arm_disarm_digit_ack_byte_set_) { + // Learn this sequence's "digit accepted, more expected" display_code from the + // first digit's response, for diagnostics only. Trace: logs-7/logs-33 (2026-07-08) + // show address 0x05 consistently getting 0x07 here — never 0x01 — across four + // separate arm and disarm attempts with a code independently confirmed correct via + // the physical IP keypad (0x07), while address 0x07 consistently got 0x01. The + // controller does not validate code correctness per digit (only at ENTER), so this + // is a per-keypad-type display quirk, not a rejection. A mid-sequence change isn't + // treated as an abort-worthy anomaly either (see docs/arm_disarm_state_machine.md): + // its meaning was never established, and logs-18 showed the abort itself causing a + // stuck alarm_control_panel entity for a sequence that may well have succeeded. + // Success/failure comes solely from the ARMED_STATE broadcast or the shared 1s + // watchdog, same as CODE_ENTER_PENDING. + this->arm_disarm_digit_ack_byte_ = cmd_byte; + this->arm_disarm_digit_ack_byte_set_ = true; + } else if (cmd_byte != this->arm_disarm_digit_ack_byte_) { + ESP_LOGD(TAG, "Arm/disarm: CMD byte changed from 0x%02X to 0x%02X in CODE_DIGIT_PENDING, continuing", + this->arm_disarm_digit_ack_byte_, cmd_byte); + } + if (this->arm_disarm_code_idx_ < this->arm_disarm_code_digits_.size()) { + uint8_t digit = this->arm_disarm_code_digits_[this->arm_disarm_code_idx_++]; + ESP_LOGD(TAG, "Code sequence: sending digit %u (index %u)", digit, + this->arm_disarm_code_idx_ - 1); + this->keypress(digit); + this->arm_disarm_state_enter_ms_ = millis(); + } else { + ESP_LOGD(TAG, "Code sequence: sending terminal key 0x%02X", + this->arm_disarm_terminal_key_); + this->keypress(this->arm_disarm_terminal_key_); + this->arm_disarm_state_ = ArmDisarmState::CODE_ENTER_PENDING; + this->arm_disarm_state_enter_ms_ = millis(); + } + break; } - if (this->arm_disarm_code_idx_ < this->arm_disarm_code_digits_.size()) { - uint8_t digit = this->arm_disarm_code_digits_[this->arm_disarm_code_idx_++]; - ESP_LOGD(TAG, "Code sequence: sending digit %u (index %u)", digit, - this->arm_disarm_code_idx_ - 1); - this->keypress(digit); - this->arm_disarm_state_enter_ms_ = millis(); - } else { - ESP_LOGD(TAG, "Code sequence: sending terminal key 0x%02X", - this->arm_disarm_terminal_key_); - this->keypress(this->arm_disarm_terminal_key_); - this->arm_disarm_state_ = ArmDisarmState::CODE_ENTER_PENDING; - this->arm_disarm_state_enter_ms_ = millis(); + case ArmDisarmState::CODE_ENTER_PENDING: { + // byte[1] here is deliberately not used to judge success/failure — see the enum + // comment in crow_alarm_panel.h for why (logs-10/35, logs-12/37, 2026-07-12). This + // KEYPAD_COMMAND only proves the controller is still responding; only the + // independent ARMED_STATE broadcast (below) or the shared 1s watchdog resolves + // this state. + uint8_t cmd_byte = (data.size() > 1) ? data[1] : 0; + ESP_LOGD(TAG, "Code sequence: CMD 0x%02X after terminal key, awaiting ARMED_STATE confirmation", + cmd_byte); + break; } - break; - } - case ArmDisarmState::CODE_ENTER_PENDING: { - // byte[1] here is deliberately not used to judge success/failure — see the enum - // comment in crow_alarm_panel.h for why (logs-10/35, logs-12/37, 2026-07-12). This - // KEYPAD_COMMAND only proves the controller is still responding; only the - // independent ARMED_STATE broadcast (below) or the shared 1s watchdog resolves - // this state. - uint8_t cmd_byte = (data.size() > 1) ? data[1] : 0; - ESP_LOGD(TAG, "Code sequence: CMD 0x%02X after terminal key, awaiting ARMED_STATE confirmation", - cmd_byte); - break; + default: + break; } - default: - break; } // Drive zone-bypass state machine forward. // See docs/zone_bypass_state_machine.md for observed timing. @@ -906,18 +934,21 @@ void CrowAlarmPanel::loop() { // Arm/disarm watchdog: abort if any non-IDLE state exceeds 1s without progress. See // ARM_DISARM_MAX_RETRIES in crow_alarm_panel.h for why retrying here (unlike output-select/ // zone-bypass above) is safe: a timeout reliably means the panel's state did not change. + // Retries are paced by a growing backoff (ARM_DISARM_RETRY_BACKOFF_MS) instead of restarting + // immediately: back-to-back retries burned the whole budget in ~7 s during a controller + // degradation episode that outlasted them (HA log 2026-08-19 17:09 — see + // arm_disarm_state_machine.md), while a manual call ~8 s after the abort succeeded first try. if (this->arm_disarm_state_ != ArmDisarmState::IDLE) { const uint32_t now_ms = millis(); - if (now_ms - this->arm_disarm_state_enter_ms_ > 1000) { - if (this->arm_disarm_retry_count_ < ARM_DISARM_MAX_RETRIES) { - this->arm_disarm_retry_count_++; - ESP_LOGW(TAG, "Arm/disarm: timeout in state %u, retrying (%u/%u)", - static_cast(this->arm_disarm_state_), this->arm_disarm_retry_count_, + if (this->arm_disarm_retry_pending_) { + if (now_ms - this->arm_disarm_state_enter_ms_ >= this->arm_disarm_retry_backoff_ms_) { + ESP_LOGD(TAG, "Arm/disarm: starting retry %u/%u", this->arm_disarm_retry_count_, ARM_DISARM_MAX_RETRIES); - // Refresh the watchdog timer BEFORE any keypress() below — keypress()->send_packet() - // can delay()/yield() and re-enter this loop(), and with the old timestamp still in - // place the watchdog would see itself as still timed out, firing again and sending an - // extra keypress (same race the output-select retry above avoids the same way). + // Clear the pending flag and refresh the watchdog timer BEFORE any keypress() below — + // keypress()->send_packet() can delay()/yield() and re-enter this loop(), and with the + // old values still in place this branch would fire again and send an extra keypress + // (same race the output-select retry above avoids the same way). + this->arm_disarm_retry_pending_ = false; this->arm_disarm_state_enter_ms_ = millis(); switch (this->arm_disarm_state_) { case ArmDisarmState::ARM_AWAY_PENDING: @@ -938,6 +969,16 @@ void CrowAlarmPanel::loop() { default: break; } + } + } else if (now_ms - this->arm_disarm_state_enter_ms_ > 1000) { + if (this->arm_disarm_retry_count_ < ARM_DISARM_MAX_RETRIES) { + this->arm_disarm_retry_count_++; + this->arm_disarm_retry_backoff_ms_ = ARM_DISARM_RETRY_BACKOFF_MS[this->arm_disarm_retry_count_ - 1]; + ESP_LOGW(TAG, "Arm/disarm: timeout in state %u, retrying (%u/%u) in %u ms", + static_cast(this->arm_disarm_state_), this->arm_disarm_retry_count_, + ARM_DISARM_MAX_RETRIES, this->arm_disarm_retry_backoff_ms_); + this->arm_disarm_retry_pending_ = true; + this->arm_disarm_state_enter_ms_ = now_ms; } else { ESP_LOGW(TAG, "Arm/disarm: timeout in state %u, aborting after %u retries", static_cast(this->arm_disarm_state_), this->arm_disarm_retry_count_); @@ -945,6 +986,7 @@ void CrowAlarmPanel::loop() { this->arm_disarm_code_digits_.clear(); this->arm_disarm_code_idx_ = 0; this->arm_disarm_retry_count_ = 0; + this->arm_disarm_retry_pending_ = false; // CrowAlarmControlPanel::control() optimistically publishes ACP_STATE_ARMING/DISARMING // before this sequence resolves. On abort no ARMED_STATE broadcast is coming to correct // that, so without this the entity would be stuck in the transitional state forever, @@ -1000,6 +1042,7 @@ void CrowAlarmPanel::start_code_sequence_(const std::string &code, uint8_t termi this->arm_disarm_state_ = ArmDisarmState::CODE_DIGIT_PENDING; this->arm_disarm_state_enter_ms_ = millis(); this->arm_disarm_retry_count_ = 0; + this->arm_disarm_retry_pending_ = false; this->arm_disarm_digit_ack_byte_set_ = false; this->keypress(this->arm_disarm_code_digits_[0]); } @@ -1021,6 +1064,7 @@ void CrowAlarmPanel::arm_away(const std::string &code) { this->arm_disarm_state_ = ArmDisarmState::ARM_AWAY_PENDING; this->arm_disarm_state_enter_ms_ = millis(); this->arm_disarm_retry_count_ = 0; + this->arm_disarm_retry_pending_ = false; this->keypress(KEY_ARM); } } @@ -1042,6 +1086,7 @@ void CrowAlarmPanel::arm_stay(const std::string &code) { this->arm_disarm_state_ = ArmDisarmState::ARM_STAY_PENDING; this->arm_disarm_state_enter_ms_ = millis(); this->arm_disarm_retry_count_ = 0; + this->arm_disarm_retry_pending_ = false; this->keypress(KEY_STAY); } } diff --git a/components/crow_alarm_panel/crow_alarm_panel.h b/components/crow_alarm_panel/crow_alarm_panel.h index 1617a9d..0280b35 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.h +++ b/components/crow_alarm_panel/crow_alarm_panel.h @@ -327,10 +327,25 @@ class CrowAlarmPanel : public Component { // A watchdog timeout here reliably means the panel's state did not change (see // docs/arm_disarm_state_machine.md — multiple sessions with an independent monitor capture // confirm no ARMED_STATE broadcast occurred around the timeout), so unlike output-select/ - // zone-bypass a blind retry can't undo a change that already landed. 5 covers the worst - // consecutive-failure streak observed so far (logs-24, logs-42: 5 failures before success). - static const uint8_t ARM_DISARM_MAX_RETRIES = 5; + // zone-bypass a blind retry can't undo a change that already landed. 6 with the growing + // backoff below spans the worst episode observed so far (HA log 2026-08-19 17:09: two + // consecutive calls exhausted 5 back-to-back retries each, and a manual call ~29 s after the + // first attempt succeeded — back-to-back retries burned the whole budget in ~7 s, well inside + // that episode). + static const uint8_t ARM_DISARM_MAX_RETRIES = 6; uint8_t arm_disarm_retry_count_{0}; + // Pause before each retry, indexed by (retry number - 1). Controller degradation episodes + // last tens of seconds (see arm_disarm_state_machine.md, 2026-08-19 entry); every eventual + // success in the trace record came after multi-second gaps, so the retry envelope must span + // ~40 s rather than hammering attempts every ~1.5 s (which also adds bus contention, the + // prime suspect for the collision failure mode). + static constexpr uint32_t ARM_DISARM_RETRY_BACKOFF_MS[ARM_DISARM_MAX_RETRIES] = {1000, 2000, 3000, + 5000, 8000, 13000}; + // While true, the previous attempt has timed out and the next retry is waiting out its + // backoff: arm_disarm_state_enter_ms_ marks the wait start and arm_disarm_retry_backoff_ms_ + // the required pause. KEYPAD_COMMANDs must not advance the (dead) sequence during the wait. + bool arm_disarm_retry_pending_{false}; + uint32_t arm_disarm_retry_backoff_ms_{0}; // "Digit accepted" display_code (KEYPAD_COMMAND byte[1]) learned from the first digit's // response each sequence. Physical keypad types disagree on this value (0x01 is common, // but address 0x05 has been observed sending 0x07 for the same "more digits expected" diff --git a/components/crow_alarm_panel/docs/arm_disarm_state_machine.md b/components/crow_alarm_panel/docs/arm_disarm_state_machine.md index 8668310..9ee7731 100644 --- a/components/crow_alarm_panel/docs/arm_disarm_state_machine.md +++ b/components/crow_alarm_panel/docs/arm_disarm_state_machine.md @@ -102,14 +102,15 @@ CODE_ENTER_PENDING (terminal key sent, waiting for final Command) │ └─> CODE_ENTER_PENDING (logged only, proves the controller is still responding; byte[1] is │ not used to judge success or failure here — see "CODE_ENTER_PENDING success detection │ redesigned" below for why) -├─ on ARMED_STATE (0x11) recognized as Arming/Armed Away/Disarmed (dispatched independently by -│ the message handler, not gated on this state) -│ └─> IDLE (success) +├─ on ARMED_STATE (0x11) matching the request's intent (Disarmed for a disarm; Arming or +│ Armed Away for an arm — dispatched independently by the message handler) +│ └─> IDLE (success; applies from ANY non-IDLE state, including a retry backoff wait — see +│ "Retry backoff + intent-matched ARMED_STATE resolution" (2026-08-22) below) ├─ on timeout (>1s) -│ └─> IDLE (abort; failure) +│ └─> retry with growing backoff, up to ARM_DISARM_MAX_RETRIES; then IDLE (abort; failure) [Note: ARMED_STATE (0x11) messages are dispatched by the message handler independently; -the state machine does not gate on them.] +entity state is always published from them regardless of the state machine.] ``` ### Two paths per operation @@ -168,7 +169,9 @@ Code-entry path is available when the controller requires code authentication. 3. **Code required for disarm:** `disarm()` always takes a code; there is no no-code toggle path 4. **Shared code-sequence helper:** `start_code_sequence_(code, terminal_key)` handles arm-with-code and disarm identically; terminal key differs (KEY_ARM, KEY_STAY, or KEY_ENTER) 5. **No ACK barrier:** Unlike OUTPUT, the code sequence does not wait for an ACK (0x1D) -6. **Timeout recovery:** ~1s per state; if no Command received, abort to IDLE +6. **Timeout recovery:** ~1s per state; on timeout the sequence is retried up to + `ARM_DISARM_MAX_RETRIES` times with a growing backoff between attempts (see the 2026-08-05 + and 2026-08-22 entries below), then aborts to IDLE 7. **Command byte validation:** `CODE_DIGIT_PENDING` does not hardcode the "digit accepted" byte[1] value to 0x01. It learns the value from the first response of each sequence for diagnostics, but no longer aborts if a later response doesn't match that baseline — see "Digit-ack byte is keypad-address-specific, not a validity signal" below for why that broke ESPHome-initiated sequences, and "CMD-byte-change abort removed" (2026-07-25, logs-18) below for why the mid-sequence-abort behavior itself was removed. `CODE_ENTER_PENDING` **also does not use byte[1]** to judge success or failure — see "CODE_ENTER_PENDING success detection redesigned" (2026-07-12) below for why that was proven unreliable in both directions. Both states now rely solely on an independent `ARMED_STATE` broadcast for success, or the shared 1s watchdog for failure. ## Timing Observations @@ -438,11 +441,81 @@ A single `disarm()` call resolved in 3.8s total across two automatic retries, wi **Further confirmed (2026-08-19, logs-33):** `esphome-aap-alarm-interface-logs-33.txt`, four arm/disarm cycles in a ~14-minute session (`14:06`–`14:20`). All four arms completed cleanly on the first attempt. Two of the four disarms (`14:08:36.196`, `14:20:00.756`) hit the ordinary `CODE_ENTER_PENDING` silence signature — terminal key acked (`CMD 0x07`), then genuine bus silence, 1s watchdog fires `Arm/disarm: timeout in state 4, retrying (1/5)`, full code+terminal sequence resent, succeeds immediately on the retry with no user action. Same shape as the logs-27 validation above, one retry each instead of two, still 100% eventual success. No new failure mechanism, no code change proposed. +## Retry backoff + intent-matched ARMED_STATE resolution (2026-08-22) + +**Observed facts** (`traces/home-assistant_2026-08-19T08-03-09.331Z.log` — this episode falls +outside every interface/monitor capture, including logs-33 from earlier the same day): at +`2026-08-19 17:09:17` and `17:09:30`, two consecutive `disarm()` calls each exhausted **all 5 +automatic retries** and aborted (`Arm/disarm: timeout in state 4, aborting after 5 retries`) — +12 failed attempts in ~21 s, alternating between state 3 (digit silence) and state 4 +(terminal-key silence). A third call at `17:09:46` — ~8 s after the second abort, ~29 s after the +first attempt — succeeded with no retry. The wider context marks this as a controller-side +degradation episode, the strongest evidence yet for the "session-level bad state" theory: the +controller's `KEYPAD_PING` polling stopped entirely around `17:10:55` (the "No ping for 60 s" +storm fires at `17:11:55`), and both physical keypads spontaneously re-registered +(`17:11:16` IP Keypad, `17:11:32` AAP Keypad) — the whole bus lost the controller for a while, +starting ~90 s after the failed disarms. Two near-misses in the same HA logs reinforce the trend: +`2026-08-18 16:17:54` needed 3–4 retries and `2026-08-17 16:47:23` needed 4–5 retries to succeed. + +**Inference (medium confidence):** the 2026-08-05 back-to-back retry loop burns its whole budget +in ~7 s (attempt + 1 s watchdog ≈ 1.5 s cadence), well inside a degradation episode that +evidently lasted tens of seconds — while every eventual success in the whole trace record +(manual retries in logs-22/23/24/25/26, and this episode's `17:09:46` recovery) came after +multi-second gaps. Hammering also adds bus contention, the prime suspect for the collision +failure mode (modes documented above). + +**Fix applied (1/2 — retry backoff):** the watchdog no longer restarts the sequence immediately +on timeout. It schedules the retry after a growing pause (`ARM_DISARM_RETRY_BACKOFF_MS`: +1/2/3/5/8/13 s for retries 1–6; `ARM_DISARM_MAX_RETRIES` raised 5 → 6), stretching one call's +retry envelope from ~7 s to ~40 s — spanning the ~29 s recovery observed on 2026-08-19. While +the backoff is pending, `KEYPAD_COMMAND`s no longer advance the (dead) sequence — without that +gate, a stray periodic command arriving mid-wait would type leftover digits into the panel +outside any attempt. `send_packet()`'s existing bus-idle wait covers TX timing when the retry +fires. The abort fallback (restore `last_confirmed_acp_state_`) is unchanged. + +**Fix applied (2/2 — intent-matched ARMED_STATE resolution from any state):** two related holes +in the previous `CODE_ENTER_PENDING`-only resolution, both made more likely by retries: + +- A slow controller can broadcast the outcome of attempt N after the watchdog has already + restarted the sequence (logs-47/26 recorded an 825 ms ack, right at the 1 s boundary; logs-12/37 + recorded a `Disarmed` broadcast 92 ms *after* an abort). A late `Disarmed` arriving while the + retry sits in `CODE_DIGIT_PENDING` previously didn't stop the machine — it would keep typing + the remaining digits + ENTER into a now-disarmed panel, and code+ENTER while disarmed is + exactly the "arm with code" gesture (logs-42 armed that way), i.e. **the disarm retry could + re-arm the panel**. Never observed in a trace, but structurally reachable; now closed. +- The registration handshake re-broadcasts the *current* state (logs-46: repeated `Armed Away` + lines while armed during a re-registration storm). `CODE_ENTER_PENDING` previously accepted + *any* recognized ARMED_STATE as success, so an `Armed Away` re-announce landing in a disarm's + `CODE_ENTER_PENDING` window would have falsely completed the disarm while the panel stayed + armed. Also never directly observed, but the 2026-08-19 episode shows registration storms and + disarm attempts do co-occur. + +Resolution is now intent-matched — a disarm (terminal key `KEY_ENTER`) resolves only on +`Disarmed`; an arm (no-code or code path) resolves on `Arming` or `Armed Away` — and applies +from every non-IDLE state including the retry backoff wait, fully resetting the state machine +(retry counter and pending flag included). Non-matching recognized broadcasts are logged and +ignored by the state machine (entity state is still published from them unconditionally, as +always). + +Compiles and passes `esphome config`/`esphome compile` against `crow_alarm_panel_test.yaml`. +**Not yet validated on real hardware** — needs a fresh capture reproducing a multi-failure +streak (ideally another degradation episode like 2026-08-19 17:09) to confirm the backoff +envelope rides it out within a single call, and that the intent-match never wrongly ignores a +genuine confirmation (watch specifically for an arm-stay confirmation shape other than +`Arming`/`Armed Away`, which would now retry instead of resolving — see Open point below). + +**Open point:** if the controller broadcasts a distinct "Armed Stay" ARMED_STATE pattern (none +of the three recognized patterns `00.01`/`01.00`/`00.00`), an arm-stay sequence would no longer +resolve via broadcast at all and would fall to the watchdog/retry path, re-pressing KEY_STAY. +The previous code had the same recognition gap (unknown patterns were logged and ignored), so +this is not a regression, but a stay-mode capture confirming the actual broadcast bytes would +settle it. + ## Notes - ARM/STAY/DISARM sequences are simpler than OUTPUT because there's no ACK handshake - No-code arm_away()/arm_stay() complete after a single KEYPAD_COMMAND (~100ms total) - Code-based sequences share the same `CODE_DIGIT_PENDING` / `CODE_ENTER_PENDING` states regardless of whether the operation is arm or disarm -- ARMED_STATE messages (0x11) are published independently regardless of `arm_disarm_state_` (entities always reflect them). As of 2026-07-12, `CODE_ENTER_PENDING` is the one exception that actively *waits* on one to resolve — see "CODE_ENTER_PENDING success detection redesigned" above. `ARM_AWAY_PENDING`/`ARM_STAY_PENDING`/`CODE_DIGIT_PENDING` still don't gate on it. +- ARMED_STATE messages (0x11) are published independently regardless of `arm_disarm_state_` (entities always reflect them). As of 2026-08-22, an ARMED_STATE broadcast matching the request's intent resolves the sequence from **any** non-IDLE state (including a retry backoff wait) — see "Retry backoff + intent-matched ARMED_STATE resolution" below. `CODE_ENTER_PENDING` remains the only state that *requires* one to succeed (`ARM_AWAY_PENDING`/`ARM_STAY_PENDING` still resolve on their KEYPAD_COMMAND ack). - All three keypads receive Command broadcasts during arming/disarming; only the originating keypad controls the sequence - `disarm()` also guards against calling when already disarmed — it returns early if `is_armed()` is false From d4416e74b9542093a2f17106f2ddb5ed0184dd26 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sat, 22 Aug 2026 16:31:10 +1200 Subject: [PATCH 02/12] fix(crow_alarm_panel): silence -Wformat warning on backoff log Cast arm_disarm_retry_backoff_ms_ (uint32_t, long unsigned int on this target) to unsigned for the %u format specifier, matching the existing (unsigned) casts used for the other uint32 durations logged here. Co-Authored-By: Claude Fable 5 --- components/crow_alarm_panel/crow_alarm_panel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index 8ff3237..00a7726 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -976,7 +976,7 @@ void CrowAlarmPanel::loop() { this->arm_disarm_retry_backoff_ms_ = ARM_DISARM_RETRY_BACKOFF_MS[this->arm_disarm_retry_count_ - 1]; ESP_LOGW(TAG, "Arm/disarm: timeout in state %u, retrying (%u/%u) in %u ms", static_cast(this->arm_disarm_state_), this->arm_disarm_retry_count_, - ARM_DISARM_MAX_RETRIES, this->arm_disarm_retry_backoff_ms_); + ARM_DISARM_MAX_RETRIES, (unsigned) this->arm_disarm_retry_backoff_ms_); this->arm_disarm_retry_pending_ = true; this->arm_disarm_state_enter_ms_ = now_ms; } else { From 00a7d1fa831b2194968c15e838000335e0580c4d Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sat, 22 Aug 2026 17:06:38 +1200 Subject: [PATCH 03/12] fix(crow_alarm_panel): set CODE_ENTER_PENDING before terminal keypress keypress()->send_packet() can delay()/yield() and re-enter loop(). If the matching ARMED_STATE broadcast resolved the sequence during that yield, assigning CODE_ENTER_PENDING after the call resurrected the completed sequence with an emptied code vector, and the watchdog retry would then index arm_disarm_code_digits_[0] out of bounds. Set state and timestamp before the keypress, as every other send path already does. Flagged by Copilot review on PR #12. Co-Authored-By: Claude Fable 5 --- components/crow_alarm_panel/crow_alarm_panel.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index 00a7726..c12372c 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -696,9 +696,14 @@ void CrowAlarmPanel::loop() { } else { ESP_LOGD(TAG, "Code sequence: sending terminal key 0x%02X", this->arm_disarm_terminal_key_); - this->keypress(this->arm_disarm_terminal_key_); + // Set state BEFORE the keypress — send_packet() delays/yields internally and can + // re-enter loop(). If the matching ARMED_STATE broadcast resolves the sequence + // during that yield (state -> IDLE, code cleared), assigning after the call would + // resurrect CODE_ENTER_PENDING and the watchdog retry would index the emptied + // code vector. this->arm_disarm_state_ = ArmDisarmState::CODE_ENTER_PENDING; this->arm_disarm_state_enter_ms_ = millis(); + this->keypress(this->arm_disarm_terminal_key_); } break; } From ad453ec8c3c3d929436d8a7365803db33ad3ec32 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sat, 22 Aug 2026 17:16:18 +1200 Subject: [PATCH 04/12] fix(crow_alarm_panel): drop arm/disarm keys committed before resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolving the arm/disarm state machine to IDLE could not recall a keypress already waiting inside send_packet()'s bus-idle loop — that wait delays/yields (re-entering loop()) and then transmits unconditionally. If a late matching ARMED_STATE broadcast resolved the sequence during a retry's terminal-key wait, ENTER still went out; on a just-disarmed panel the retry's buffered digits + ENTER is exactly the 'arm with code' gesture and could re-arm it. Arm/disarm keys now go through arm_disarm_keypress_(), which captures a generation token before the bus-idle wait and re-checks it immediately before the blocking transmit; every resolution to IDLE (broadcast confirmation, ARM/STAY ack, watchdog abort) bumps the token, so a key whose sequence has since resolved is dropped instead of typed into the panel. send_packet() is split into wait_for_bus_idle_() + transmit_packet_() to share those pieces; behavior of other send paths is unchanged. Flagged by Copilot review on PR #12. Co-Authored-By: Claude Fable 5 --- .../crow_alarm_panel/crow_alarm_panel.cpp | 60 ++++++++++++++----- .../crow_alarm_panel/crow_alarm_panel.h | 12 ++++ .../docs/arm_disarm_state_machine.md | 9 +++ 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index c12372c..6167f49 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -483,6 +483,7 @@ void CrowAlarmPanel::loop() { if (matches_intent) { ESP_LOGD(TAG, "Code sequence: complete (confirmed via ARMED_STATE broadcast)"); this->arm_disarm_state_ = ArmDisarmState::IDLE; + this->arm_disarm_generation_++; this->arm_disarm_code_digits_.clear(); this->arm_disarm_code_idx_ = 0; this->arm_disarm_retry_count_ = 0; @@ -665,6 +666,7 @@ void CrowAlarmPanel::loop() { case ArmDisarmState::ARM_STAY_PENDING: ESP_LOGD(TAG, "Arm/stay: CMD received, sequence complete"); this->arm_disarm_state_ = ArmDisarmState::IDLE; + this->arm_disarm_generation_++; break; case ArmDisarmState::CODE_DIGIT_PENDING: { uint8_t cmd_byte = (data.size() > 1) ? data[1] : 0; @@ -691,7 +693,7 @@ void CrowAlarmPanel::loop() { uint8_t digit = this->arm_disarm_code_digits_[this->arm_disarm_code_idx_++]; ESP_LOGD(TAG, "Code sequence: sending digit %u (index %u)", digit, this->arm_disarm_code_idx_ - 1); - this->keypress(digit); + this->arm_disarm_keypress_(digit); this->arm_disarm_state_enter_ms_ = millis(); } else { ESP_LOGD(TAG, "Code sequence: sending terminal key 0x%02X", @@ -703,7 +705,7 @@ void CrowAlarmPanel::loop() { // code vector. this->arm_disarm_state_ = ArmDisarmState::CODE_ENTER_PENDING; this->arm_disarm_state_enter_ms_ = millis(); - this->keypress(this->arm_disarm_terminal_key_); + this->arm_disarm_keypress_(this->arm_disarm_terminal_key_); } break; } @@ -957,10 +959,10 @@ void CrowAlarmPanel::loop() { this->arm_disarm_state_enter_ms_ = millis(); switch (this->arm_disarm_state_) { case ArmDisarmState::ARM_AWAY_PENDING: - this->keypress(KEY_ARM); + this->arm_disarm_keypress_(KEY_ARM); break; case ArmDisarmState::ARM_STAY_PENDING: - this->keypress(KEY_STAY); + this->arm_disarm_keypress_(KEY_STAY); break; case ArmDisarmState::CODE_DIGIT_PENDING: case ArmDisarmState::CODE_ENTER_PENDING: @@ -969,7 +971,7 @@ void CrowAlarmPanel::loop() { this->arm_disarm_code_idx_ = 1; this->arm_disarm_state_ = ArmDisarmState::CODE_DIGIT_PENDING; this->arm_disarm_digit_ack_byte_set_ = false; - this->keypress(this->arm_disarm_code_digits_[0]); + this->arm_disarm_keypress_(this->arm_disarm_code_digits_[0]); break; default: break; @@ -988,6 +990,7 @@ void CrowAlarmPanel::loop() { ESP_LOGW(TAG, "Arm/disarm: timeout in state %u, aborting after %u retries", static_cast(this->arm_disarm_state_), this->arm_disarm_retry_count_); this->arm_disarm_state_ = ArmDisarmState::IDLE; + this->arm_disarm_generation_++; this->arm_disarm_code_digits_.clear(); this->arm_disarm_code_idx_ = 0; this->arm_disarm_retry_count_ = 0; @@ -1049,7 +1052,7 @@ void CrowAlarmPanel::start_code_sequence_(const std::string &code, uint8_t termi this->arm_disarm_retry_count_ = 0; this->arm_disarm_retry_pending_ = false; this->arm_disarm_digit_ack_byte_set_ = false; - this->keypress(this->arm_disarm_code_digits_[0]); + this->arm_disarm_keypress_(this->arm_disarm_code_digits_[0]); } void CrowAlarmPanel::arm_away(const std::string &code) { @@ -1070,7 +1073,7 @@ void CrowAlarmPanel::arm_away(const std::string &code) { this->arm_disarm_state_enter_ms_ = millis(); this->arm_disarm_retry_count_ = 0; this->arm_disarm_retry_pending_ = false; - this->keypress(KEY_ARM); + this->arm_disarm_keypress_(KEY_ARM); } } @@ -1092,7 +1095,7 @@ void CrowAlarmPanel::arm_stay(const std::string &code) { this->arm_disarm_state_enter_ms_ = millis(); this->arm_disarm_retry_count_ = 0; this->arm_disarm_retry_pending_ = false; - this->keypress(KEY_STAY); + this->arm_disarm_keypress_(KEY_STAY); } } @@ -1117,6 +1120,23 @@ void CrowAlarmPanel::keypress(uint8_t key) { this->send_packet(KEYPRESS, {key}); } +void CrowAlarmPanel::arm_disarm_keypress_(uint8_t key) { + // The bus-idle wait delays/yields and re-enters loop(). If the sequence resolves during that + // window (matching ARMED_STATE broadcast, or watchdog abort), the generation bumps and this + // key must no longer go out: setting arm_disarm_state_ back to IDLE can't recall a keypress + // already committed to send_packet(), and after a disarm resolution a retry's already-typed + // digits + ENTER is exactly the "arm with code" gesture (could re-arm the panel). + const uint32_t gen = this->arm_disarm_generation_; + if (!this->wait_for_bus_idle_()) { + return; + } + if (gen != this->arm_disarm_generation_) { + ESP_LOGD(TAG, "Arm/disarm: sequence resolved during bus-idle wait, suppressing key 0x%02X", key); + return; + } + this->transmit_packet_(KEYPRESS, {key}); +} + void CrowAlarmPanel::set_output(uint8_t output, bool state) { if (!this->is_active_keypad()) { ESP_LOGW(TAG, "set_output(%u, %s): passive monitor mode, ignoring", output, state ? "on" : "off"); @@ -1212,22 +1232,32 @@ void CrowAlarmPanel::send_packet(uint8_t type, const std::vector &data) ESP_LOGW(TAG, "No keypad address configured — cannot send packet (passive monitor mode)"); return; } - std::vector packet; - packet.push_back(BOUNDARY); - packet.push_back(type); - packet.push_back(this->keypad_address_); - packet.insert(packet.end(), data.begin(), data.end()); - packet.push_back(BOUNDARY); + if (!this->wait_for_bus_idle_()) { + return; + } + this->transmit_packet_(type, data); +} +bool CrowAlarmPanel::wait_for_bus_idle_() { uint32_t start_ms = millis(); while (!this->is_bus_idle_()) { if (millis() - start_ms > 5000) { ESP_LOGW(TAG, "Timeout waiting for bus idle before sending packet"); - return; + return false; } delay(2); yield(); } + return true; +} + +void CrowAlarmPanel::transmit_packet_(uint8_t type, const std::vector &data) { + std::vector packet; + packet.push_back(BOUNDARY); + packet.push_back(type); + packet.push_back(this->keypad_address_); + packet.insert(packet.end(), data.begin(), data.end()); + packet.push_back(BOUNDARY); ESP_LOGD(TAG, "Sending packet: [%s]", format_hex_pretty(packet).c_str()); this->send_packet_blocking_(packet); diff --git a/components/crow_alarm_panel/crow_alarm_panel.h b/components/crow_alarm_panel/crow_alarm_panel.h index 0280b35..100e9a7 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.h +++ b/components/crow_alarm_panel/crow_alarm_panel.h @@ -283,7 +283,12 @@ class CrowAlarmPanel : public Component { protected: CrowAlarmPanelKeypad find_keypad_(uint8_t address); bool is_bus_idle_(); + bool wait_for_bus_idle_(); + void transmit_packet_(uint8_t type, const std::vector &data); void start_code_sequence_(const std::string &code, uint8_t terminal_key); + // keypress() variant for the arm/disarm state machine only: suppresses the transmit if the + // sequence resolved while waiting for bus idle (see arm_disarm_generation_). + void arm_disarm_keypress_(uint8_t key); void send_packet_blocking_(const std::vector &packet); bool wait_for_clock_edge_(bool wait_for_state, uint32_t timeout_us); @@ -352,6 +357,13 @@ class CrowAlarmPanel : public Component { // semantic — see docs/arm_disarm_state_machine.md), so it can't be hardcoded to 0x01. uint8_t arm_disarm_digit_ack_byte_{0}; bool arm_disarm_digit_ack_byte_set_{false}; + // Cancellation token for in-flight arm/disarm keypresses. arm_disarm_keypress_() captures + // this before the bus-idle wait (which delays/yields and re-enters loop()) and re-checks it + // immediately before transmitting; every resolution to IDLE increments it. Without this, a + // matching ARMED_STATE broadcast landing during that wait resolves the sequence but the + // already-committed key still goes out — after a disarm resolution, a retry's already-typed + // digits + ENTER is exactly the "arm with code" gesture and could re-arm the panel. + uint32_t arm_disarm_generation_{0}; bool raw_frame_logging_enabled_{false}; diff --git a/components/crow_alarm_panel/docs/arm_disarm_state_machine.md b/components/crow_alarm_panel/docs/arm_disarm_state_machine.md index 9ee7731..6a993de 100644 --- a/components/crow_alarm_panel/docs/arm_disarm_state_machine.md +++ b/components/crow_alarm_panel/docs/arm_disarm_state_machine.md @@ -497,6 +497,15 @@ from every non-IDLE state including the retry backoff wait, fully resetting the ignored by the state machine (entity state is still published from them unconditionally, as always). +A review follow-up (PR #12) closed one more layer of the same hazard: resolving the state +machine to IDLE cannot recall a keypress already inside `send_packet()`'s bus-idle wait — that +wait delays/yields (re-entering `loop()`) and then transmits unconditionally once the bus goes +idle. Arm/disarm keys are now sent via `arm_disarm_keypress_()`, which captures a generation +token (`arm_disarm_generation_`) before the wait and re-checks it immediately before the +blocking transmit; every resolution to IDLE (broadcast confirmation, ARM/STAY ack, watchdog +abort) bumps the token, so a key committed by a sequence that has since resolved is suppressed +instead of typed into the panel. + Compiles and passes `esphome config`/`esphome compile` against `crow_alarm_panel_test.yaml`. **Not yet validated on real hardware** — needs a fresh capture reproducing a multi-failure streak (ideally another degradation episode like 2026-08-19 17:09) to confirm the backoff From ff3e9988cd2bf0317bc15b26cf17e86356e4a6c5 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sat, 22 Aug 2026 17:28:06 +1200 Subject: [PATCH 05/12] fix(crow_alarm_panel): gate CMD handling, watchdog on in-flight key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generation-token cancellation (ad453ec) exposed two windows where the state machine acts on events preceding the committed key's actual transmission (Copilot review on PR #12): - A periodic KEYPAD_COMMAND arriving during KEY_ARM/KEY_STAY's bus-idle wait falsely completed ARM_*_PENDING (it cannot ack a key that has not transmitted) and bumped the generation, so the arm key was then suppressed entirely — sequence reported done, panel never armed. The same pre-transmit CMD could also advance CODE_DIGIT_PENDING and emit digits out of order. - The watchdog could start a retry from inside an earlier keypress's bus-idle wait; both waiters then passed the generation check and transmitted duplicate keys. arm_disarm_key_in_flight_ is now true from commit to transmit; the KEYPAD_COMMAND-driven transitions and the watchdog are skipped while it is set. ARMED_STATE resolution stays ungated — it is the controller's own state and cancelling the in-flight key is the token's purpose. On transmit the no-progress window restarts from the actual send, since the wait may have consumed the 1 s budget while the watchdog was gated. Also updates the state diagram's stale timeout branches (retries apply to every non-IDLE state, not just CODE_ENTER_PENDING). Co-Authored-By: Claude Fable 5 --- .../crow_alarm_panel/crow_alarm_panel.cpp | 24 ++++++++++++++++--- .../crow_alarm_panel/crow_alarm_panel.h | 9 +++++++ .../docs/arm_disarm_state_machine.md | 16 +++++++++++-- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index 6167f49..2f71ce0 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -659,8 +659,12 @@ void CrowAlarmPanel::loop() { // Drive arm/disarm state machine forward. Skipped while a retry backoff is pending: // the timed-out attempt is dead and will restart from the first digit, so a stray // periodic KEYPAD_COMMAND arriving mid-wait must not advance the stale sequence - // (it would type leftover digits into the panel outside any attempt). - if (!this->arm_disarm_retry_pending_) { + // (it would type leftover digits into the panel outside any attempt). Also skipped + // while a key is committed but not yet transmitted (arm_disarm_key_in_flight_): a + // command arriving during that bus-idle wait cannot acknowledge the pending key, so + // acting on it would falsely complete ARM/STAY (suppressing the never-sent key) or + // send the next digit out of order. + if (!this->arm_disarm_retry_pending_ && !this->arm_disarm_key_in_flight_) { switch (this->arm_disarm_state_) { case ArmDisarmState::ARM_AWAY_PENDING: case ArmDisarmState::ARM_STAY_PENDING: @@ -945,7 +949,11 @@ void CrowAlarmPanel::loop() { // immediately: back-to-back retries burned the whole budget in ~7 s during a controller // degradation episode that outlasted them (HA log 2026-08-19 17:09 — see // arm_disarm_state_machine.md), while a manual call ~8 s after the abort succeeded first try. - if (this->arm_disarm_state_ != ArmDisarmState::IDLE) { + // Gated while a key is committed but not yet transmitted (arm_disarm_key_in_flight_): timing + // out an attempt whose key hasn't gone out yet is premature, and a retry started from inside + // that key's bus-idle wait would create a second waiter transmitting a duplicate key. + // arm_disarm_keypress_() restarts the no-progress window when the key actually transmits. + if (this->arm_disarm_state_ != ArmDisarmState::IDLE && !this->arm_disarm_key_in_flight_) { const uint32_t now_ms = millis(); if (this->arm_disarm_retry_pending_) { if (now_ms - this->arm_disarm_state_enter_ms_ >= this->arm_disarm_retry_backoff_ms_) { @@ -1126,15 +1134,25 @@ void CrowAlarmPanel::arm_disarm_keypress_(uint8_t key) { // key must no longer go out: setting arm_disarm_state_ back to IDLE can't recall a keypress // already committed to send_packet(), and after a disarm resolution a retry's already-typed // digits + ENTER is exactly the "arm with code" gesture (could re-arm the panel). + // arm_disarm_key_in_flight_ gates the KEYPAD_COMMAND-driven transitions and the watchdog + // for the duration of the wait — see the member comment in crow_alarm_panel.h. const uint32_t gen = this->arm_disarm_generation_; + this->arm_disarm_key_in_flight_ = true; if (!this->wait_for_bus_idle_()) { + this->arm_disarm_key_in_flight_ = false; return; } if (gen != this->arm_disarm_generation_) { ESP_LOGD(TAG, "Arm/disarm: sequence resolved during bus-idle wait, suppressing key 0x%02X", key); + this->arm_disarm_key_in_flight_ = false; return; } this->transmit_packet_(KEYPRESS, {key}); + // Restart the no-progress window from the actual transmit — the bus-idle wait above may + // have consumed most (or more) of the 1 s budget before the key even went out, and the + // watchdog was gated during it. + this->arm_disarm_state_enter_ms_ = millis(); + this->arm_disarm_key_in_flight_ = false; } void CrowAlarmPanel::set_output(uint8_t output, bool state) { diff --git a/components/crow_alarm_panel/crow_alarm_panel.h b/components/crow_alarm_panel/crow_alarm_panel.h index 100e9a7..58f328d 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.h +++ b/components/crow_alarm_panel/crow_alarm_panel.h @@ -364,6 +364,15 @@ class CrowAlarmPanel : public Component { // already-committed key still goes out — after a disarm resolution, a retry's already-typed // digits + ENTER is exactly the "arm with code" gesture and could re-arm the panel. uint32_t arm_disarm_generation_{0}; + // True while arm_disarm_keypress_() is between commit and transmit (i.e. inside the bus-idle + // wait, which re-enters loop()). Gates the KEYPAD_COMMAND-driven state machine transitions + // and the watchdog during that window: a command arriving before the key transmits cannot be + // its acknowledgement (acting on it would falsely complete ARM/STAY, or advance the digit + // sequence and send digits out of order), and a watchdog retry starting mid-wait would + // create a second waiter that transmits a duplicate key once the bus goes idle. ARMED_STATE + // resolution is NOT gated — it is the controller's own state, independent of our TX, and + // cancelling the in-flight key is exactly what arm_disarm_generation_ is for. + bool arm_disarm_key_in_flight_{false}; bool raw_frame_logging_enabled_{false}; diff --git a/components/crow_alarm_panel/docs/arm_disarm_state_machine.md b/components/crow_alarm_panel/docs/arm_disarm_state_machine.md index 6a993de..868fa46 100644 --- a/components/crow_alarm_panel/docs/arm_disarm_state_machine.md +++ b/components/crow_alarm_panel/docs/arm_disarm_state_machine.md @@ -81,7 +81,7 @@ ARM_AWAY_PENDING / ARM_STAY_PENDING (KEY_ARM or KEY_STAY sent, waiting for Comma ├─ on Command(0x14) addressed to us │ └─> IDLE (sequence done; ARMED_STATE 0x11 follows independently) ├─ on timeout (>1s) -│ └─> IDLE (abort) +│ └─> retry with growing backoff, up to ARM_DISARM_MAX_RETRIES; then IDLE (abort) CODE_DIGIT_PENDING (a code digit sent, waiting for Command) ├─ on first Command(0x14) addressed to us in this sequence @@ -95,7 +95,7 @@ CODE_DIGIT_PENDING (a code digit sent, waiting for Command) │ └─> CODE_DIGIT_PENDING (logged, not treated as an anomaly — see logs-18 below; next digit │ sent immediately as if byte[1] == baseline) ├─ on timeout (>1s) -│ └─> IDLE (abort) +│ └─> retry with growing backoff, up to ARM_DISARM_MAX_RETRIES; then IDLE (abort) CODE_ENTER_PENDING (terminal key sent, waiting for final Command) ├─ on Command(0x14) addressed to us @@ -506,6 +506,18 @@ blocking transmit; every resolution to IDLE (broadcast confirmation, ARM/STAY ac abort) bumps the token, so a key committed by a sequence that has since resolved is suppressed instead of typed into the panel. +The commit-to-transmit window needs the reverse guard too (`arm_disarm_key_in_flight_`): while +a key sits in the bus-idle wait, KEYPAD_COMMAND-driven transitions and the watchdog are gated. +A command arriving before the key transmits cannot be its acknowledgement — acting on it would +falsely complete `ARM_AWAY_PENDING`/`ARM_STAY_PENDING` (and, combined with the token, suppress +the never-sent arm key) or advance the digit sequence and emit digits out of order — and a +watchdog retry started from inside that wait would create a second waiter that transmits a +duplicate key once the bus goes idle. ARMED_STATE resolution is deliberately *not* gated: it is +the controller's own state, independent of our TX, and cancelling the in-flight key is exactly +what the token exists for. On transmit, the no-progress window (`arm_disarm_state_enter_ms_`) +restarts from the actual send, since the wait may have consumed the whole 1 s budget while the +watchdog was gated. + Compiles and passes `esphome config`/`esphome compile` against `crow_alarm_panel_test.yaml`. **Not yet validated on real hardware** — needs a fresh capture reproducing a multi-failure streak (ideally another degradation episode like 2026-08-19 17:09) to confirm the backoff From b9e78ff3dc5ac380fa0d883cb3a85e96003c4ed2 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sat, 22 Aug 2026 18:16:35 +1200 Subject: [PATCH 06/12] fix(crow_alarm_panel): enforce keypress state-machine mutual exclusion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three keypress state machines (output-select, arm/disarm, zone bypass) advance on the same addressed KEYPAD_COMMAND frames, so only one may run at a time — but only set_zone_bypass() enforced that. set_output() could start during an arm/disarm sequence (and vice versa), and the new multi-second retry backoff keeps arm_disarm_state_ non-IDLE long enough to make that overlap realistic; both machines would then emit keys and double-consume confirmations (Copilot review on PR #12). set_output(), arm_away(), arm_stay() and disarm() now reject starting while another machine is non-IDLE, matching set_zone_bypass(). Also rewrites the ARM_DISARM_MAX_RETRIES rationale: 'a timeout reliably means no state change' is disproven by the observed 825 ms boundary ack — retry safety actually comes from intent-matched ARMED_STATE resolution plus generation-token cancellation. Fixes two stale doc references (send_packet() vs arm_disarm_keypress_() in the retry description, and a below-vs-above section pointer) and a duplicated phrase. Co-Authored-By: Claude Fable 5 --- .../crow_alarm_panel/crow_alarm_panel.cpp | 30 +++++++++++++++++++ .../crow_alarm_panel/crow_alarm_panel.h | 11 ++++--- .../docs/arm_disarm_state_machine.md | 9 +++--- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index 2f71ce0..2fd6561 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -1072,6 +1072,13 @@ void CrowAlarmPanel::arm_away(const std::string &code) { ESP_LOGW(TAG, "arm_away: ARM/DISARM already in progress, ignoring"); return; } + // All three keypress state machines advance on the same KEYPAD_COMMAND frames; running + // two at once would double-consume confirmations. + if (this->output_select_state_ != OutputSelectState::IDLE || + this->zone_bypass_state_ != ZoneBypassState::IDLE) { + ESP_LOGW(TAG, "arm_away: another keypress sequence in progress, ignoring"); + return; + } if (!code.empty()) { ESP_LOGI(TAG, "Arm away (with code)"); this->start_code_sequence_(code, KEY_ARM); @@ -1094,6 +1101,13 @@ void CrowAlarmPanel::arm_stay(const std::string &code) { ESP_LOGW(TAG, "arm_stay: ARM/DISARM already in progress, ignoring"); return; } + // All three keypress state machines advance on the same KEYPAD_COMMAND frames; running + // two at once would double-consume confirmations. + if (this->output_select_state_ != OutputSelectState::IDLE || + this->zone_bypass_state_ != ZoneBypassState::IDLE) { + ESP_LOGW(TAG, "arm_stay: another keypress sequence in progress, ignoring"); + return; + } if (!code.empty()) { ESP_LOGI(TAG, "Arm stay (with code)"); this->start_code_sequence_(code, KEY_STAY); @@ -1120,6 +1134,13 @@ void CrowAlarmPanel::disarm(const std::string &code) { ESP_LOGW(TAG, "disarm: ARM/DISARM already in progress, ignoring"); return; } + // All three keypress state machines advance on the same KEYPAD_COMMAND frames; running + // two at once would double-consume confirmations. + if (this->output_select_state_ != OutputSelectState::IDLE || + this->zone_bypass_state_ != ZoneBypassState::IDLE) { + ESP_LOGW(TAG, "disarm: another keypress sequence in progress, ignoring"); + return; + } ESP_LOGI(TAG, "Disarm"); this->start_code_sequence_(code, KEY_ENTER); } @@ -1164,6 +1185,15 @@ void CrowAlarmPanel::set_output(uint8_t output, bool state) { ESP_LOGW(TAG, "set_output(%u, %s): output-select sequence already in progress", output, state ? "on" : "off"); return; } + // All three keypress state machines advance on the same KEYPAD_COMMAND frames; running + // two at once would double-consume confirmations. arm_disarm_state_ stays non-IDLE for the + // whole multi-second retry-backoff envelope, so this window is long enough to matter. + if (this->arm_disarm_state_ != ArmDisarmState::IDLE || + this->zone_bypass_state_ != ZoneBypassState::IDLE) { + ESP_LOGW(TAG, "set_output(%u, %s): another keypress sequence in progress, ignoring", output, + state ? "on" : "off"); + return; + } // Build digit queue consumed one per KEYPAD_COMMAND received after the ACK. this->output_select_keys_.clear(); if (output >= 10) { diff --git a/components/crow_alarm_panel/crow_alarm_panel.h b/components/crow_alarm_panel/crow_alarm_panel.h index 58f328d..4b025ff 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.h +++ b/components/crow_alarm_panel/crow_alarm_panel.h @@ -329,10 +329,13 @@ class CrowAlarmPanel : public Component { std::vector arm_disarm_code_digits_; // code digits consumed one per KEYPAD_COMMAND uint8_t arm_disarm_code_idx_{0}; uint8_t arm_disarm_terminal_key_{KEY_ENTER}; // KEY_ENTER (disarm), KEY_ARM or KEY_STAY (arm-with-code) - // A watchdog timeout here reliably means the panel's state did not change (see - // docs/arm_disarm_state_machine.md — multiple sessions with an independent monitor capture - // confirm no ARMED_STATE broadcast occurred around the timeout), so unlike output-select/ - // zone-bypass a blind retry can't undo a change that already landed. 6 with the growing + // A watchdog timeout here has almost always meant the panel's state did not change (multiple + // sessions with an independent monitor capture show no ARMED_STATE broadcast around the + // timeout), but retry safety does NOT rest on that: an 825 ms ack has been observed right at + // the 1 s boundary, so a confirmation can land after the watchdog fires. What makes retries + // safe — unlike output-select/zone-bypass — is intent-matched ARMED_STATE resolution (resolves + // from any non-IDLE state, including a backoff wait) plus arm_disarm_generation_ cancelling + // any in-flight key once resolved (see docs/arm_disarm_state_machine.md). 6 with the growing // backoff below spans the worst episode observed so far (HA log 2026-08-19 17:09: two // consecutive calls exhausted 5 back-to-back retries each, and a manual call ~29 s after the // first attempt succeeded — back-to-back retries burned the whole budget in ~7 s, well inside diff --git a/components/crow_alarm_panel/docs/arm_disarm_state_machine.md b/components/crow_alarm_panel/docs/arm_disarm_state_machine.md index 868fa46..21c79d4 100644 --- a/components/crow_alarm_panel/docs/arm_disarm_state_machine.md +++ b/components/crow_alarm_panel/docs/arm_disarm_state_machine.md @@ -462,7 +462,7 @@ in ~7 s (attempt + 1 s watchdog ≈ 1.5 s cadence), well inside a degradation ep evidently lasted tens of seconds — while every eventual success in the whole trace record (manual retries in logs-22/23/24/25/26, and this episode's `17:09:46` recovery) came after multi-second gaps. Hammering also adds bus contention, the prime suspect for the collision -failure mode (modes documented above). +failure mode (documented above). **Fix applied (1/2 — retry backoff):** the watchdog no longer restarts the sequence immediately on timeout. It schedules the retry after a growing pause (`ARM_DISARM_RETRY_BACKOFF_MS`: @@ -470,8 +470,9 @@ on timeout. It schedules the retry after a growing pause (`ARM_DISARM_RETRY_BACK retry envelope from ~7 s to ~40 s — spanning the ~29 s recovery observed on 2026-08-19. While the backoff is pending, `KEYPAD_COMMAND`s no longer advance the (dead) sequence — without that gate, a stray periodic command arriving mid-wait would type leftover digits into the panel -outside any attempt. `send_packet()`'s existing bus-idle wait covers TX timing when the retry -fires. The abort fallback (restore `last_confirmed_acp_state_`) is unchanged. +outside any attempt. When the retry fires, its keys go through `arm_disarm_keypress_()` — +the same bus-idle wait `send_packet()` uses, plus the generation check and in-flight gating +described in the review follow-up below — which covers TX timing. The abort fallback (restore `last_confirmed_acp_state_`) is unchanged. **Fix applied (2/2 — intent-matched ARMED_STATE resolution from any state):** two related holes in the previous `CODE_ENTER_PENDING`-only resolution, both made more likely by retries: @@ -537,6 +538,6 @@ settle it. - ARM/STAY/DISARM sequences are simpler than OUTPUT because there's no ACK handshake - No-code arm_away()/arm_stay() complete after a single KEYPAD_COMMAND (~100ms total) - Code-based sequences share the same `CODE_DIGIT_PENDING` / `CODE_ENTER_PENDING` states regardless of whether the operation is arm or disarm -- ARMED_STATE messages (0x11) are published independently regardless of `arm_disarm_state_` (entities always reflect them). As of 2026-08-22, an ARMED_STATE broadcast matching the request's intent resolves the sequence from **any** non-IDLE state (including a retry backoff wait) — see "Retry backoff + intent-matched ARMED_STATE resolution" below. `CODE_ENTER_PENDING` remains the only state that *requires* one to succeed (`ARM_AWAY_PENDING`/`ARM_STAY_PENDING` still resolve on their KEYPAD_COMMAND ack). +- ARMED_STATE messages (0x11) are published independently regardless of `arm_disarm_state_` (entities always reflect them). As of 2026-08-22, an ARMED_STATE broadcast matching the request's intent resolves the sequence from **any** non-IDLE state (including a retry backoff wait) — see "Retry backoff + intent-matched ARMED_STATE resolution" above. `CODE_ENTER_PENDING` remains the only state that *requires* one to succeed (`ARM_AWAY_PENDING`/`ARM_STAY_PENDING` still resolve on their KEYPAD_COMMAND ack). - All three keypads receive Command broadcasts during arming/disarming; only the originating keypad controls the sequence - `disarm()` also guards against calling when already disarmed — it returns early if `is_armed()` is false From 2f8d089942585bf2d829a857b034532803c1895e Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sat, 22 Aug 2026 19:45:00 +1200 Subject: [PATCH 07/12] fix(crow_alarm_panel): gate optimistic publishes on request acceptance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mutual-exclusion guards (b9e78ff) made rejected requests silent to their callers, which publish optimistic entity states unconditionally (Copilot review on PR #12): - CrowAlarmControlPanel::control() published ARMING/DISARMING after arm_away()/arm_stay()/disarm() even when the request was rejected — no sequence and no watchdog started, so nothing would ever move the entity out of the transitional state again (ESPHome's validate_() then rejects both future arm and disarm calls). - The output switch published the requested state after set_output() even when no output keys were sent, misreporting the output until the next controller broadcast. arm_away(), arm_stay(), disarm(), set_output() (and the shared start_code_sequence_()) now return whether the request was accepted, and both callers publish the optimistic state only on acceptance. Buttons ignore the result — they are stateless and the rejection is already logged. Also updates the loop() watchdog comment still asserting 'a timeout reliably means the panel's state did not change' (disproven by the 825 ms boundary ack; the header rationale was fixed in b9e78ff). Co-Authored-By: Claude Fable 5 --- .../crow_alarm_control_panel.cpp | 18 +++-- .../crow_alarm_panel/crow_alarm_panel.cpp | 77 ++++++++++--------- .../crow_alarm_panel/crow_alarm_panel.h | 17 ++-- .../switch/crow_alarm_panel_switch.cpp | 8 +- 4 files changed, 70 insertions(+), 50 deletions(-) diff --git a/components/crow_alarm_panel/crow_alarm_control_panel.cpp b/components/crow_alarm_panel/crow_alarm_control_panel.cpp index cd18991..6e86d59 100644 --- a/components/crow_alarm_panel/crow_alarm_control_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_control_panel.cpp @@ -27,8 +27,12 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel this->status_momentary_warning("Code required to arm", 2000); return; } - this->parent_->arm_away(code); - this->publish_state(alarm_control_panel::ACP_STATE_ARMING); + // Publish the optimistic transitional state only if the request was accepted — a + // rejected request starts no sequence and no watchdog, so nothing would ever move the + // entity out of ARMING again. + if (this->parent_->arm_away(code)) { + this->publish_state(alarm_control_panel::ACP_STATE_ARMING); + } break; } case alarm_control_panel::ACP_STATE_ARMED_HOME: { @@ -37,8 +41,9 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel this->status_momentary_warning("Code required to arm", 2000); return; } - this->parent_->arm_stay(code); - this->publish_state(alarm_control_panel::ACP_STATE_ARMING); + if (this->parent_->arm_stay(code)) { + this->publish_state(alarm_control_panel::ACP_STATE_ARMING); + } break; } case alarm_control_panel::ACP_STATE_DISARMED: { @@ -47,8 +52,9 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel this->status_momentary_warning("Code required to disarm", 2000); return; } - this->parent_->disarm(code); - this->publish_state(alarm_control_panel::ACP_STATE_DISARMING); + if (this->parent_->disarm(code)) { + this->publish_state(alarm_control_panel::ACP_STATE_DISARMING); + } break; } case alarm_control_panel::ACP_STATE_TRIGGERED: diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index 2fd6561..83ead83 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -944,7 +944,8 @@ void CrowAlarmPanel::loop() { // Arm/disarm watchdog: abort if any non-IDLE state exceeds 1s without progress. See // ARM_DISARM_MAX_RETRIES in crow_alarm_panel.h for why retrying here (unlike output-select/ - // zone-bypass above) is safe: a timeout reliably means the panel's state did not change. + // zone-bypass above) is safe: intent-matched ARMED_STATE resolution plus generation-token + // cancellation cover a confirmation landing after the watchdog fires. // Retries are paced by a growing backoff (ARM_DISARM_RETRY_BACKOFF_MS) instead of restarting // immediately: back-to-back retries burned the whole budget in ~7 s during a controller // degradation episode that outlasted them (HA log 2026-08-19 17:09 — see @@ -1039,7 +1040,7 @@ void CrowAlarmPanel::loop() { } } -void CrowAlarmPanel::start_code_sequence_(const std::string &code, uint8_t terminal_key) { +bool CrowAlarmPanel::start_code_sequence_(const std::string &code, uint8_t terminal_key) { this->arm_disarm_code_digits_.clear(); for (char c : code) { if (c >= '0' && c <= '9') { @@ -1048,7 +1049,7 @@ void CrowAlarmPanel::start_code_sequence_(const std::string &code, uint8_t termi } if (this->arm_disarm_code_digits_.empty()) { ESP_LOGW(TAG, "start_code_sequence_: empty or non-numeric code, ignoring"); - return; + return false; } ESP_LOGD(TAG, "Code sequence: %u digits, terminal key 0x%02X", this->arm_disarm_code_digits_.size(), terminal_key); @@ -1061,88 +1062,89 @@ void CrowAlarmPanel::start_code_sequence_(const std::string &code, uint8_t termi this->arm_disarm_retry_pending_ = false; this->arm_disarm_digit_ack_byte_set_ = false; this->arm_disarm_keypress_(this->arm_disarm_code_digits_[0]); + return true; } -void CrowAlarmPanel::arm_away(const std::string &code) { +bool CrowAlarmPanel::arm_away(const std::string &code) { if (!this->is_active_keypad()) { ESP_LOGW(TAG, "arm_away: passive monitor mode, ignoring"); - return; + return false; } if (this->arm_disarm_state_ != ArmDisarmState::IDLE) { ESP_LOGW(TAG, "arm_away: ARM/DISARM already in progress, ignoring"); - return; + return false; } // All three keypress state machines advance on the same KEYPAD_COMMAND frames; running // two at once would double-consume confirmations. if (this->output_select_state_ != OutputSelectState::IDLE || this->zone_bypass_state_ != ZoneBypassState::IDLE) { ESP_LOGW(TAG, "arm_away: another keypress sequence in progress, ignoring"); - return; + return false; } if (!code.empty()) { ESP_LOGI(TAG, "Arm away (with code)"); - this->start_code_sequence_(code, KEY_ARM); - } else { - ESP_LOGI(TAG, "Arm away"); - this->arm_disarm_state_ = ArmDisarmState::ARM_AWAY_PENDING; - this->arm_disarm_state_enter_ms_ = millis(); - this->arm_disarm_retry_count_ = 0; - this->arm_disarm_retry_pending_ = false; - this->arm_disarm_keypress_(KEY_ARM); + return this->start_code_sequence_(code, KEY_ARM); } + ESP_LOGI(TAG, "Arm away"); + this->arm_disarm_state_ = ArmDisarmState::ARM_AWAY_PENDING; + this->arm_disarm_state_enter_ms_ = millis(); + this->arm_disarm_retry_count_ = 0; + this->arm_disarm_retry_pending_ = false; + this->arm_disarm_keypress_(KEY_ARM); + return true; } -void CrowAlarmPanel::arm_stay(const std::string &code) { +bool CrowAlarmPanel::arm_stay(const std::string &code) { if (!this->is_active_keypad()) { ESP_LOGW(TAG, "arm_stay: passive monitor mode, ignoring"); - return; + return false; } if (this->arm_disarm_state_ != ArmDisarmState::IDLE) { ESP_LOGW(TAG, "arm_stay: ARM/DISARM already in progress, ignoring"); - return; + return false; } // All three keypress state machines advance on the same KEYPAD_COMMAND frames; running // two at once would double-consume confirmations. if (this->output_select_state_ != OutputSelectState::IDLE || this->zone_bypass_state_ != ZoneBypassState::IDLE) { ESP_LOGW(TAG, "arm_stay: another keypress sequence in progress, ignoring"); - return; + return false; } if (!code.empty()) { ESP_LOGI(TAG, "Arm stay (with code)"); - this->start_code_sequence_(code, KEY_STAY); - } else { - ESP_LOGI(TAG, "Arm stay"); - this->arm_disarm_state_ = ArmDisarmState::ARM_STAY_PENDING; - this->arm_disarm_state_enter_ms_ = millis(); - this->arm_disarm_retry_count_ = 0; - this->arm_disarm_retry_pending_ = false; - this->arm_disarm_keypress_(KEY_STAY); + return this->start_code_sequence_(code, KEY_STAY); } + ESP_LOGI(TAG, "Arm stay"); + this->arm_disarm_state_ = ArmDisarmState::ARM_STAY_PENDING; + this->arm_disarm_state_enter_ms_ = millis(); + this->arm_disarm_retry_count_ = 0; + this->arm_disarm_retry_pending_ = false; + this->arm_disarm_keypress_(KEY_STAY); + return true; } -void CrowAlarmPanel::disarm(const std::string &code) { +bool CrowAlarmPanel::disarm(const std::string &code) { if (!this->is_active_keypad()) { ESP_LOGW(TAG, "disarm: passive monitor mode, ignoring"); - return; + return false; } if (!this->is_armed()) { ESP_LOGW(TAG, "disarm: not armed, ignoring"); - return; + return false; } if (this->arm_disarm_state_ != ArmDisarmState::IDLE) { ESP_LOGW(TAG, "disarm: ARM/DISARM already in progress, ignoring"); - return; + return false; } // All three keypress state machines advance on the same KEYPAD_COMMAND frames; running // two at once would double-consume confirmations. if (this->output_select_state_ != OutputSelectState::IDLE || this->zone_bypass_state_ != ZoneBypassState::IDLE) { ESP_LOGW(TAG, "disarm: another keypress sequence in progress, ignoring"); - return; + return false; } ESP_LOGI(TAG, "Disarm"); - this->start_code_sequence_(code, KEY_ENTER); + return this->start_code_sequence_(code, KEY_ENTER); } void CrowAlarmPanel::keypress(uint8_t key) { @@ -1176,14 +1178,14 @@ void CrowAlarmPanel::arm_disarm_keypress_(uint8_t key) { this->arm_disarm_key_in_flight_ = false; } -void CrowAlarmPanel::set_output(uint8_t output, bool state) { +bool CrowAlarmPanel::set_output(uint8_t output, bool state) { if (!this->is_active_keypad()) { ESP_LOGW(TAG, "set_output(%u, %s): passive monitor mode, ignoring", output, state ? "on" : "off"); - return; + return false; } if (this->output_select_state_ != OutputSelectState::IDLE) { ESP_LOGW(TAG, "set_output(%u, %s): output-select sequence already in progress", output, state ? "on" : "off"); - return; + return false; } // All three keypress state machines advance on the same KEYPAD_COMMAND frames; running // two at once would double-consume confirmations. arm_disarm_state_ stays non-IDLE for the @@ -1192,7 +1194,7 @@ void CrowAlarmPanel::set_output(uint8_t output, bool state) { this->zone_bypass_state_ != ZoneBypassState::IDLE) { ESP_LOGW(TAG, "set_output(%u, %s): another keypress sequence in progress, ignoring", output, state ? "on" : "off"); - return; + return false; } // Build digit queue consumed one per KEYPAD_COMMAND received after the ACK. this->output_select_keys_.clear(); @@ -1210,6 +1212,7 @@ void CrowAlarmPanel::set_output(uint8_t output, bool state) { this->output_select_state_ = OutputSelectState::OUTPUT_PENDING; this->output_select_state_enter_ms_ = millis(); this->keypress(KEY_OUTPUT); + return true; } void CrowAlarmPanel::set_zone_bypass(uint8_t zone, bool state) { diff --git a/components/crow_alarm_panel/crow_alarm_panel.h b/components/crow_alarm_panel/crow_alarm_panel.h index 4b025ff..77652dc 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.h +++ b/components/crow_alarm_panel/crow_alarm_panel.h @@ -198,7 +198,11 @@ class CrowAlarmPanel : public Component { void dump_config() override; void loop() override; - void set_output(uint8_t output, bool state); + // Returns true if the request was accepted (sequence started); false if it was rejected + // (passive mode, or another keypress sequence in progress). Callers that publish an + // optimistic entity state must do so only on acceptance — a rejected request starts no + // sequence and no watchdog, so nothing would ever correct the optimistic publish. + bool set_output(uint8_t output, bool state); void set_zone_bypass(uint8_t zone, bool state); void set_clock_pin(InternalGPIOPin *clock) { this->clock_pin_ = clock; } @@ -251,9 +255,12 @@ class CrowAlarmPanel : public Component { } void register_alarm_control_panel(alarm_control_panel::AlarmControlPanel *acp) { this->alarm_control_panel_ = acp; } - void arm_away(const std::string &code = ""); - void arm_stay(const std::string &code = ""); - void disarm(const std::string &code); + // These return true if the request was accepted (sequence started); false if rejected + // (passive mode, invalid code, already/not armed, or another sequence in progress). See the + // acceptance note on set_output() — optimistic publishes must be gated on the result. + bool arm_away(const std::string &code = ""); + bool arm_stay(const std::string &code = ""); + bool disarm(const std::string &code); bool is_armed() const; Trigger> *get_on_message_trigger() const { return this->on_message_trigger_; } @@ -285,7 +292,7 @@ class CrowAlarmPanel : public Component { bool is_bus_idle_(); bool wait_for_bus_idle_(); void transmit_packet_(uint8_t type, const std::vector &data); - void start_code_sequence_(const std::string &code, uint8_t terminal_key); + bool start_code_sequence_(const std::string &code, uint8_t terminal_key); // keypress() variant for the arm/disarm state machine only: suppresses the transmit if the // sequence resolved while waiting for bus idle (see arm_disarm_generation_). void arm_disarm_keypress_(uint8_t key); diff --git a/components/crow_alarm_panel/switch/crow_alarm_panel_switch.cpp b/components/crow_alarm_panel/switch/crow_alarm_panel_switch.cpp index 66192de..0faead6 100644 --- a/components/crow_alarm_panel/switch/crow_alarm_panel_switch.cpp +++ b/components/crow_alarm_panel/switch/crow_alarm_panel_switch.cpp @@ -15,8 +15,12 @@ void CrowAlarmPanelOutputSwitch::write_state(bool state) { ESP_LOGE(TAG, "Parent not set, ignoring output switch command"); return; } - this->parent_->set_output(this->output_number_, state); - this->publish_state(state); + // Publish the requested state only if the sequence actually started — on rejection + // (passive mode, another keypress sequence running) no output keys are sent, so an + // unconditional publish would misreport the output until the next controller broadcast. + if (this->parent_->set_output(this->output_number_, state)) { + this->publish_state(state); + } } void CrowAlarmPanelOutputSwitch::dump_config() { From be5ed6019ec8596eed1a659618298c283dbd2a27 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sat, 22 Aug 2026 21:08:14 +1200 Subject: [PATCH 08/12] fix(crow_alarm_panel): keep synchronously confirmed state over ARMING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An accepted arm/disarm call can resolve inside the call itself: the bus-idle wait yields into loop(), where a matching ARMED_STATE broadcast publishes the confirmed terminal state and resolves the sequence to IDLE. control() then overwrote that confirmed state with the optimistic ARMING/DISARMING — and with the machine already IDLE, no watchdog remained to correct it (Copilot review on PR #12). control() now publishes the transitional state only when the request was accepted AND the sequence is still unresolved on return, via a new is_arm_disarm_in_progress() accessor. The synchronous-resolution path is the only way the machine can reach IDLE during the call (the KEYPAD_COMMAND transitions and the watchdog are both gated while the key is in flight), and it always publishes the confirmed state first, so skipping the optimistic publish is correct. Co-Authored-By: Claude Fable 5 --- .../crow_alarm_panel/crow_alarm_control_panel.cpp | 12 +++++++----- components/crow_alarm_panel/crow_alarm_panel.h | 6 ++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/components/crow_alarm_panel/crow_alarm_control_panel.cpp b/components/crow_alarm_panel/crow_alarm_control_panel.cpp index 6e86d59..ff8118a 100644 --- a/components/crow_alarm_panel/crow_alarm_control_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_control_panel.cpp @@ -27,10 +27,12 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel this->status_momentary_warning("Code required to arm", 2000); return; } - // Publish the optimistic transitional state only if the request was accepted — a + // Publish the optimistic transitional state only if the request was accepted (a // rejected request starts no sequence and no watchdog, so nothing would ever move the - // entity out of ARMING again. - if (this->parent_->arm_away(code)) { + // entity out of ARMING again) AND the sequence is still unresolved — the call can yield + // waiting for the bus, during which a matching ARMED_STATE broadcast may already have + // published the confirmed terminal state; overwriting that would strand the entity. + if (this->parent_->arm_away(code) && this->parent_->is_arm_disarm_in_progress()) { this->publish_state(alarm_control_panel::ACP_STATE_ARMING); } break; @@ -41,7 +43,7 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel this->status_momentary_warning("Code required to arm", 2000); return; } - if (this->parent_->arm_stay(code)) { + if (this->parent_->arm_stay(code) && this->parent_->is_arm_disarm_in_progress()) { this->publish_state(alarm_control_panel::ACP_STATE_ARMING); } break; @@ -52,7 +54,7 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel this->status_momentary_warning("Code required to disarm", 2000); return; } - if (this->parent_->disarm(code)) { + if (this->parent_->disarm(code) && this->parent_->is_arm_disarm_in_progress()) { this->publish_state(alarm_control_panel::ACP_STATE_DISARMING); } break; diff --git a/components/crow_alarm_panel/crow_alarm_panel.h b/components/crow_alarm_panel/crow_alarm_panel.h index 77652dc..31e39ce 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.h +++ b/components/crow_alarm_panel/crow_alarm_panel.h @@ -262,6 +262,12 @@ class CrowAlarmPanel : public Component { bool arm_stay(const std::string &code = ""); bool disarm(const std::string &code); bool is_armed() const; + // True while an arm/disarm sequence is unresolved. An accepted request can resolve inside + // the call itself (the bus-idle wait yields into loop(), where a matching ARMED_STATE + // broadcast publishes the confirmed state and resolves to IDLE), so callers must also check + // this before publishing an optimistic transitional state — publishing after a synchronous + // resolution would overwrite the confirmed state with no watchdog left to fix it. + bool is_arm_disarm_in_progress() const { return this->arm_disarm_state_ != ArmDisarmState::IDLE; } Trigger> *get_on_message_trigger() const { return this->on_message_trigger_; } From 372c3e92cc695069b84f4b63409d88d2b6fe4f50 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sun, 23 Aug 2026 08:36:05 +1200 Subject: [PATCH 09/12] fix(crow_alarm_panel): identify arm/disarm ops by generation token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The is_arm_disarm_in_progress() gate (be5ed60) is a global flag: after an accepted call's operation resolved during its bus-idle yield, a different arm/disarm operation could start re-entrantly (e.g. an on_message automation) before the original stack resumed, making the flag true for the newer operation — the stale caller would then publish ARMING/DISARMING over the newer operation's state (Copilot review on PR #12). control() now captures arm_disarm_generation() before each call and publishes only if it is unchanged after an accepted call. The counter bumps on every resolution to IDLE, and a new operation can only start after the previous one resolved, so an unchanged token proves this call's operation is still the active one — covering both the synchronous-resolution case (be5ed60's target) and the re-entrant replacement case. The in-progress accessor is replaced by the generation accessor. Co-Authored-By: Claude Fable 5 --- .../crow_alarm_control_panel.cpp | 16 ++++++++++------ components/crow_alarm_panel/crow_alarm_panel.h | 16 ++++++++++------ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/components/crow_alarm_panel/crow_alarm_control_panel.cpp b/components/crow_alarm_panel/crow_alarm_control_panel.cpp index ff8118a..6e9ef33 100644 --- a/components/crow_alarm_panel/crow_alarm_control_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_control_panel.cpp @@ -29,10 +29,12 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel } // Publish the optimistic transitional state only if the request was accepted (a // rejected request starts no sequence and no watchdog, so nothing would ever move the - // entity out of ARMING again) AND the sequence is still unresolved — the call can yield - // waiting for the bus, during which a matching ARMED_STATE broadcast may already have - // published the confirmed terminal state; overwriting that would strand the entity. - if (this->parent_->arm_away(code) && this->parent_->is_arm_disarm_in_progress()) { + // entity out of ARMING again) AND this call's operation is still the active one. The + // call can yield waiting for the bus, during which the operation may resolve (matching + // ARMED_STATE publishes the confirmed state) and a different operation may even start + // re-entrantly; an unchanged generation rules out both (see arm_disarm_generation()). + const uint32_t gen = this->parent_->arm_disarm_generation(); + if (this->parent_->arm_away(code) && this->parent_->arm_disarm_generation() == gen) { this->publish_state(alarm_control_panel::ACP_STATE_ARMING); } break; @@ -43,7 +45,8 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel this->status_momentary_warning("Code required to arm", 2000); return; } - if (this->parent_->arm_stay(code) && this->parent_->is_arm_disarm_in_progress()) { + const uint32_t gen = this->parent_->arm_disarm_generation(); + if (this->parent_->arm_stay(code) && this->parent_->arm_disarm_generation() == gen) { this->publish_state(alarm_control_panel::ACP_STATE_ARMING); } break; @@ -54,7 +57,8 @@ void CrowAlarmControlPanel::control(const alarm_control_panel::AlarmControlPanel this->status_momentary_warning("Code required to disarm", 2000); return; } - if (this->parent_->disarm(code) && this->parent_->is_arm_disarm_in_progress()) { + const uint32_t gen = this->parent_->arm_disarm_generation(); + if (this->parent_->disarm(code) && this->parent_->arm_disarm_generation() == gen) { this->publish_state(alarm_control_panel::ACP_STATE_DISARMING); } break; diff --git a/components/crow_alarm_panel/crow_alarm_panel.h b/components/crow_alarm_panel/crow_alarm_panel.h index 31e39ce..cc546f1 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.h +++ b/components/crow_alarm_panel/crow_alarm_panel.h @@ -262,12 +262,16 @@ class CrowAlarmPanel : public Component { bool arm_stay(const std::string &code = ""); bool disarm(const std::string &code); bool is_armed() const; - // True while an arm/disarm sequence is unresolved. An accepted request can resolve inside - // the call itself (the bus-idle wait yields into loop(), where a matching ARMED_STATE - // broadcast publishes the confirmed state and resolves to IDLE), so callers must also check - // this before publishing an optimistic transitional state — publishing after a synchronous - // resolution would overwrite the confirmed state with no watchdog left to fix it. - bool is_arm_disarm_in_progress() const { return this->arm_disarm_state_ != ArmDisarmState::IDLE; } + // Identity token for arm/disarm operations, for callers that publish an optimistic + // transitional state: capture it BEFORE an arm/disarm call and publish only if it is + // unchanged after an accepted call. Every resolution to IDLE bumps the underlying counter, + // and a new operation can only start once the previous one has resolved, so an unchanged + // token proves the operation this call started is still the active one. Both hazards the + // calls' re-entrant bus-idle yield allows are covered: a synchronous resolution (matching + // ARMED_STATE publishes the confirmed state — bumped, don't overwrite it), and a newer + // operation started re-entrantly after that resolution (bumped by the resolution first — + // don't stomp the newer operation's state either). + uint32_t arm_disarm_generation() const { return this->arm_disarm_generation_; } Trigger> *get_on_message_trigger() const { return this->on_message_trigger_; } From 94f1dbed4886b9118f31bcd188ea7820058a9310 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sun, 23 Aug 2026 08:56:29 +1200 Subject: [PATCH 10/12] fix(crow_alarm_panel): drop stale timestamp write after digit keypress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit arm_disarm_keypress_() has refreshed arm_disarm_state_enter_ms_ on successful transmit since the in-flight gating change, so the assignment after the CODE_DIGIT_PENDING digit send was redundant on success — and harmful when stale: if the operation resolved during the call's bus-idle yield and a newer operation started re-entrantly, the resumed continuation overwrote the newer operation's watchdog/backoff timestamp, delaying or corrupting its retry schedule (Copilot review on PR #12). Co-Authored-By: Claude Fable 5 --- components/crow_alarm_panel/crow_alarm_panel.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index 83ead83..b0b2e60 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -697,8 +697,12 @@ void CrowAlarmPanel::loop() { uint8_t digit = this->arm_disarm_code_digits_[this->arm_disarm_code_idx_++]; ESP_LOGD(TAG, "Code sequence: sending digit %u (index %u)", digit, this->arm_disarm_code_idx_ - 1); + // No timestamp update here: arm_disarm_keypress_() already refreshes + // arm_disarm_state_enter_ms_ when the key actually transmits, and assigning + // after the call would be stale if the operation resolved (and a newer one + // started re-entrantly) during the yield — overwriting the newer operation's + // watchdog/backoff timestamp. this->arm_disarm_keypress_(digit); - this->arm_disarm_state_enter_ms_ = millis(); } else { ESP_LOGD(TAG, "Code sequence: sending terminal key 0x%02X", this->arm_disarm_terminal_key_); From 374ab5b274a5e8722d18d14fa08d07f7fd8a8b20 Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sun, 23 Aug 2026 14:36:11 +1200 Subject: [PATCH 11/12] fix(crow_alarm_panel): stop registration watchdog double-announce The 60s ping watchdog only checked last_ping_ms_, so a still-stale ping re-tripped it on the very next loop() pass right after a resend and fired a second announce ~1s later, before the controller had a chance to respond to the first. Gate re-trips on time since our own last announce too. --- components/crow_alarm_panel/crow_alarm_panel.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/components/crow_alarm_panel/crow_alarm_panel.cpp b/components/crow_alarm_panel/crow_alarm_panel.cpp index b0b2e60..61b5ce1 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -1027,8 +1027,13 @@ void CrowAlarmPanel::loop() { const uint32_t now_ms = millis(); // Watchdog: if we were being polled but haven't heard from the controller in 60 s, // re-send registration (handles controller resets where physical keypads re-register). + // Also gated on last_registration_announce_ms_: resending doesn't update last_ping_ms_, + // so without this a still-stale ping would re-trip the watchdog on the very next loop() + // pass and fire a second, usually unnecessary, announce ~1 s later (before the controller + // has had a chance to respond to the first one) instead of waiting the full 60 s. if (this->registration_sent_ && this->last_ping_ms_ != 0 && - (now_ms - this->last_ping_ms_) >= 60000) { + (now_ms - this->last_ping_ms_) >= 60000 && + (now_ms - this->last_registration_announce_ms_) >= 60000) { ESP_LOGW(TAG, "No ping for 60 s, re-sending registration announce"); this->registration_sent_ = false; } From 4a0d7d9e2b1ac70e6d7f7c246353258e04acee0e Mon Sep 17 00:00:00 2001 From: dan-s-github Date: Sun, 23 Aug 2026 14:36:18 +1200 Subject: [PATCH 12/12] docs(crow_alarm_panel): flag new unlabeled 0x91 packet (logs-34) Three occurrences, consistent poll-cycle slot, address-like leading byte not matching any active keypad. Meaning not established; logged so it's recognized and not confused with the ff/fe or 0x8A corruption signatures if seen again. --- .../docs/protocol_investigations.md | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/components/crow_alarm_panel/docs/protocol_investigations.md b/components/crow_alarm_panel/docs/protocol_investigations.md index 39fc98e..05898e8 100644 --- a/components/crow_alarm_panel/docs/protocol_investigations.md +++ b/components/crow_alarm_panel/docs/protocol_investigations.md @@ -402,3 +402,29 @@ This does not currently appear to cause the `CODE_ENTER_PENDING` arm/disarm fail **Assumption (unverified):** whether `0x8A` is itself a corrupted decode of something else (like `ff.`/`fe.`) or a legitimate-but-rare packet type is not established. Whether the corruption mechanism is the same across all three trigger contexts now observed (our own TX, registration storms, another keypad's physical arm) or several distinct mechanisms sharing a symptom is likewise still open. **Practical takeaway:** no code change proposed. The registration-storm correlation is now real evidence rather than speculation, but still not proven causal (which direction, if either, is cause vs. effect isn't established) — worth deliberately targeting in a future capture (e.g. watching for `ff.`/`fe.` bursts specifically around forced re-registrations). `0x8A` should be logged if seen again but not acted on. + +## `Unknown [91.]` — new unlabeled packet type + +**Source:** `traces/esphome-aap-alarm-interface-logs-34.txt`, a ~1h47m standard-debug-level session with four keypads live (`AAP`, `ESPHome`, `Control 4`, `IP`), otherwise unremarkable (normal disarmed-state zone activity, the already-documented `CURRENT_TIME` glitch, one controller reset with the usual re-registration signature). + +### Findings (observed facts) + +Three occurrences of an undocumented type `0x91`, not present in `crow_alarm_panel.h`'s known-type list: + +```sh +[13:02:52.261][D][crow_alarm_panel:885]: Unknown [91.82.01.46.81.00.80.11 (7)] +[13:09:52.193][D][crow_alarm_panel:885]: Unknown [91.83.01.46.81.00.80.11 (7)] +[13:55:21.897][D][crow_alarm_panel:885]: Unknown [91.82.01.46.81.00.80.11 (7)] +``` + +- Payload is identical across all three except the leading byte (`0x82` vs `0x83`), which doesn't match any of the four keypad addresses active on this bus (`0x00`/`0x05`/`0x06`/`0x07`), so — if it follows the usual `data[0] = address` convention seen in other packet types — it points at a fifth device not otherwise visible via `KEYPAD_PING`. +- All three land in the same relative slot in the ~15s poll cycle: immediately after the `AAP Keypad` (`0x00`) ping and before the `ESPHome Keypad` (`0x05`) ping, with spacing to its neighbors consistent with the other keypads' own ping cadence — i.e. it behaves like it occupies a normal polling slot rather than appearing as async/interrupt-style traffic. +- Timing between occurrences isn't a clean fixed period: `13:02:52` → `13:09:52` is almost exactly 7 minutes, but the third (`13:55:21`) is ~45.5 minutes after the second — not a small multiple of the first gap. Three samples is too few to characterize a period. + +### Assumption (unverified) + +Meaning is not established. Candidates not distinguished by this data: a status/keep-alive poll for a fifth bus device (e.g. a peripheral or wireless receiver module) at address `0x82`/`0x83`, or a corrupted decode of some other type (cf. the `0x8A` entry above, which was also seen only twice with inconsistent context). The consistent poll-cycle-slot positioning across all three occurrences argues mildly against pure decode corruption, but isn't conclusive. + +### Practical takeaway + +No code change proposed — `0x91` correctly falls through to the `default:` "Unknown" branch already, and nothing here indicates it needs special handling yet. Logged here so it's recognized (not mistaken for noise, and not confused with the `0xff`/`0xfe`/`0x8A` corruption signatures above) if seen again in a future capture; a repeat occurrence with a fixed period or a clearer address correlation would be the next useful data point.