AutoFilm-ESP32/docs/TARGET_ARCHITECTURE.md

# AutoFilm-ESP32 — Target Architecture

Status: proposed on `refactor/esp-idf-modular-ui`, for review via PR into **`develop`**.
Does not change default process times, temperatures, or rotation patterns.

## Goals

1. Rebuild as an **ESP-IDF** project that still links **Arduino-as-component** so AccelStepper, Keypad, LiquidCrystal_I2C, OneWire, and DallasTemperature keep working in the first cut.
2. Produce **two firmware images in CI**: classic ESP32-WROOM (2004 + keypad) and Guition **JC4827W543C** (ESP32-S3-WROOM-1-N4R8, NV3041A 480×272 QSPI, GT911 capacitive touch).
3. Split **UI / input** from **mechatronics** so a touch panel can replace the character LCD without rewriting agitation or recipes.
4. Use FreeRTOS tasks, queues, and interrupts so the UI never blocks on motor moves, OneWire conversions, or tones.
5. **Always stop motion on Esc** (keypad position 4 across / 4 down, key `'X'`) and, later, on a touch STOP control. Same command path for both.
6. Keep Arduino libraries isolated behind HAL files; replacing them with native IDF drivers is planned debt, not this document’s implementation phase.
7. Leave room for pumps, valves, reservoirs, and drain without breaking the command/event contract.

Arduino-as-component is **explicit technical debt**. New modules must not add Arduino types to public headers.

## Non-goals (this refactor)

- Changing C41 / E6 / ECN-2 / B&W / Custom / B&WREV default numbers.
- A real Custom-programme editor (session ±5 s only; NVS profiles later).
- Closed-loop bath heating.
- Implementing pumps/valves now — only the extension points.

## Dual board model

| | `board_wroom` | `board_jc4827w543` |
| --- | --- | --- |
| SoC | ESP32-WROOM (classic) | ESP32-S3-WROOM-1-N4R8 (4 MB flash, 8 MB octal PSRAM) |
| UI | 20×4 I²C LCD `0x27` + 5×4 keypad | 4.3" IPS 480×272 NV3041A QSPI + GT911 I²C |
| Touch | none | GT911, typical pins SDA 8, SCL 4, INT 3, RST 38, addr `0x5D` |
| Display bus | I²C 21/22 | QSPI CS 45, SCK 47, D0–D3 21/48/40/39, backlight GPIO 1 |
| Mechatronics | current pin map | **remapped**; panel consumes most GPIOs, ~10 usable extras + headers |

Pin maps live only in board components. Application code sees `hal_motor_enable()`, not `GPIO_NUM_27`.

CI builds:

```
idf.py -D AUTOFILM_BOARD=wroom     @sdkconfig.defaults @sdkconfig.wroom
idf.py -D AUTOFILM_BOARD=jc4827w543 @sdkconfig.defaults @sdkconfig.s3
```

Automated by Gitea Actions — see `docs/CICD.md`. Both images are required checks on PRs into `develop`.

WROOM remains the machine that already exists. S3 firmware is the future operator panel; mechatronics may stay on the WROOM or move when the S3 I/O map is proven. The **same** `app_machine` binary interface is compiled for both.

Vendor references for the S3 panel (not copied into this repo): [lsdlsd88/JC4827W543](https://github.com/lsdlsd88/JC4827W543), [profi-max board notes](https://github.com/profi-max/JC4827W543_4.3inch_ESP32S3_board) — NV3041A + GT911, 480×272.

## Proposed tree

Existing `src/` and `include/` stay until each file is moved. New code lands here:

```
AGENTS.md
docs/
  CURRENT_STATE.md
  TARGET_ARCHITECTURE.md
main/                      # idf app: NVS, task spawn, board select
components/
  app_process/             # recipe table + lookup + session time overrides
  app_machine/             # process state machine (no display types)
  app_ui/                  # screens + command emission (no stepper types)
  ui_cmd/                  # command / event structs only
  hal_motor/
  hal_temp/
  hal_input/               # keypad + (later) GT911 → ui_event
  hal_display/             # 2004 adapter; later NV3041A adapter
  hal_audio/
  board_wroom/
  board_jc4827w543/
managed_components/        # Arduino-esp32 component, IDF drivers
```

Public headers of `app_*` and `ui_cmd` are C (or `extern "C"`) and Arduino-free.

## Command / event contract

UI never starts the motor. UI posts commands. Machine never writes pixels. Machine posts events.

### Commands (`ui → machine`)

| Command | Now | Later |
| --- | --- | --- |
| `SelectProcess(id)` | keypad 1–6 | touch tile |
| `BrowseStep(delta)` | U/D | swipe / list |
| `AdjustStepTime(step, delta_s)` | L/R ±5 | stepper buttons |
| `ArmStep(step)` | show Ent/Esc prompt | confirm sheet |
| `StartStep(step)` | Enter | START |
| `Stop` | Esc during run | STOP |
| `Resume` | after Stop | RESUME |
| `ReturnToStepSelect` | after Stop | BACK |
| `CancelArmed` | Esc before start | cancel |

Reserved, not implemented:

`PumpSet`, `ValveSet`, `ReservoirSelect`, `DrainStart`, `DrainStop`, `HeaterSetpoint`.

Adding an actuator is a new command + HAL + machine state. It must not require `app_ui` to include motor headers.

### Events (`machine → ui`)

| Event | Payload |
| --- | --- |
| `ProcessSelected` | process id, name, step count |
| `StepView` | index, name, time_s, temp pref/min/max, cw, ccw |
| `StepArmed` | same |
| `StepStarted` | start tick, duration_ms |
| `StepProgress` | remaining_ms, temp_c, motion {cw\|ccw\|idle} |
| `StepComplete` | index |
| `StepStopped` | remaining_ms, index |
| `StepResumed` | remaining_ms |
| `ProcessIdle` | — |
| `TempUpdated` | °C or disconnected |
| `Fault` | code, message |

Required on-screen fields while running: **temperature, step name, time remaining**. Motion direction is optional. STOP / RESUME / BACK are controls, not telemetry.

### Transport

One FreeRTOS queue each way (or a tiny pub/sub wrapping `xQueueSend`). Bounded structs, no `String`. ISR context may only `xQueueSendFromISR` a `Stop` or raw key/touch sample.

```
┌────────────┐   cmd_q    ┌──────────────┐   actuator calls   ┌──────────┐
│  app_ui    │ ─────────► │ app_machine  │ ─────────────────► │ hal_*    │
│ + hal_in   │ ◄───────── │ + app_process│ ◄── temp / done    │          │
└────────────┘   evt_q    └──────────────┘                    └──────────┘
```

## State machine

```
Idle → ProcessSelected → StepSelect ⇄ StepArmed
                              │
                              ▼
                         StepRunning ◄── Resume
                              │ Stop
                              ▼
                         StepStopped → StepSelect
                              │ timer done
                              ▼
                         StepComplete → StepSelect (or auto-advance policy later)
```

`run == 1` auto-chaining from the current `startDev()` is preserved as a **machine policy flag** (`auto_advance`), not as UI blocking. Default: after `StepComplete`, arm the next step and wait for Start (safer with pour/drain). If we must match today’s “immediately start the next step after the alarm”, that is one flag, not a second code path.

### Stop / resume (product rule)

- **Stop** = immediately end the agitation routine, **disable the motor driver**, keep the step and remaining time, show Resume or Return to step select.
- **Resume** = continue the **same step** for the remaining time with the same CW/CCW pattern. Short beep.
- **Return** = leave the step stopped, motor disabled, back to step list. Remaining time for that step stays at whatever was left (session override), unless we later decide to snap back to nominal — default is keep remaining.
- Esc **before** Start stays “cancel arm”, not Stop.
- Short beep on Stop and on Resume. The current 10-beep end-of-step alarm becomes a non-blocking audio request on `StepComplete`.

Esc must work in `StepRunning`, `StepStopped` (as Back), and during the complete-alarm.

## Tasks, cores, interrupts

Watchdogs stay **on**. Every long-lived task calls `esp_task_wdt_reset()` or is not on the TWDT.

| Task | Priority (relative) | Work |
| --- | --- | --- |
| `input_task` | high | Keypad scan ~20–50 Hz; GT911 read on INT or poll. Debounce. Map to commands. |
| `ui_task` | mid | Consume events, draw dirty regions only. Never `delay` for motor or OneWire. |
| `machine_task` | mid-high | State machine, timers via `vTaskDelayUntil` / `esp_timer`. |
| `motor_task` | high | Agitation pattern. Waits on notifications, not `vTaskDelete`. |
| `temp_task` | low | DS18B20: kick conversion, wait conversion time, publish `TempUpdated`. |
| `audio_task` | low | Short beeps / alarm pattern from a queue. |

Arduino `loop()` becomes a thin idle (or is not used). `setup()` only starts IDF tasks.

### Stop path (must be low latency)

1. Keypad Esc or GT911 hit-test on STOP is recognized in `input_task` or, for the matrix Esc line if we later wire a dedicated GPIO, in a GPIO ISR.
2. `Stop` is queued to `machine_task` **and** `hal_motor_request_stop()` runs:
   - set an atomic `stop_req`
   - drive **EN HIGH immediately** (driver off)
   - `xTaskNotify` the motor task to abandon the current move
3. Motor task leaves `stepper.run()` at the next loop check (or aborts the RMT transaction in the IDF driver) and parks.
4. Machine publishes `StepStopped` with remaining time. UI shows Resume / Back.

EN-off is the safety action. It does not need a new motor-driver IC.

### Question 8, resolved: RMT / MCPWM vs AccelStepper

No extra power electronics. The existing STEP/DIR driver stays.

Today AccelStepper **busy-polls** `stepper.run()` on a CPU core to generate step edges. That works, but:

- Stop latency is “next poll of `distanceToGo`”, and `vTaskDelete` is unsafe mid-pulse.
- The core cannot sleep; jitter depends on competing work.

ESP32 **RMT** (or MCPWM + a step counter) can emit the pulse train in hardware. The CPU only queues “N steps this direction”. Stop then is: abort the RMT TX + drop EN, which is cleaner.

**Phase 1:** keep AccelStepper behind `hal_motor`, but add `stop_req` checks inside the inner `run()` loop and never `vTaskDelete` the task.  
**Phase 2 (debt burn-down):** replace AccelStepper with an IDF RMT stepper. Same `hal_motor` API.

## HAL sketches

```c
void hal_motor_init(void);
void hal_motor_enable(bool on);
void hal_motor_request_stop(void);          // ISR-safe: EN off + flag
esp_err_t hal_motor_move_revs(float revs, int dir, uint32_t rpm);
bool hal_motor_is_busy(void);

void hal_temp_init(void);
esp_err_t hal_temp_read_c(float *out);      // blocking OK — temp_task only

typedef enum { UI_KEY, UI_TOUCH } ui_src_t;
void hal_input_init(void);
bool hal_input_pop(ui_raw_event_t *ev);

void hal_display_init(void);
void hal_display_clear(void);
void hal_display_text(int col, int row, const char *s);   // 2004 mapping
void hal_display_flush(void);                             // no-op on 2004

void hal_audio_beep_short(void);
void hal_audio_alarm_complete(void);
```

Colour UI implements a richer draw API *or* LVGL flush, still inside `hal_display` / `app_ui`, never inside `app_machine`.

## UI strategy: 2004 now, touch without a rewrite

`app_ui` is a set of **screens** (`ProgramSelect`, `StepSelect`, `Armed`, `Running`, `Stopped`) that read the last event snapshot and emit commands.

- **WROOM adapter:** screens print 20×4 strings. Input adapter translates `'1'`…`'X'` into commands.
- **S3 adapter:** same screens, different renderer.

### LVGL vs custom (recommendation)

The JC4827W543 community stack is Arduino_GFX + LVGL + TAMC_GT911 / TouchLib. LVGL will render a pretty 480×272 panel and is already demoed on this exact board.

For AutoFilm the visible surface is small: six programmes, a step list, one run view (name / remaining / temp), Stop / Resume / Back. That is a few dirty-rectangle updates per second.

**Recommendation: custom screens first, LVGL optional later.**

- Custom keeps the UI task tiny, avoids 4 MB-flash + PSRAM fight with LVGL v8/v9 buffers, and does not force Arduino_GFX deeper into the tree.
- LVGL widgets are worth it when we have graphs, profile editors, or multi-language layouts. Those are out of scope.
- If a first S3 prototype is faster with vendor LVGL demos, isolate LVGL in `board_jc4827w543` + `app_ui` and **do not** put `lv_*` types in `ui_cmd` or `app_machine`.
- Either way: LVGL `flush_cb` and touch read run on `ui_task` / `input_task` only; no `lv_timer_handler()` on `machine_task`.

Touch STOP is a hit-test that emits the same `Stop` command as Esc.

## Process data

Keep the six recipes and their numeric defaults. The storage shape may change.

Proposed:

```c
typedef struct {
    const char *name;
    uint8_t step_count;
    const process_step_t *steps;   /* flash / const */
} process_def_t;

typedef struct {
    const char *name;
    uint16_t time_s;
    float cw_revs;
    float ccw_revs;
    float temp_min_c;
    float temp_pref_c;
    float temp_max_c;
} process_step_t;
```

Session overlay: `int16_t time_delta_s[step]` in RAM (L/R ±5). Not persisted. NVS profile slot is a later increment behind `app_process` (`process_save_custom()`, etc.). Custom remains the 4×10 s stub.

`processName[7]` becomes a pointer; `"B&WREV"` no longer depends on a 6-char cap.

## Audio

`tone()` + `delay()` moves into `audio_task`. Machine sends `AUDIO_BEEP_SHORT` or `AUDIO_ALARM_COMPLETE`. Stop must pre-empt an in-progress alarm.

## Safety and timing

- Motor EN is safe-state HIGH (disabled) at boot and on any `Fault` or `Stop`.
- Step deadline is `esp_timer` one-shot, not a `millis()` busy loop.
- Temp offset `+0.4 °C` stays a board/config constant.
- Re-enable TWDT; conversion wait lives in `temp_task`.
- No `vTaskDelete` of worker tasks after init. Tasks block on queues/notifications for life.

## Migration plan

Executable session phases: `docs/megaplans/REFACTOR-MEGAPLAN.md`.

1. **Docs + conventions** (P00, this branch).
2. **IDF skeleton** (P01) Arduino-free, two sdkconfigs.
3. **`ui_cmd` + `app_process` + golden tests** (P02).
4. **`app_machine` + stub HAL** (P03). Host tests become the gate.
5. **Gitea Actions** (P04) `.gitea/workflows/ci.yml`.
6. **WROOM motion HAL** (P05) then **UI cutover** (P06) including Stop/Resume.
7. **S3 display+touch** (P07).
8. **Arduino debt burn-down** (P08).

## Acceptance checks for the first behavioural cut (WROOM)

- Defaults for all six processes match `docs/CURRENT_STATE.md` tables.
- Esc during a running step disables the motor within one step-pulse loop iteration and shows Resume / Back.
- Resume continues remaining time; Back returns to step select.
- Short beep on Stop and Resume; complete-alarm does not block Esc.
- Temperature updates while running without stalling remaining-time display.
- UI remains navigable if the DS18B20 is disconnected (`--` as today).
- Watchdog no longer disabled as a matter of policy.