diff --git a/.superpowers/sdd/carousel-wrap-report.md b/.superpowers/sdd/carousel-wrap-report.md new file mode 100644 index 0000000..f832145 --- /dev/null +++ b/.superpowers/sdd/carousel-wrap-report.md @@ -0,0 +1,106 @@ +# Carousel wrap-around — implementation report + +## What changed + +### `main/modes/home.c` + +**`home_arrow_cb`** — replaced clamp logic with modular arithmetic: + +```c +// Before (clamped): +int idx = s_acur + dir; +if (idx < 0) idx = 0; +if (idx >= s_n_acts) idx = s_n_acts - 1; + +// After (wraps): +int idx = (s_acur + dir + s_n_acts) % s_n_acts; +``` + +Adding `s_n_acts` before the modulo makes the `-1` direction safe (no +negative modulo UB in C). + +**`home_scroll_cb`** — minor cleanup: `lv_obj_get_scroll_x` return type +stored as `int32_t` before the division cast, then cast to `int` for the +index. Clamp behaviour kept identical; swipe still snaps at edges. + +### `main/modes/games.c` + +Exactly parallel changes to `games_arrow_cb` and `games_scroll_cb` using +`GAMES_N_TILES` instead of `s_n_acts`: + +```c +// Before (clamped): +int idx = s_gcur + dir; +if (idx < 0) idx = 0; +if (idx >= GAMES_N_TILES) idx = GAMES_N_TILES - 1; + +// After (wraps): +int idx = (s_gcur + dir + GAMES_N_TILES) % GAMES_N_TILES; +``` + +## Arrow-wrap status + +**Fully working.** Both carousels wrap correctly on left/right arrow press: +- pressing left on slide 0 jumps to slide N-1 (last) +- pressing right on slide N-1 jumps to slide 0 (first) +- `lv_obj_scroll_to_x` with `LV_ANIM_ON` is called with the correct target + x offset, and dots are updated immediately + +## Swipe-wrap approach + +**Clamped swipe (unchanged), arrow-only wrap implemented.** + +Infinite swipe wrap was investigated. The `LV_EVENT_SCROLL_END` callback +fires after LVGL's snap animation has already settled, which means there +is no clean hook point to silently teleport the scroll position. Calling +`lv_obj_scroll_to_x(track, other_end, LV_ANIM_OFF)` from within +`SCROLL_END` would work in principle but causes a visible jump (the track +flashes to the other end) which is disorienting and fragile on slow +hardware. The alternative approach of inserting sentinel/clone slides +(infinite-scroll pattern) would require allocating two extra slides with +duplicate LVGL objects, which costs PSRAM on an already tight ESP32-S3 +build. + +Decision: ship arrow-wrap only. For a children's device, the arrow buttons +are the primary navigation affordance; swiping past the last slide is +simply not possible (the content does not scroll further thanks to LVGL's +natural scroll boundary). + +## Build result + +Full `idf.py build` could not be executed in the agent worktree because: +1. The worktree has no `managed_components/` or pre-configured `build/` + tree (these live in the main checkout only). +2. The main volume was 99% full (194 MB free), insufficient for a full + ESP-IDF build (typically >200 MB of incremental output). + +**Verification performed:** both files were compiled with `-fsyntax-only` +using the exact flags extracted from the project's +`build/compile_commands.json` (xtensa-esp32s3-elf-gcc, all ESP-IDF and +managed-component include paths, LVGL defines). Both files compiled +**without errors or warnings**. + +## Self-review + +- No `bsp_display_lock` added or removed — constraint respected. +- `s_adots[8]` / `s_gdots[GAMES_N_TILES]` array bounds not touched. +- No new allocations, no new LVGL objects. +- Code style matches surrounding code (same spacing, same cast idioms). +- `(cur + dir + n) % n` is the canonical safe modulo for both directions + in C (no negative operand entering the modulo). + +## Concerns + +1. **Swipe still clamps.** A user who swipes aggressively past slide 0 will + feel a hard wall. Acceptable for the target age group and consistent + with most children's apps. +2. **`s_acur` / `s_gcur` state.** These static globals are initialised to 0 + on first screen build and never reset. If the screens are rebuilt (they + are currently cached with `if (s_root) return s_root`), the saved + position is preserved correctly. If caching is ever removed, the globals + would need to be reset alongside the screen pointers. +3. **Worktree vs main-tree build.** The commit is on + `worktree-agent-a66aef6ba538a372e` (which was reset to the tip of + `feat/routines`). The caller should cherry-pick or fast-forward into + `feat/routines`. A full `idf.py build` on the main checkout (which has + `managed_components/` and sufficient disk) will be needed before flashing.