docs: carousel wrap report
This commit is contained in:
@@ -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.
|
||||
Reference in New Issue
Block a user