fix(crow_alarm_panel): pace arm/disarm retries, match ARMED_STATE intent

[dan-s-github]

Summary

• Back off arm/disarm retries (1/2/3/5/8/13 s, 6 attempts) instead of restarting immediately, so one disarm()/arm_away()/arm_stay() call can ride out a multi-second controller degradation episode instead of exhausting its whole retry budget in ~7 s. Motivated by traces/home-assistant_2026-08-19T08-03-09.331Z.log, where two consecutive disarm calls each burned all 5 old-style retries in ~10 s during an episode where the controller later stopped pinging and both physical keypads re-registered.
• Resolve on an ARMED_STATE broadcast from any non-IDLE arm/disarm state (not just CODE_ENTER_PENDING), but only when it matches the request's intent (Disarmed for disarm; Arming/Armed Away for arm). Closes two latent hazards made more likely by longer retry windows: a late broadcast landing mid-retry could previously let a disarm retry keep typing code+ENTER into an already-disarmed panel (re-arming it), and a registration-handshake re-announce of the current state could previously false-positive a disarm as complete.
• Fix a -Wformat warning (uint32_t/%u mismatch) on the new backoff log line, verified against both the repo's pinned esphome 2026.7.2 and an isolated esphome 2026.8.0 install.
• docs/arm_disarm_state_machine.md updated with the full trace analysis and rationale for both changes.

Test plan

• uv run esphome config crow_alarm_panel_test.yaml
• uv run esphome compile crow_alarm_panel_test.yaml (esphome 2026.7.2, pinned) — clean, no warnings
• Isolated esphome==2026.8.0 compile of the same fixture — clean, no warnings
• Real-hardware validation of the retry-backoff/intent-match behavior (not yet captured — see "Not yet validated on real hardware" note in the doc update)

[Copilot]

🟡 Changes recommended

Terminal-key handling has a re-entrancy race that can resurrect a completed sequence and access an empty code vector.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds paced arm/disarm retries and intent-matched controller confirmations.

Changes:

• Adds growing retry backoff with six retries.
• Resolves requests from matching ARMED_STATE broadcasts.
• Documents trace evidence and behavior.

File summaries

File	Description
components/crow_alarm_panel/crow_alarm_panel.cpp	Implements retry pacing and intent matching.
components/crow_alarm_panel/crow_alarm_panel.h	Defines retry state and backoff schedule.
components/crow_alarm_panel/docs/arm_disarm_state_machine.md	Documents rationale and state-machine changes.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[dan-s-github]

Fixed in 00a7d1f — CODE_ENTER_PENDING and its timestamp are now set before the terminal keypress(), matching the ordering the other send paths use, so a matching ARMED_STATE broadcast arriving during the send's yield can no longer be overwritten and the watchdog retry can't index the cleared code vector.

[Copilot]

🟡 Changes recommended

An in-flight keypress can still transmit after a matching broadcast resolves the operation, preserving the re-arming hazard.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

components/crow_alarm_panel/docs/arm_disarm_state_machine.md:110

• This updates only the CODE_ENTER_PENDING timeout transition, but the watchdog retries every non-IDLE arm/disarm state. The diagram still says ARM_AWAY_PENDING/ARM_STAY_PENDING and CODE_DIGIT_PENDING timeouts abort directly to IDLE at lines 83–84 and 97–98; update those transitions to show the same backoff/retry behavior.

├─ on timeout (>1s)
│  └─> retry with growing backoff, up to ARM_DISARM_MAX_RETRIES; then IDLE (abort; failure)

components/crow_alarm_panel/crow_alarm_panel.h:335

• ARM_DISARM_MAX_RETRIES = 6 produces seven total attempts: the initial attempt plus six retries, as the six backoff slots also imply. The PR Summary says “6 attempts”; update it to “6 retries / 7 attempts” so the advertised behavior matches the implementation and the documented ~40-second envelope.

  static const uint8_t ARM_DISARM_MAX_RETRIES = 6;

components/crow_alarm_panel/docs/arm_disarm_state_machine.md:465

• Remove the duplicated singular/plural wording in “failure mode (modes documented above).”

multi-second gaps. Hammering also adds bus contention, the prime suspect for the collision
failure mode (modes documented above).

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[dan-s-github]

Fixed in ad453ec. Arm/disarm keys now go through a cancellable send path: arm_disarm_keypress_() captures a generation token (arm_disarm_generation_) before the bus-idle wait and re-checks it immediately before the blocking transmit. Every resolution to IDLE (matching ARMED_STATE broadcast, ARM/STAY ack, watchdog abort) bumps the token, so a key committed by a sequence that resolved during the wait is suppressed instead of transmitted. send_packet() was split into wait_for_bus_idle_() + transmit_packet_() so both paths share the same wait/transmit code; other send paths are unchanged.

[Copilot]

🟡 Changes recommended

Re-entrant bus waits can currently drop or duplicate arm/disarm keypresses.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

components/crow_alarm_panel/docs/arm_disarm_state_machine.md:110

• The diagram now documents retries only for CODE_ENTER_PENDING, but the watchdog retries every non-IDLE state. The timeout branches for ARM_AWAY_PENDING/ARM_STAY_PENDING (lines 83–84) and CODE_DIGIT_PENDING (lines 97–98) still say they abort directly, so the state-machine reference remains contradictory. Update those branches to show the same backoff/retry transition.

├─ on timeout (>1s)
│  └─> retry with growing backoff, up to ARM_DISARM_MAX_RETRIES; then IDLE (abort; failure)

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Balanced

[dan-s-github]

Fixed in ff3e998 via the gating option: arm_disarm_key_in_flight_ is true from commit to transmit, and the KEYPAD_COMMAND-driven state machine transitions are skipped while it's set — a command arriving during the bus-idle wait can no longer falsely complete ARM_*_PENDING (or advance CODE_DIGIT_PENDING out of order), so the pending key is no longer suppressed by its own pre-transmit window. ARMED_STATE resolution stays ungated deliberately: it's the controller's own state, independent of our TX, and cancelling the in-flight key is exactly what the generation token is for.

[dan-s-github]

Fixed in ff3e998. Rather than a per-send generation, the watchdog is now gated while arm_disarm_key_in_flight_ is set — a retry can no longer start from inside an earlier keypress's bus-idle wait, so two concurrent waiters can't exist (the CMD-driven transitions are gated too, and new user requests are already blocked by the non-IDLE state guard). Timing out an attempt whose key hasn't transmitted yet was premature anyway; the no-progress window now restarts from the actual transmit.

[Copilot]

🟡 Changes recommended

Output-select can start during the extended backoff and interleave keys with the arm/disarm retry.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

components/crow_alarm_panel/docs/arm_disarm_state_machine.md:465

• The sentence contains a duplicated construction: “failure mode (modes documented above).”

failure mode (modes documented above).

components/crow_alarm_panel/docs/arm_disarm_state_machine.md:540

• This section is above the Notes section, so “below” points readers in the wrong direction.

- 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).

• Files reviewed: 3/3 changed files
• Comments generated: 3
• Review effort level: Balanced

[dan-s-github]

Fixed in b9e78ff. Every keypress-sequence entry point now enforces the mutual exclusion: set_output(), arm_away(), arm_stay() and disarm() reject starting while any other machine is non-IDLE, matching the check set_zone_bypass() already had. Since arm_disarm_state_ stays non-IDLE for the whole retry-backoff envelope, that covers the multi-second window this comment describes.

[dan-s-github]

Fixed in b9e78ff — the comment now states the observed facts (timeouts have almost always meant no state change, but an 825 ms boundary ack shows a confirmation can land after the watchdog fires) and attributes retry safety to intent-matched ARMED_STATE resolution plus the generation-token cancellation instead of the disproven timeout assumption.

[dan-s-github]

Fixed in b9e78ff — the retry description now says the keys go through arm_disarm_keypress_() (same bus-idle wait, plus the generation check and in-flight gating described in the follow-up section). Also fixed the two suppressed doc nits from this review (duplicated phrase, below-vs-above pointer).

[Copilot]

🟡 Changes recommended

Newly rejected overlapping operations can leave ESPHome entities reporting optimistic states for actions that never started.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

components/crow_alarm_panel/crow_alarm_panel.cpp:948

• The new backoff explanation still follows the assertion that a timeout “reliably means the panel's state did not change,” which this PR's late-confirmation analysis explicitly disproves. Retry safety now comes from intent matching and cancellation, so retaining the old rationale can lead future watchdog changes to remove those safeguards.

  // Retries are paced by a growing backoff (ARM_DISARM_RETRY_BACKOFF_MS) instead of restarting

components/crow_alarm_panel/crow_alarm_panel.cpp:1109

• This rejection is invisible to CrowAlarmControlPanel::control(), which unconditionally publishes ACP_STATE_ARMING after arm_stay() (crow_alarm_control_panel.cpp:40-41). When another keypress sequence is active, the arm-stay operation never starts and there is no watchdog to restore the optimistic state. Return an acceptance result and publish ARMING only when the parent accepted the request.

  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;

components/crow_alarm_panel/crow_alarm_panel.cpp:1142

• CrowAlarmControlPanel::control() publishes ACP_STATE_DISARMING after this void call regardless of this new rejection (crow_alarm_control_panel.cpp:50-51). A disarm request overlapping an output/bypass sequence therefore leaves the entity in DISARMING even though no disarm state machine or abort recovery is running. Return whether the request was accepted and condition the optimistic publish on that result.

  if (this->output_select_state_ != OutputSelectState::IDLE ||
      this->zone_bypass_state_ != ZoneBypassState::IDLE) {
    ESP_LOGW(TAG, "disarm: another keypress sequence in progress, ignoring");
    return;

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Balanced

[dan-s-github]

Fixed in 2f8d089. arm_away(), arm_stay() and disarm() (plus the shared start_code_sequence_()) now return whether the request was accepted, and control() publishes ACP_STATE_ARMING/DISARMING only on acceptance — a rejected request leaves the entity in its current state instead of stranding it in a transitional one. The buttons ignore the result deliberately: they're stateless and the rejection is already logged.

[dan-s-github]

Fixed in 2f8d089 — set_output() now returns whether the sequence started, and the output switch publishes the requested state only on acceptance; on rejection the entity keeps its last state (no restore needed since nothing was published).

[Copilot]

🔵 Needs a closer look

Synchronous confirmations can be overwritten by optimistic transitional states after arm/disarm calls return.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

components/crow_alarm_panel/crow_alarm_control_panel.cpp:35

• arm_away() may yield while waiting for an idle bus. A matching ARMED_STATE can therefore publish ARMED_AWAY and resolve the sequence before this call returns, after which this unconditional optimistic publish overwrites the confirmed state with ARMING and no watchdog remains to restore it. Preserve a terminal state that arrived synchronously during the call.

This issue also appears in the following locations of the same file:

• line 44
• line 55

      if (this->parent_->arm_away(code)) {
        this->publish_state(alarm_control_panel::ACP_STATE_ARMING);

components/crow_alarm_panel/crow_alarm_control_panel.cpp:45

• arm_stay() can process a matching ARMED_STATE during its bus-idle yield and return after the handler has already published ARMED_AWAY and resolved the sequence. Publishing ARMING afterward regresses the confirmed state permanently because the operation is already IDLE. Skip the optimistic publish when a terminal armed state arrived during the call.

      if (this->parent_->arm_stay(code)) {
        this->publish_state(alarm_control_panel::ACP_STATE_ARMING);

components/crow_alarm_panel/crow_alarm_control_panel.cpp:56

• disarm() may yield until a matching Disarmed broadcast publishes the confirmed state and resolves the sequence, yet it still returns true. This subsequent publish then overwrites DISARMED with DISARMING; because the machine is already IDLE, no retry/abort path corrects it. Keep the confirmed state when it arrived during the call.

      if (this->parent_->disarm(code)) {
        this->publish_state(alarm_control_panel::ACP_STATE_DISARMING);

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[dan-s-github]

Addressed the latest review's suppressed findings in be5ed60: control() now publishes the optimistic ARMING/DISARMING state only when the request was accepted and the sequence is still unresolved on return (new is_arm_disarm_in_progress() accessor). A matching ARMED_STATE broadcast landing during the call's bus-idle yield publishes the confirmed terminal state and resolves the machine to IDLE; that is the only way the machine can reach IDLE mid-call (KEYPAD_COMMAND transitions and the watchdog are gated while the key is in flight), so skipping the optimistic publish preserves the confirmed state.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Re-entrant waits can let stale control calls overwrite a newer operation’s state.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

components/crow_alarm_panel/crow_alarm_control_panel.cpp:47

• As above, the global in-progress flag does not prove that the active sequence is this arm_stay() request. Because the call yields, a resolved request can be replaced re-entrantly by a different operation before returning; this stale caller would then publish ARMING over the newer operation's state. Gate the publish with the identity/generation of the operation started by this call.

      if (this->parent_->arm_stay(code) && this->parent_->is_arm_disarm_in_progress()) {
        this->publish_state(alarm_control_panel::ACP_STATE_ARMING);

components/crow_alarm_panel/crow_alarm_control_panel.cpp:58

• A newer arm operation can make this global in-progress check true after this disarm already resolved during its bus-idle yield. The older control call would then publish DISARMING, overwriting the newer request's ARMING state. Return/capture an operation generation and only publish when that same disarm operation is still active.

      if (this->parent_->disarm(code) && this->parent_->is_arm_disarm_in_progress()) {
        this->publish_state(alarm_control_panel::ACP_STATE_DISARMING);

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[dan-s-github]

Fixed in 372c3e9 with the suggested generation approach. control() captures arm_disarm_generation() before each call and publishes the transitional state only if it's 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 the synchronous resolution and the re-entrant replacement (this comment, plus both suppressed variants for arm_stay/disarm). The global is_arm_disarm_in_progress() accessor is gone, replaced by the generation accessor.

[Copilot]

🔵 Needs a closer look

A stale re-entrant digit continuation can overwrite a newer operation’s retry timestamp.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

components/crow_alarm_panel/crow_alarm_panel.cpp:701

• arm_disarm_keypress_() can yield, resolve this operation, and let an on_message callback start a newer arm/disarm operation before returning. This stale continuation then overwrites the newer operation's watchdog/backoff timestamp. The helper already timestamps a successfully transmitted key, so remove this post-call assignment to avoid delaying or corrupting the newer operation's retry schedule.

                  this->arm_disarm_state_enter_ms_ = millis();

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[dan-s-github]

Addressed the latest review's suppressed finding in 94f1dbe: removed the arm_disarm_state_enter_ms_ assignment after the CODE_DIGIT_PENDING digit send. arm_disarm_keypress_() already refreshes the timestamp when the key actually transmits, so the post-call write was redundant on success and could overwrite a newer operation's watchdog/backoff timestamp when the continuation resumed stale.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

The re-entrant bus timing and retry changes require the planned real-hardware validation before final approval.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The unmentioned registration retry-cadence change needs explicit scope and validation in the PR description.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Balanced