car-key-killer/POTENTIAL_ISSUES.md

Potential bugs / edge cases (firmware + UI)

Scope: This file lists things that look potentially wrong, brittle, or surprising in the current codebase. None of these are confirmed failures on your hardware; they are review notes and future-hardening targets.

---

1. Capture / replay concurrency and timing

• Non-atomic shared counters between ISR and main loop

  • capIdx, capTransitions, and capLongRuns are updated in capRecordISR() and read from the main loop / OLED / HTTP handlers without any critical section.
  • On ESP32, aligned 32‑bit loads/stores are usually atomic, but you can still observe off‑by‑one or partially updated values when reading while the ISR is running.
  • Risk: Displayed progress / bitrate / squelch metrics can be slightly wrong or jittery while recording. Functionally low‑risk, but it’s technically a race.

• Capture analysis first bit — addressed: bit 0 is now included in the ones count.

• Bitrate assumptions vs real keyfob signals

  • Capture is hard‑wired at CAP_SAMPLE_HZ = 100000 (100 kHz) with a fixed 4‑second window.
  • Many car fobs run significantly faster than 10 kbit/s; very high data‑rate or very short packets can alias or barely fill the buffer before the squelch decides “signal caught”.
  • Risk: Certain high‑rate or exotic protocols may be captured with degraded timing or not detected by the long‑run squelch at all.

---

2. Interaction between jamming and capture / replay

• State restoration depends on capPrevJamming flag only

  • startCapture() and startReplay() store capPrevJamming = jammingEnabled before calling stopJamming(), and stopCapture() uses that flag to restart jamming.
  • If another part of the system toggles jammingEnabled while capture is in progress (e.g. a web API call), capPrevJamming can become stale and the final jamming state after STOP may not match user expectations.
  • Risk: Rare UX bug where jamming ends up on/off opposite to what the web UI last requested when you mix capture/replay and manual toggles aggressively.

• GDO0 direction flips vs noise ISR

  • noiseISR() drives both CC1101_*_GDO0 pins every 20 µs during jamming, but capture/replay reconfigures those same pins as input/output for bit‑banging.
  • The code tries to prevent overlap by calling stopJamming() before touching the capture timer and then restoring pin direction, but this depends on stopJamming() always fully killing s_noiseTimer first.
  • Risk: If stopJamming() ever early‑returns or is modified later, you could get noise ISR writes colliding with capture/replay GPIO direction changes. Right now it looks correct, but it’s a fragile area to touch.

---

3. Fast sweep / VCO cache (removed in current firmware)

Jamming is fixed dual-carrier (315 MHz + 433.92 MHz); buildSweepTable, tickSweepFast, and sweep tables are not present. The web “Apply Sweep” path still stores dwell/steps/span in NVS but does not affect jam TX.

If sweeps are reintroduced, restore bounded step counts, setFrequency error checks, and VCO cal timeouts.

---

3b. CC1101 (TI SWRS061) — fixed-carrier notes

• LO / PLL: Channel frequency is FREQ2:FREQ1:FREQ0 after RadioLib setFrequency. Large temperature swing can drift the VCO vs a trim-heavy fob RX; optional future work is periodic SCAL or re-init (not done here).
• DEVIAT: FM deviation for the LFSR async TX path is JAM_DEV_KHZ_R1_NARROW vs JAM_DEV_KHZ_R2_WIDE in config.h. If 315 MHz jam feels weak, raise R1 deviation toward R2.
• PATABLE: Explicit burst PATABLE is used for OOK replay only; jam uses direct async + RadioLib defaults unless you add more SPI.
• SPI / GDO0: Noise ISR only toggles GDO0 GPIOs; register SPI stays on the main thread — keep it that way when editing stopJamming / capture.

---

4. Timer usage and ISR safety

• Multiple hardware timers, no central ownership tracking

  • Timer 2 is used for noiseISR(); timer 3 is used for capRecordISR()/capReplayISR(). Each *_Start() tears down and re‑creates its timer instance.
  • There is no global check to prevent future code from reusing the same timer IDs for something else; reuse would race with the existing teardown, especially if done from another task.
  • Risk: Currently safe as long as no new timers are introduced. Future features must avoid timer IDs 2 and 3 or add a small timer allocation helper.

• GPIO driver calls from ISRs

  • noiseISR() and capReplayISR() call gpio_set_level() directly from IRAM ISRs.
  • On ESP32 the GPIO driver is generally ISR‑safe and IRAM‑resident, but this depends on IDF/Arduino internals. If the platform evolves or gets misconfigured (e.g. non‑IRAM gpio functions), these ISRs could start hitting flash and cause WDT resets under load.
  • Risk: Low on current IDF/Arduino, but this is one of the first places to check if you ever see random WDT resets under heavy jamming.

---

5. Web UI / HTTP handlers

• Log text is built with String and served as a big blob

  • getLogsText() builds a single large String (reserve(4096)) and returns it for /log downloads.
  • On its own this is fine, but if log lines become much longer than expected or you ever increase LOG_LINES, the 4 KB reserve may under‑estimate and cause heap fragmentation again.
  • Risk: Potential future fragmentation if log length grows substantially; currently appears safe with short 100‑line logs.

• AP password is a hard‑coded weak string

  • WIFI_AP_PASS is literally "password".
  • Risk: Anyone in RF range can connect to the AP and control the jammer UI. For a lab toy this is fine; for anything outside a controlled environment this is a security hole.

---

6. OLED and rotary encoder

• Encoder ISR uses digitalRead() twice per detent

  • encISR() calls digitalRead(ENC_CLK_PIN) and digitalRead(ENC_DT_PIN) directly; those are relatively slow, and they’re called from an ISR.
  • Risk: Under high interrupt storm (very fast dial spins) you could see jitter or missed ticks. This is more of a performance nit than a hard bug, but it’s the weak point of the input path.

• Notifications can delay page auto‑advance longer than expected

  • oledNotify() sets oledPageMs = notifEnd, and the auto‑advance check uses now > notifEnd && now - oledPageMs >= 8000.
  • After a long notification (e.g. multiple back‑to‑back events), page cycling waits an extra 8 seconds after the last notification before moving again.
  • Risk: UX oddity where pages seem “stuck” on status after a burst of notifications; not a functional bug.

---

7. Miscellaneous assumptions

• Radio init retries are hard‑coded to 3 attempts

  • startJamming() retries radio.begin(...) up to 3 times with 50 ms between attempts.
  • Risk: If a board needs a longer warm‑up (slow 3V3 rail, bad caps), you might hit a permanent “Init failed” state when a slightly longer retry or backoff would have recovered.

• Power math in health screen clips at 9999 mW

  • Effective mW is computed from dBm and then clamped at 9999: any higher values silently display XXXX / 9999mW style numbers.
  • Risk: Pure cosmetic; if you ever configured absurd gain values in the UI the display no longer reflects the math exactly.