Repository navigation
Arm/disarm retry pacing, frame-FIFO fix, and protocol doc updates - #15
Merged
Merged
Conversation
- Fix the 0x91 update to use the logger's actual UTC timestamps (the logger stamps UTC while the host runs NZST; an earlier pass built its "24h" cutoff from host-local time and undercounted). - Show 0x91 shares its payload/addressing/poll-slot behavior with four other type bytes (0x93/0x13/0x83/0xa3), each a 1-2 bit flip from 0x93 - likely one packet type with type-byte corruption, not five distinct unlabeled types. - Note this session's Unknown [ff.]/[fe.] volume (570/24h) is far above any previously documented session, and isn't gated by the CURRENT_TIME day/month-stuck state below. - Document a new persistent CURRENT_TIME failure mode: day/month stuck at 31/16 continuously for 18+ hours (vs. previously characterized low-rate/short-burst behavior), still ongoing as of the last check. - Add a field-reported pattern to arm_disarm_state_machine.md: disarm right after arming is reliable, but disarm after hours-long armed periods hits the full retry sequence - corroborated by one paired example, and cross-linked to the CURRENT_TIME finding as a candidate shared cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136wm9ud7ja7fkPGxXh79jw
Replace the single buffer2/data_length RX mailbox with a 4-slot frame FIFO (ISR producer, loop() consumer, lock-free via slot ownership). The old scheme overwrote an unread frame unconditionally, so a loop() pass stalled past one inter-frame gap — e.g. by the raw-bit-trace ESP_LOGI — silently lost frames, including KEYPAD_PINGs, tripping the 60s registration watchdog (~15x rate increase observed while raw trace was enabled; see protocol_investigations.md 2026-09-02). Hardware ACK behavior is unchanged in every case, including queue-full, which still ACKs like the old overwrite path did — withholding it would trigger the controller's rapid-retransmission mode (logs-11/36). Also fixes the pre-existing cross-core torn read of buffer2 (InterruptLock is core-local and never protected the handoff). Two counters make the loss hypothesis measurable in production: frame_backlog_events_ (frame completed while one still queued — exactly the old scheme's silent-loss case; DEBUG on change) and frame_overflow_events_ (actual drop at 4-deep backlog; WARN, expected 0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N5o5aaJZifFjdA8mYtYhRM
Three long-term-logger findings (frigate capture, UTC timestamps): - CURRENT_TIME 31/16 stuck state recurred ~4h after the 2026-08-31 OTA reboot, then self-cleared within ~1h24m with no further reboot — argues panel-side, but not a simple stuck-until-power-cycle latch. - Registration-watchdog double-announce fix (374ab5b) confirmed in ~22h of production traffic: zero duplicate announces vs dozens pre-OTA. - New 2026-09-02 section: enabling the raw bit trace correlates with a ~15x jump in 60s-no-ping watchdog trips (0.24/h -> 3.6/h at the exact enable boundary), while ff/fe corruption stayed at baseline. Suspected mechanism: loop() stalled by trace logging losing frames to the old single-buffer RX mailbox overwrite. Records why a skip-guard was rejected and documents the frame-FIFO fix (previous commit) plus the counter signature that will confirm or refute the hypothesis after the next OTA. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N5o5aaJZifFjdA8mYtYhRM
Frigate captures through 2026-09-05: pin the frame-FIFO fix's remaining watchdog trips to Unknown [ff.]/[fe.] bursts landing in our own poll slot, narrow the logs-35 double-halving case to a recurring fixed corrupted month value, and log two more instant no-retry disarms plus the five-day arm/disarm and CURRENT_TIME analyses from the prior session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UuwHMPJUCtttD8tfkDsv3C
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011PGafdP2HQA97YpYDPewnT
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011PGafdP2HQA97YpYDPewnT
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011PGafdP2HQA97YpYDPewnT
There was a problem hiding this comment.
🟡 Changes recommended
The documentation includes a future-dated update header (2026-09-06) that should be corrected to avoid confusing chronology.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves the Crow Alarm Panel external component by making frame reception more robust under heavy loop() load (replacing the single completed-frame mailbox with a small FIFO), and by extending/clarifying the protocol investigation documentation (including redacting a previously committed sensitive value).
Changes:
- Replace the ISR→
loop()completed-frame handoff with a 4-slot lock-free frame FIFO and add backlog/overflow diagnostic counters. - Expand protocol investigation notes with additional long-term capture findings and correlations.
- Update the arm/disarm state machine documentation to remove a plaintext sensitive value while preserving the rationale.
File summaries
| File | Description |
|---|---|
| components/crow_alarm_panel/docs/protocol_investigations.md | Adds long-term capture findings and updated analyses/correlations. |
| components/crow_alarm_panel/docs/arm_disarm_state_machine.md | Redacts a sensitive value and adds arm/disarm correlation notes from field data. |
| components/crow_alarm_panel/crow_alarm_panel.h | Introduces the ISR→loop() frame FIFO fields and diagnostic counter state. |
| components/crow_alarm_panel/crow_alarm_panel.cpp | Implements the frame FIFO producer/consumer logic and emits diagnostic logs on counter changes. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
b3fb2f2..372c3e9).374ab5b).loop()can't dropKEYPAD_PINGframes under raw-trace load (7e8044f), replacing the old single-buffer RX mailbox.arm_disarm_state_machine.md.Test plan
uv run esphome config crow_alarm_panel_test.yamluv run esphome compile crow_alarm_panel_test.yamlhttps://claude.ai/code/session_011PGafdP2HQA97YpYDPewnT