AutoFilm-ESP32/docs/audit_remediation_plan.md

Codebase Audit & Remediation Plan (develop branch)

After a thorough audit of the develop branch focusing on the recent modular ESP-IDF port, I've identified several critical defects primarily centered around FreeRTOS concurrency, system timers, and legacy feature regressions.

Here is the step-by-step remediation plan to address these issues. No code has been changed yet.

1. Fix Critical Step Timing Bug in app_machine.c

Defect: CMD_START_STEP triggers start_running(0), passing a hardcoded 0 for now_ms. The task then calculates s_deadline_ms = 0 + remaining_ms. When app_machine_tick() runs moments later, it reads the real system uptime (e.g., 200,000 ms). If the uptime is larger than the step duration, the step instantly completes. Remediation Steps:

• Modify start_running() to remove the now_ms argument.
• Instead of calculating s_deadline_ms immediately, configure the state machine to defer calculation: set s_have_deadline = false; and s_resume_pending = true;.
• This safely delegates the deadline calculation to the next app_machine_tick() cycle, which naturally computes it using the true system now_ms.
• Update app_machine_handle_cmd() to call start_running() without arguments.

2. Fix Event Queue Concurrency in app_machine.c

Defect: The system uses a raw array s_q (with s_q_head, s_q_tail, and s_q_count++) to pass events from the machine to the UI. However, emit() is called by both the machine_task and the temp_task (via app_machine_on_temp), while app_machine_last_event() is read by the ui_task. These are non-atomic read-modify-write operations across three threads, which will inevitably corrupt the queue and crash the UI. Remediation Steps:

• Replace the raw ring buffer variables (s_q, s_q_head, s_q_tail, s_q_count) with a standard FreeRTOS QueueHandle_t s_evtq.
• In app_machine_init(), initialize the queue: s_evtq = xQueueCreate(UI_EVT_QUEUE_LEN, sizeof(ui_evt_t));.
• Update emit() to use xQueueSend(s_evtq, &ev, 0);.
• Update app_machine_last_event() to use xQueueReceive(s_evtq, out, 0) == pdTRUE.

3. Restore Auto-Advance Functionality in app_machine.c

Defect: The legacy Arduino loop automatically chained processing steps together (run == 1). The new state machine includes a maybe_auto_advance() function guarded by s_auto_advance, but s_auto_advance is permanently hardcoded to false and never toggled. As a result, the machine halts after every single step. Remediation Steps:

• Initialize s_auto_advance = true to match the legacy behavior of chaining steps automatically.
• (Optional) Wire up a UI command (CMD_TOGGLE_AUTO_ADVANCE) to allow users to turn this off if manual pausing between steps is desired.

4. Fix Task Watchdog Initialization in main.c

Defect: ui_task, machine_task, input_task, and temp_task all invoke esp_task_wdt_add(NULL). However, the Task Watchdog Timer (TWDT) is never initialized in app_main(). Depending on the ESP-IDF version and sdkconfig defaults, this can cause silent failures or panic at boot. Remediation Steps:

• In app_main(), invoke esp_task_wdt_init(&wdt_config) before creating the FreeRTOS tasks.
• Ensure the WDT timeout is set generously (e.g., 3-5 seconds) to accommodate the 1.5-second blocking time in hal_temp_tick().

5. Clean up hal_temp_tick() Timing (Low Priority)

Defect: hal_temp_tick() blocks temp_task with multiple vTaskDelay(pdMS_TO_TICKS(750)) calls for the DS18B20 conversion. While safely isolated from the UI, it forces the watchdog timeout to be artificially large and limits responsiveness if multiple sensors were ever added. Remediation Steps:

• Refactor the 1-Wire sequence into a non-blocking state machine within temp_task, or simply keep it as-is but acknowledge the WDT requirement constraint outlined in step 4.