From 75d9ed8162d5ea7205ba8b6c34e873ab48c55741 Mon Sep 17 00:00:00 2001
From: drjones <drjones@MBook-Air.local>
Date: Thu, 12 Mar 2026 15:01:04 -0700
Subject: [PATCH] Fix firmware potential issues from code review

- encISR: now uses direct REG_READ instead of digitalRead for massive speedup
- capAnalyze: fixed off-by-one where bit 0 wasn't counted in duty cycle
- tickSweepFast: added step boundary guard (< 100)
- main loop/OLED: volatile capture counters (capIdx, capTransitions, capLongRuns) are now read inside noInterrupts()/interrupts() blocks to prevent race conditions

Made-with: Cursor
---
 src/main.cpp | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/src/main.cpp b/src/main.cpp
index 018042f..d1aaae0 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -112,11 +112,12 @@ static volatile int8_t  encDelta   = 0;   // +1 CW / -1 CCW per detent
 static          uint8_t encLastClk = HIGH;
 
 void IRAM_ATTR encISR() {
-  const uint8_t clk = digitalRead(ENC_CLK_PIN);
+  const uint32_t in = REG_READ(GPIO_IN_REG);
+  const uint8_t clk = (in >> ENC_CLK_PIN) & 1u;
   if (clk == encLastClk) return;           // filter glitch
   encLastClk = clk;
   if (clk == LOW) {                        // falling edge = one detent
-    encDelta += (digitalRead(ENC_DT_PIN) == HIGH) ? +1 : -1;
+    encDelta += ((in >> ENC_DT_PIN) & 1u) ? +1 : -1;
   }
 }
 
@@ -354,6 +355,7 @@ static String capAnalyze() {
 
   uint32_t ones = 0, transitions = 0;
   uint8_t  prev = (capBuf[0] >> 0) & 1u;
+  if (prev) ones++; // count bit 0
   for (uint32_t i = 1; i < capRecBits; i++) {
     const uint8_t b = (capBuf[i >> 3] >> (i & 7)) & 1u;
     if (b)        ones++;
@@ -694,13 +696,18 @@ static void oledDrawStatus() {
 
     // Progress bar for recording
     if (cm == CapMode::RECORDING) {
-      const uint32_t pct = capIdx * 100 / (CAP_BUF_BYTES * 8);
+      uint32_t currentIdx, currentTransitions;
+      noInterrupts();
+      currentIdx = capIdx;
+      currentTransitions = capTransitions;
+      interrupts();
+      const uint32_t pct = currentIdx * 100 / (CAP_BUF_BYTES * 8);
       u8g2.drawFrame(0, 26, 128, 5);
       u8g2.drawBox(0, 26, (uint8_t)(pct * 128 / 100), 5);
       snprintf(buf, sizeof(buf), "%lus / %us  %lu tr",
-               (unsigned long)(capIdx / CAP_SAMPLE_HZ),
+               (unsigned long)(currentIdx / CAP_SAMPLE_HZ),
                (unsigned)CAP_DURATION_S,
-               (unsigned long)capTransitions);
+               (unsigned long)currentTransitions);
     } else {
       // Replaying — show loop position
       const uint32_t pct = capRecBits ? capIdx * 100 / capRecBits : 0;
@@ -1744,6 +1751,8 @@ static void handleCaptureReplay() {
 static void handleCaptureStatus() {
   static const char* const modeStr[] = {"idle","recording","recorded","replaying"};
   const uint8_t m = (uint8_t)capMode;
+  uint32_t currentIdx;
+  noInterrupts(); currentIdx = capIdx; interrupts();
   static char buf[512];
   int n = snprintf(buf, sizeof(buf),
     "{\"mode\":%u,\"mode_str\":\"%s\","
@@ -1751,10 +1760,10 @@ static void handleCaptureStatus() {
     "\"pct\":%lu,\"freq\":%.3f",
     (unsigned)m,
     modeStr[m < 4 ? m : 0],
-    (unsigned long)capIdx,
+    (unsigned long)currentIdx,
     (unsigned long)(CAP_BUF_BYTES * 8),
     (unsigned long)capRecBits,
-    (unsigned long)(capIdx * 100UL / (CAP_BUF_BYTES * 8)),
+    (unsigned long)(currentIdx * 100UL / (CAP_BUF_BYTES * 8)),
     (double)capFreq);
 
   if (capMode == CapMode::RECORDED || capMode == CapMode::REPLAYING) {
@@ -2003,6 +2012,8 @@ static void tickSweepFast(uint8_t csPin, uint8_t& step, uint8_t steps,
   // Jump to IDLE to safely change registers
   spiStrobe(csPin, 0x36); // SIDLE
   
+  if (step >= 100) step = 0; // bounds check
+
   // Write cached FREQ registers (0x0D, 0x0E, 0x0F)
   spiWriteReg(csPin, 0x0D, table[step].freqRegs[0]);
   spiWriteReg(csPin, 0x0E, table[step].freqRegs[1]);
@@ -2039,15 +2050,24 @@ void loop() {
   // Signal-present detection: fire OLED "SIGNAL!" once per session when
   // the ISR has seen enough stable bits to indicate a real RF burst.
   // capLongRuns > 10 filters out thermal noise which transitions almost constantly.
-  if (capMode == CapMode::RECORDING && !capSigNotified && capLongRuns > 10) {
+  uint32_t currentLongRuns;
+  noInterrupts();
+  currentLongRuns = capLongRuns;
+  interrupts();
+  
+  if (capMode == CapMode::RECORDING && !capSigNotified && currentLongRuns > 10) {
     capSigNotified = true;
+    uint32_t currentIdx;
+    noInterrupts(); currentIdx = capIdx; interrupts();
     oledNotify("SIGNAL!", "CAUGHT -- PRESS STOP", 3000);
-    logLine("[CAP] Signal detected: " + String(capLongRuns) + " valid symbols @ bit " + String(capIdx));
+    logLine("[CAP] Signal detected: " + String(currentLongRuns) + " valid symbols @ bit " + String(currentIdx));
   }
 
   if (capMode == CapMode::RECORDING && capBufFull) {
     capTimerStop();
+    noInterrupts();
     capRecBits = capIdx;
+    interrupts();
     capBufFull = false;
     capMode    = CapMode::RECORDED;
     gpio_set_direction(capGdoPin, GPIO_MODE_OUTPUT);