Exit a running cart back to the launcher (IN-6) - #35
Open
aldwinhermanudin wants to merge 4 commits into
Open
aldwinhermanudin wants to merge 4 commits into
aldwinhermanudin wants to merge 4 commits into
Conversation
Once a cart launched there was no way back to the launcher short of a power
cycle: fake-08's GameLoop() never returns, and MENU only toggles a pause flag
that renders nothing. A launcher holding 3145 carts played exactly one per boot.
Holding MENU for 1.2 s now restarts into the carousel. Restart is the return
path deliberately — the launcher is ~1.8 s from reset with the SD mounted and
the radio off, which beats unwinding a loop documented never to return.
The gesture lives in the input layer rather than a watcher task, because
input_poll() is destructive on the serial backend: a second caller would eat
input the VM never sees. Each backend calls input_exit_check(held) from inside
its own poll.
Two things worth keeping in mind, both learned the hard way here:
- The arm sits ABOVE main.cpp's five-way loop ladder, not inside a branch.
Placed in one branch it exists only in that build, and is missing from
exactly the dev builds where it gets exercised.
- The threshold is wall-clock, not a poll count. "72 polls at 60 Hz" silently
changes duration in any loop that is not 60 Hz, and four of the five are not.
Detect -> restart -> launcher is verified on the P4. The 1.2 s hold duration is
NOT verified and cannot be over serial: USB-Serial-JTAG batches input at roughly
1 Hz here, so a held button is unrepresentable. It needs a touch build and a
finger on the deck; the worklog says so too.
Four of these changed behaviour; three were comments and docs asserting things that are not true. Host test no longer linked. host_test/run.sh compiles test_input_scheduled.c as a single translation unit against stub IDF headers, so pulling input_exit_check() into input_scheduled.c broke it with an undefined reference. It passes on main and failed on this branch. Stubbed the hook in the test; the ring/parser logic under test is unaffected by the gesture. Armed where there is no launcher to return to. The carousel needs LAUNCHER and a mounted card, but arming was unconditional: on a card-less boot of the shipped build, holding MENU rebooted into the same cart from the top while logging "returning to the launcher". Now gated on launcher_is_boot_dest. A tap plus a stall could reboot the device. The threshold compared two polls that happened to see MENU held, requiring nothing in between. One serial byte holds INPUT_PAUSE for INPUT_HOLD_FRAMES polls, so a tap followed by carousel_fb_dump()'s multi-second transfer (or a GC pause, or a slow SD read) landed a held poll >1.2 s after the first and rebooted. Held polls further apart than INPUT_EXIT_MAX_GAP_MS (250 ms) now restart the measurement. INPUT_EXIT_HOLD_MS never reached the compiler. DEFS='-D X=Y' only sets a CMake cache variable; every other tunable is plumbed with target_compile_definitions. So the documented escape hatch -D INPUT_EXIT_HOLD_MS=0 silently built the 1200 ms default with a live gesture, as did the 30 ms bench threshold. Both tunables are now plumbed, and the flags are confirmed in the build output. The comment claiming nothing is lost on exit was wrong: dset() only pokes RAM at 0x5e00, and CloseCart/vm_load/a cartdata key change are the only paths that serialise it, all bypassed by esp_restart(). Cart save state IS discarded, and the header now says so. input_stub.c and input_i2c.c did not call the hook the CMakeLists claimed every backend calls; they do now, which matters because input_i2c.c is the IN-3 skeleton for the physical-button handheld. The input spec described the poll-counting design this work deliberately rejected. Builds clean for P4 and S3; host test passes.
Second review round on this PR, five findings. The first is the worst defect this feature has had, and the previous round's fix created it. Closing the tap-plus-stall hole added INPUT_EXIT_MAX_GAP_MS, but the function still zeroed the run on ANY single poll that did not report MENU. On the P4's shipped touch backend that is not an edge case: board_touch_read() returns 0 on any GT911 I2C hiccup and, routinely, on every poll where the controller's buffer-ready bit is clear because it has not posted a new frame since the last read. At a 60 Hz poll that happens constantly, so across the ~72 polls a 1.2 s hold needs, the run was almost always reset partway. The gesture would have shipped effectively unfirable, with nothing logged to say why. It is P4-only — the S3's FT6236 read is level-based — so the bug lived on the one board that cannot be driven over serial. "Verified over serial" was never the same as verified. A MENU-less poll no longer ends a run on its own; only silence for longer than the gap does, which is the tolerance already granted between two held polls. Accepted residual, documented in the header: taps closer together than 250 ms keep one run alive, so mashing MENU at ~5/s for a solid 1.2 s trips the exit. This logic has now been wrong twice, in opposite directions, and neither bug was reachable from the bench. So it is pinned by a host test with a fake clock: components/input/host_test/test_input_exit.c, six cases including both regressions. Verified the test earns its place by reinstating each bug in turn — case 5 fails with the dropout bug, case 4 fails with the stall bug. Also: input_touch.c and input_scheduled.c returned early on !s_ok before the hook, breaking the invariant the previous commit bought by adding the call to the stub and i2c backends; worklog section 3 still described the rejected poll-counting design and its file list had gone stale against its own review section; and the branch added an HTML render with no index.html card, which .ai/AGENTS.md requires. Builds clean for P4 and S3; both host tests pass.
Third review round. The headline finding is that the gap this feature was
written to fill was described wrongly in three shipped documents.
fake-08 DOES render a pause menu. The `//todo: pause menu here` at vm.cpp:1247
that I cited as evidence is inside a fully commented-out function and describes
nothing that runs. The live path is Vm::Step() -> __z8_tick(), whose
__ispaused() branch calls __f08_menu_update() + __f08_menu_draw() every frame
and draws a navigable menu: continue / reset cart / exit to menu / exit to
settings. So "there is no exit" was false, and so was "a launcher holding 3145
carts plays exactly one per boot" -- you could always leave a cart.
I had a photograph of that menu. The first bench-camera frame I took this
session, and described in detail, shows CONTINUE / RESET CART / EXIT TO MENU /
EXIT TO SETTINGS on the P4 panel. I read that screen for "is the board alive"
and not for "is what I wrote true".
What survives, and is now what the docs claim: "exit to menu" runs
__loaddefaultcart -> QueueCartChange("__FAKE08-DEFAULT.p8"), fake-08's built-in
plain-text browser -- not our cover-art carousel. The carousel is native code in
main.cpp that runs before GameLoop(), so the VM cannot return to it, and
redirecting that menu item would mean changing the submodule, which is the cost
this design exists to avoid. IN-6 is a convenience that reaches OUR launcher,
not the only way out of a cart.
Also from this round: run.sh built and ran the exit test twice (a copy-paste
slip in the previous commit); -D INPUT_EXIT_HOLD_MS=0, the documented way to
compile the gesture out, left TAG unused and warned, so TAG and the esp_timer /
esp_system includes now sit inside the #if; the host log stub now consumes its
tag so -Wall stays meaningful instead of being switched off; the arm-site
comment claimed dev builds are where the gesture gets exercised while the
launcher_is_boot_dest gate makes it inert in exactly those builds; and the file
list had gone stale against the host test the previous round added.
Builds clean for P4 and S3; both host tests pass, once each.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Holding MENU for 1.2 s while a cart is running restarts into the carousel. Before this, a launched cart could only be left by power-cycling —
GameLoop()never returns, and MENU toggles a pause flag that renders nothing.Restart is the return path, deliberately: the launcher is ~1.8 s from reset with the SD mounted and the radio off, which beats unwinding a loop documented never to return.
Why it lives in the input layer
Two alternatives don't work:
input_poll()is destructive on the serial backend; a second caller would eat input the VM never sees.app_mainis blocked insideGameLoop().So each backend calls
input_exit_check(held)from inside its own poll.Two mistakes this went through
main.cpp's five mutually-exclusive frame loops, so the gesture existed only in that build — absent from exactly the dev builds where it gets exercised. It's now armed above the ladder.The first was masked by a sticky
-D FB_DUMPin the CMake cache, so the initial hardware test ran the wrong loop.Verified, and how far
Detect → restart → launcher confirmed on the P4, and
entering GameLoop (hold MENU to return to the launcher)now appears on a normal launch in the shipped build.The 1.2 s duration itself is NOT verified, and cannot be over serial. USB-Serial-JTAG batches input at roughly 1 Hz here, so a held button is unrepresentable — the passing run used a temporary 30 ms threshold. Confirming the feel of the hold, and that a short tap still reaches PICO-8's pause, needs a touch build and a finger on the deck.
Worklog:
docs/worklog/2026-08-07-exit-cart-to-launcher.md