cute-handshake-capture/handshake-capture-c6/MUST_DO_IMPROVEMENTS.md

Must-do improvements (firmware review)

Full pass over handshake-capture-c6: capture logic, SD, UI, display SPI, and main loop. Items below are simple in concept (some need careful implementation). Ordered by severity.

Status vs v1.0.3 (re-review)

#	Item	Status
1	Serialize shared SPI (LCD + SD)	Done — SPI_Bus_Lock.cpp recursive mutex; all LCD_Write* paths lock; LCD_addWindow wraps full flush; SD_Init / SD_WriteFileAtomic lock + SPIBus_SetDisplayPaused so Timer_Loop skips LVGL during SD.
2	Cap / document eapol_count	Done — increment only while eapol_count < HANDSHAKE_EAPOL_FRAMES; README notes heuristic.
3	2.4 GHz–only sweep	Done — isSupportedCaptureChannel, isCaptureable filters; list shows 5G+ + grey for off-band secure APs; status line off-band count.
4	Production vs debug serial	Done — HANDSHAKE_DEBUG (default 0) + HANDSHAKE_LOGF / HANDSHAKE_LOGLN; boot version line always.
5	USE_SOFTAP default	Done — default 0 in HandshakeCapture.h.
6	Long SD write / WDT / UI	Partially — yield() after large write in SD_WriteFileAtomic; WDT not explicitly fed (usually OK on Arduino main task).
7	Rescan vs network_index	Unchanged — still no mid-capture rescan; README covers behavior.
8	Magic numbers	Done — timing + channel range centralized in HandshakeCapture.h.

Remaining nits (optional follow-ups)

1. SD_WriteFileAtomic: On failure it sets g_sd_ready = false even for a single-file error (e.g. disk full, bad filename) while the card may still be mounted. Consider clearing g_sd_ready only on errors that imply unmount, or always pair with a defined recovery path.
2. SD_Card.cpp → HandshakeCapture.h: Include is only for log macros — a tiny HandshakeLog.h would remove coupling.
3. LCD_Reset: Still toggles CS without the SPI mutex (boot-only; low risk).

---

1. Serialize shared SPI (LCD + SD) — critical

What: ST7789 and the SD card share SPI (MOSI/SCLK/MISO; separate CS). LVGL flushes the display from Timer_Loop() → LCD_* → SPI, while SD.open / write / close use the same bus.

Risk: Interleaved transactions corrupt SD I/O or garble a display frame → failed mounts, bogus PCAP writes, or rare crashes.

Do: Add a single mutex (e.g. portMUX_TYPE or FreeRTOS mutex) that every LCD SPI transfer and every SD filesystem call takes around the full operation (including SD.begin path if the core does SPI under the hood). Alternatively: pause LVGL (skip lv_timer_handler / block flush) for the duration of each SD block in handshakeCaptureProcessPending().

Files: Display_ST7789.cpp, SD_Card.cpp, handshake-capture-c6.ino (or one small spi_bus_lock.h used by both).

---

2. Cap or validate eapol_count semantics — high

What: Any unencrypted EAPOL-shaped frame increments the counter. Four frames ≠ guaranteed one valid 4-way (could be duplicates, other STAs, or rekeys).

Do (minimal): Document the limitation in README.md. Better (still small): stop incrementing after HANDSHAKE_EAPOL_FRAMES (saves buffer space and makes UI honest). Better later: parse EAPOL Key Info / replay counter to count distinct steps of the handshake (more code).

Files: HandshakeCapture.cpp (promisc callback EAPOL branch).

---

3. 2.4 GHz–only channel sweep — high for real environments

What: getNextChannelSweep() only considers channels 1–14. Scan results on 5 GHz (or 6 GHz, if ever reported) are never visited, so those BSSIDs are never captured.

Do: Either exclude non-2.4G networks from the UI / isCaptureable path with a clear status, or extend sweep to supported WiFi.channel() values for your core/regulatory config (larger change: channel hop rules, dwell time).

Files: HandshakeCapture.cpp (getNextChannelSweep, startMultiCapture).

---

4. Production vs debug serial — medium

What: [SD], [CAP], [SETUP] strings and deauth logs are useful on the bench but noisy and slightly slow in the field.

Do: Wrap verbose Serial.printf / println in #ifdef HANDSHAKE_DEBUG (or a build flag) default off for “release” builds; keep boot version line always.

Files: SD_Card.cpp, HandshakeCapture.cpp, handshake-capture-c6.ino.

---

5. USE_SOFTAP default — medium

What: WiFi.mode(WIFI_AP_STA) runs a soft AP without a clear use case in this sketch (capture is STA + promisc). Extra RF/channel behavior and attack surface.

Do: Default #define USE_SOFTAP 0 unless you explicitly need AP mode; document how to enable for lab tests.

Files: HandshakeCapture.cpp, README.md.

---

6. Long SD write vs watchdog / UI — medium

What: Writing ~4 KB PCAP can block the main loop for tens of ms. LVGL stalls; on some configs the task watchdog could trip if SD is slow or the card hangs.

Do: After large file.write, optional yield(); if WDT issues appear, esp_task_wdt_reset() in the SD path only with measured care, or increase WDT timeout in setup. Prefer fixing SPI contention (#1) first.

Files: HandshakeCapture.cpp (handshakeCaptureProcessPending), setup().

---

7. Rescan invalidates “ready” list vs slots — low

What: During capture, the network list is static. If user triggers rescan (or you add a button later), network_index in slots could desync from rebuilt networks[].

Do: Today there is no mid-capture rescan from UI — document that. If rescan is added, stop capture first and clear slots.

Files: README.md / future UI code.

---

8. Magic numbers centralization — low (maintainability)

What: Timing (OBSERVE_DURATION_MS, CAPTURE_TIMEOUT_MS, …) and CHANNEL_TIMEOUT_MS in the .ino are related but defined in two places; easy to desync.

Do: Move all timing #defines to one header (e.g. HandshakeCapture.h or capture_config.h) and include from the sketch.

Files: HandshakeCapture.cpp, handshake-capture-c6.ino.

---

Already in decent shape

• Spinlock around promiscuous vs main-loop slot state; SD I/O only on main loop.
• SD_IsReady() vs flaky cardType() after WiFi; FAT filename sanitization; discard on failed save (no pending deadlock).
• PCAP fixed buffer; append failure doesn’t increment EAPOL count.

---

Add new items here as you close these; bump HANDSHAKE_FIRMWARE_VERSION when behavior changes.