fix(crow_alarm_panel): address Copilot review comments on PR #10

- Refresh arm_disarm_state_enter_ms_ before keypress() in the retry
  block, not after: keypress()->send_packet() can yield and re-enter
  loop(), so the watchdog could otherwise fire again on the stale
  timestamp and send extra keypresses (same fix already applied to the
  output-select retry).
- Gate the ISR's bit-trace buffer swap on !bit_trace_ready_ so it can't
  overwrite bit_trace_buffer2_ while loop() (other core) is still
  copying it out.
- Mark bit_trace_enabled_ volatile, matching ack_pending_/
  is_transmitting_.
- Reset bit_trace_len_/bit_trace_ready_ on every toggle so disabling
  mid-buffer and re-enabling can't mix stale bits into a trace chunk.

Author: dan-s-github

components/crow_alarm_panel/crow_alarm_panel.cpp

@@ -150,7 +150,12 @@ void IRAM_ATTR HOT CrowAlarmPanelStore::interrupt(CrowAlarmPanelStore *arg) {
   // Check for boundary
   arg->boundary_buffer_ = (uint8_t) ((arg->boundary_buffer_ << 1) | data_bit);
 
-  if (arg->bit_trace_enabled_) {
+  // Gated on !bit_trace_ready_ so bit_trace_buffer2_ stays untouched while loop() (running on
+  // the other core) is still copying it out — loop()'s InterruptLock only protects against
+  // same-core reentrancy, not this ISR on the other core, so the flag itself is what prevents
+  // the overwrite. Bits are simply dropped while the consumer is behind; accumulation resumes
+  // as soon as it clears the flag (normally sub-millisecond later).
+  if (arg->bit_trace_enabled_ && !arg->bit_trace_ready_) {
     arg->bit_trace_buffer_[arg->bit_trace_len_++] = data_bit ? '1' : '0';
     if (arg->bit_trace_len_ >= BIT_TRACE_BUFFER_BITS) {
       memcpy(arg->bit_trace_buffer2_, arg->bit_trace_buffer_, BIT_TRACE_BUFFER_BITS);
@@ -898,6 +903,11 @@ void CrowAlarmPanel::loop() {
         ESP_LOGW(TAG, "Arm/disarm: timeout in state %u, retrying (%u/%u)",
                  static_cast<uint8_t>(this->arm_disarm_state_), 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).
+        this->arm_disarm_state_enter_ms_ = millis();
         switch (this->arm_disarm_state_) {
           case ArmDisarmState::ARM_AWAY_PENDING:
             this->keypress(KEY_ARM);
@@ -917,7 +927,6 @@ void CrowAlarmPanel::loop() {
           default:
             break;
         }
-        this->arm_disarm_state_enter_ms_ = millis();
       } else {
         ESP_LOGW(TAG, "Arm/disarm: timeout in state %u, aborting after %u retries",
                  static_cast<uint8_t>(this->arm_disarm_state_), this->arm_disarm_retry_count_);

components/crow_alarm_panel/crow_alarm_panel.h

@@ -172,7 +172,9 @@ class CrowAlarmPanelStore {
   // Independent of frame decoding, so mis-alignment/framing issues can be diagnosed
   // by hand from the literal bitstream instead of the already-decoded bytes.
   static const uint16_t BIT_TRACE_BUFFER_BITS = 128;
-  bool bit_trace_enabled_{false};
+  // Read in the ISR, written from the main loop via set_raw_bit_trace_enabled() — volatile like
+  // ack_pending_/is_transmitting_ above, for the same cross-core visibility reason.
+  volatile bool bit_trace_enabled_{false};
   char bit_trace_buffer_[BIT_TRACE_BUFFER_BITS + 1]{};
   char bit_trace_buffer2_[BIT_TRACE_BUFFER_BITS + 1]{};
   uint16_t bit_trace_len_{0};
@@ -264,8 +266,19 @@ class CrowAlarmPanel : public Component {
   // be toggled at runtime without recompiling with a higher logger level.
   void set_raw_frame_logging_enabled(bool enabled) { this->raw_frame_logging_enabled_ = enabled; }
 
-  // Enables the ISR-side raw bit trace (see CrowAlarmPanelStore::bit_trace_enabled_).
-  void set_raw_bit_trace_enabled(bool enabled) { this->store_.bit_trace_enabled_ = enabled; }
+  // Enables the ISR-side raw bit trace (see CrowAlarmPanelStore::bit_trace_enabled_). Disables
+  // first, then resets the in-progress buffer position and any pending-but-unconsumed batch
+  // before (re-)enabling, so a toggle never mixes bits captured before/after it into one trace
+  // chunk, and never emits a stale ready buffer left over from before the toggle.
+  void set_raw_bit_trace_enabled(bool enabled) {
+    this->store_.bit_trace_enabled_ = false;
+    {
+      InterruptLock lock;
+      this->store_.bit_trace_len_ = 0;
+      this->store_.bit_trace_ready_ = false;
+    }
+    this->store_.bit_trace_enabled_ = enabled;
+  }
 
  protected:
   CrowAlarmPanelKeypad find_keypad_(uint8_t address);