diff --git a/components/crow_alarm_panel/crow_alarm_control_panel.cpp b/components/crow_alarm_panel/crow_alarm_control_panel.cpp index cd18991..6e9ef33 100644 --- a/components/crow_alarm_panel/crow_alarm_control_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_control_panel.cpp @@ -27,8 +27,16 @@ 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) 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; } case alarm_control_panel::ACP_STATE_ARMED_HOME: { @@ -37,8 +45,10 @@ 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); + 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; } case alarm_control_panel::ACP_STATE_DISARMED: { @@ -47,8 +57,10 @@ 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); + 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; } 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 e059ee6..61b5ce1 100644 --- a/components/crow_alarm_panel/crow_alarm_panel.cpp +++ b/components/crow_alarm_panel/crow_alarm_panel.cpp @@ -458,16 +458,40 @@ 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_generation_++; + 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 +656,81 @@ 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). 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: + 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; + 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); + // 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); + } else { + ESP_LOGD(TAG, "Code sequence: sending terminal key 0x%02X", + 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->arm_disarm_keypress_(this->arm_disarm_terminal_key_); + } + 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. @@ -905,26 +948,34 @@ 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. - if (this->arm_disarm_state_ != ArmDisarmState::IDLE) { + // 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 + // arm_disarm_state_machine.md), while a manual call ~8 s after the abort succeeded first try. + // 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 (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: - 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: @@ -933,18 +984,30 @@ 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; } + } + } 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, (unsigned) 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_); 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; + 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, @@ -964,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; } @@ -981,7 +1049,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') { @@ -990,7 +1058,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); @@ -1000,81 +1068,142 @@ 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]); + 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 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->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 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->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 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) { this->send_packet(KEYPRESS, {key}); } -void CrowAlarmPanel::set_output(uint8_t output, bool state) { +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). + // 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; +} + +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 + // 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 false; } // Build digit queue consumed one per KEYPAD_COMMAND received after the ACK. this->output_select_keys_.clear(); @@ -1092,6 +1221,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) { @@ -1162,22 +1292,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 1617a9d..cc546f1 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,10 +255,23 @@ 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; + // 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_; } @@ -283,7 +300,12 @@ class CrowAlarmPanel : public Component { protected: CrowAlarmPanelKeypad find_keypad_(uint8_t address); bool is_bus_idle_(); - void start_code_sequence_(const std::string &code, uint8_t terminal_key); + bool wait_for_bus_idle_(); + void transmit_packet_(uint8_t type, const std::vector &data); + 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); void send_packet_blocking_(const std::vector &packet); bool wait_for_clock_edge_(bool wait_for_state, uint32_t timeout_us); @@ -324,19 +346,53 @@ 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. 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; + // 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 + // 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" // 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}; + // 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 8668310..21c79d4 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,21 +95,22 @@ 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 │ └─> 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,103 @@ 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 (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. 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: + +- 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). + +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. + +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 +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" 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 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. 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() {