From b246132a35c39ae357f2bcf55c148da3edd87112 Mon Sep 17 00:00:00 2001 From: MCKero Date: Fri, 28 Aug 2026 03:58:19 +0100 Subject: [PATCH] Bring the docs in line with both keypad fixes AGENTS.md still carried a stale entry telling the reader to *lengthen* key holds when a press seems ignored, which is the opposite of the fix and is what broke the tooling in the first place. Replaced with the correction and a pointer to the right section. The keypad heading also claimed the hold time was the only cause. There were two: the 2500 ms hold in key.py, and row_out missing volatile. Both are now listed up front with a link to the detail. Adds the regression test to the places someone would actually look: the "How to run it" section in AGENTS.md, the layout listing, and a build step in the README noting that a clean build is not evidence the keypad works, since the -O2 dead-code elimination produces no warning. --- AGENTS.md | 29 +++++++++++++++++++++++------ README.md | 9 +++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5eb7cad..6aa3c6f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,6 +57,12 @@ Rebuild after editing the machine: cd $QEMU/build && ninja qemu-system-arm # ~10 s incremental +After any change near the keypad or the GPIO wiring, run the regression test. It +boots its own instance on private ports, so it does not disturb a `run.sh` +session: + + python3 tools/keypad_test.py + ## Things that already went wrong **GDB breakpoints halt the guest.** A key held across a breakpoint session is @@ -80,20 +86,31 @@ unnamed `qdev_init_gpio_in` and `qdev_init_gpio_out` makes `qdev_get_gpio_in()` ambiguous, and board wiring silently attaches to the wrong line. The GPIO model uses `"pin-in"` and `"pin-out"` for this reason. Keep it that way. -**Key hold times must be generous.** Guest time runs fast, so 400 ms of wall -clock was too short for the firmware's debounce to complete. `key.py` holds for -2500 ms. If a press seems ignored, lengthen it before suspecting the wiring. +**Key hold times must be SHORT, not generous.** This entry used to say the +opposite -- that guest time runs fast so a press needs a long hold, and that +`key.py` should hold for 2500 ms. That was wrong and it broke the keypad tooling +for a long time. 2500 ms is ~250 firmware ticks, six times past the long-press +threshold, so every press was dispatched as a *hold* and handlers that act on a +short release did nothing. See the keypad section below; `key.py` now holds 200 ms. **Verify a tool's own parsing before trusting its output.** `gpio_watch.py` reported `IDR=0x0000` for several rounds because its regex did not match gdb's output format at all. The register was fine; the reader was broken. Cross-check with `tools/gpiob_dump.sh`, which uses a different path. -## The keypad works: it was always the hold time +## The keypad: two real bugs, both fixed The old note here said "keys reach the firmware but the UI does not react" and -pointed at the machine model. The model was never the problem, and there was only -ever one cause: `tools/key.py` held every key for 2500 ms. +blamed the machine model. There turned out to be two independent causes, in this +order: + +1. **`tools/key.py` held every key for 2500 ms** — a tooling bug, covered + immediately below. +2. **`row_out` was not `volatile`, so GCC deleted the row-driving code** — a real + model bug, introduced later while removing debug prints. See + [row_out must stay volatile](#row_out-must-stay-volatile-or-gcc-deletes-the-keypad). + +Both are fixed and `tools/keypad_test.py` guards against regressions in either. The two SysTick mechanisms are separate, and conflating them caused this: diff --git a/README.md b/README.md index dbbcab1..9f8fea4 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ keypresses silently stop working. Run the test after touching that code; calibration.bin 512-byte dump from a real radio docs/screenshots/ LCD captures used in this README tools/ run, screenshot, inject keys, probe state + keypad_test.py keypad regression test, boots its own instance harness/, stubs/, shim/, tests/ host build of the CW timing chain (stage A) ## Building @@ -99,6 +100,14 @@ Needs a QEMU 7.2 source tree, `meson`, `ninja`, `libfdt-dev`, `libglib2.0-dev`, ./configure --target-list=arm-softmmu --disable-docs --disable-tools cd build && ninja qemu-system-arm +Then check the build actually works, which takes about a minute: + + python3 tools/keypad_test.py + +This matters more than it looks. The keypad can break silently under -O2 without +any compiler warning -- see the `volatile` note in [Status](#status) -- so a clean +build is not evidence that keypresses work. + ## Running python3 tools/make_flash.py # once, builds assets/flash.img