Fix key.py hold times; the keypad was never broken

key.py held every key for 2500 ms, on the assumption that guest time runs
fast during delays so a press needs a long wall-clock hold. That is wrong
for this path, and it is why the keypad looked dead.

The two SysTick mechanisms are separate. poll-boost accelerates counter
*reads* so SYSTICK_DelayUs converges; it does not speed up interrupt
delivery. Interrupts drive SysTick_Handler -> gNextTimeslice ->
APP_TimeSlice10ms -> CheckKeys at close to real time, so the firmware's
thresholds hold in wall clock as written: 20 ms to register a press,
400 ms to count as held.

2500 ms is ~250 ticks, six times past the long-press threshold, so every
press was dispatched as a hold. MAIN_Key_MENU acts only on a short release
and returns early when bKeyHeld is set, so nothing happened. Confirmed by
reading gDebounceCounter mid-hold: 317 after a 3 s hold, which also proves
the timeslice was running all along.

Now HOLD_MS=200 and LONG_HOLD_MS=900. Verified with screenshots: the menu
opens and UP/DOWN move through it.

Also here:
- Drop the three TRACE fprintfs. They fired on every keypad poll and
  buried the console; the matrix is confirmed working.
- Drop a redundant forward declaration of py32_spi_xfer_byte, silencing
  the only build warning.
- Document the real remaining gap: power save (~6 s after boot) stops the
  keypad scan and the model does not wake from it. Includes the two dead
  ends already ruled out by experiment, so nobody repeats them.
- Add README screenshots captured from guest memory.

.gitignore

@@ -7,3 +7,6 @@ build/
 # Transient debug captures.
 *.log
 *.png
+
+# Screenshots used in the README are committed on purpose.
+!docs/screenshots/*.png

AGENTS.md

@@ -89,26 +89,125 @@ 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.
 
-## Known gap: the keypad
+## The keypad: two separate problems, one fixed
 
-Keypresses reach the firmware but the UI does not react. What is established:
+The old note here said "keys reach the firmware but the UI does not react" and
+pointed at the machine model. The model was not the problem. There were two
+independent causes, and only the first is fixed.
 
-- The keypad model holds the right state (`qom-get press` reads back the key)
-- Row lines are driven: TRACE shows `row0 -> 0` while MENU is held
-- The firmware's scan sees it: at the IDR read inside `KEYBOARD_Poll`,
-  `ODR=033c IDR=7fbf` — column 1 low, row 0 low
-- `KEYBOARD_Poll` returns 10, which is `KEY_MENU` in `driver/keyboard.h`
+### Fixed: key.py held keys far too long
 
-So the matrix works and the scan decodes correctly. Whatever is wrong is
-downstream, in how `app.c` debounces or dispatches the returned key. That is
-where to look — not at the wiring, which has been checked more than enough.
+`tools/key.py` was holding every key for 2500 ms.
+
+The two SysTick mechanisms are separate, and conflating them caused this:
+
+- SysTick **interrupts** fire at close to real time. `SysTick_Handler` sets
+  `gNextTimeslice`, which gates `APP_TimeSlice10ms` -> `CheckKeys`. So the
+  debounce thresholds in `App/misc.c` apply in wall clock as written:
+  `key_debounce_10ms = 2` (20 ms to register), `key_repeat_delay_10ms = 40`
+  (400 ms counts as *held*).
+- The `poll-boost` property accelerates SysTick counter **reads**, so
+  `SYSTICK_DelayUs` converges. It does not speed up interrupt delivery.
+
+A 2500 ms hold is ~250 ticks, six times past the long-press threshold. Every
+press was dispatched as a hold, and the handlers act on a short release:
+`MAIN_Key_MENU` returns early at the `if (bKeyHeld)` branch and never opens the
+menu. Confirmed by reading `gDebounceCounter` mid-hold — it stood at 317 after a
+3 s hold, which both proves the timeslice is running and shows the hold was far
+too long.
+
+Current values in `key.py`: `HOLD_MS = 200`, `LONG_HOLD_MS = 900`. Verified end
+to end — `key.py MENU DOWN DOWN` moves the menu from 01/79 to 03/79, and
+`key.py UP` moves it back to 02/79.
+
+If a press seems ignored, do not lengthen the hold. Check whether the handler
+wanted a short press, and check `gEeprom.KEY_LOCK` (the LCD draws a padlock when
+the keypad is locked, and ignoring keys is then correct behaviour).
+
+### Driving the menus: send a sequence as one burst
+
+Three things will make a key sequence land somewhere you did not intend. All
+three cost time here.
+
+**gdb between presses halts the guest.** Every `gdb-multiarch -batch` attach
+stops the machine for its duration. Inspecting `gMenuCursor` after each press
+stretches a six-press sequence past the 20 s menu timeout
+(`menu_timeout_500ms` in `App/misc.c`), so the UI silently falls back to the main
+screen and the rest of the presses tune the VFO instead of navigating. Send the
+whole sequence in one Python burst over QMP, then read state once at the end.
+
+**UP/DOWN are inverted inside a submenu.** `MENU_Key_UP_DOWN` flips `Direction`
+when `gIsInSubMenu` and `!gEeprom.SET_NAV` (`app/menu.c:2311`). In the list DOWN
+moves down; editing a value, UP *decreases* it. Values also clamp at
+`MENU_GetLimits` rather than wrapping, so overshooting sticks at the limit.
+
+**MENU toggles rather than only entering.** On the main screen a short MENU opens
+the menu; in the list it enters the submenu; in a submenu it commits
+(`gFlagAcceptSetting = true`) and steps back out. Two MENU presses in a row from
+the list therefore enter and immediately leave, which looks like nothing
+happened.
+
+Numeric jump: typing a menu number in the list jumps straight to it, which beats
+counting DOWN presses. Single digits are reliable. Two-digit entry needs both
+presses inside the same input-box window, and `MENU_Key_0_to_9` jumps and returns
+as soon as the first digit is a valid index (`app/menu.c:1826`), so `3` then `0`
+lands on 3 rather than 30. Pre-positioning `gMenuCursor` with gdb, in one attach
+right after opening the menu, is the reliable way to reach a distant entry.
+
+Verified this way: menu opens, DOWN/UP move the list, MENU enters a submenu, and
+a digit selects a value. Screenshots confirmed BatSav at 30/79 showing OFF.
+
+### Known limitation: power save stops the keypad scan
+
+The keypad works, but only while the radio is awake. Measured on a fresh boot:
+`gCurrentFunction` is 0 (`FUNCTION_FOREGROUND`) until about 6 s, then becomes 5
+(`FUNCTION_POWER_SAVE`) and the scan stops:
+
+    gRxIdleMode=1  gCurrentFunction=5
+
+From then on `KEYBOARD_Poll` returns `KEY_INVALID` however long or often a key is
+held — eight consecutive `key.py MENU` presses left `gKeyReading0` at 19 and
+`gScreenToDisplay` at 0. A real radio wakes on a keypress, so this is a gap in
+the model rather than firmware behaviour. The suspect is the sleep/wake path in
+`HandlePowerSave`, which calls `BK4819_Sleep()` and spins on `BK4819_REG_0C`
+bit 0 over the bit-banged bus. Note that bus shares GPIOB with the keypad (bus on
+PB8/PB9, columns PB3-PB6, rows PB12-PB15), and the model idles PB9 low as a
+deliberate hack so those reads return zero — worth a look when someone picks this
+up.
+
+Working around it is easy and does not need the gap closed: any keypad activity
+keeps the radio awake, and being in the menu blocks power save outright
+(`gScreenToDisplay != DISPLAY_MAIN` guards the entry at `app/app.c:1377`). So
+open the menu within the first few seconds of boot and a session stays usable
+indefinitely. Turning BatSav off through the UI works too, though the setting
+does not survive a restart — see below.
+
+Two dead ends, both confirmed by experiment, so nobody repeats them:
+
+- **Patching battery save in `assets/flash.img` does nothing.**
+  `SETTINGS_InitEEPROM` compares a version string at flash `0x00A160`, finds a
+  mismatch on a fresh image, and writes the settings sector.
+  `PY25Q16_WriteBuffer` erases the whole 4 KB sector before reprogramming, so a
+  byte planted at `0x00A00B` is gone before the read at `settings.c:169` sees it.
+- **Guest-side settings changes do not persist.** The emulated PY25Q16 loads the
+  image into RAM at realize time and never writes back, so anything the firmware
+  saves is lost on restart. Adding a flush would be the real fix if persistent
+  settings are ever wanted.
 
 Useful here: `tools/scan_trace.sh` (what the scan reads), `tools/key_result.sh`
-(what Poll returns), `tools/trace_run.sh` (the TRACE points, currently compiled
-in).
+(what Poll returns), `tools/trace_run.sh` (the TRACE points).
 
-There is `fprintf(stderr, "TRACE ...")` instrumentation in `qemu/py32f071.c` at
-three points. Remove it once the keypad works.
+The three `fprintf(stderr, "TRACE ...")` probes that used to sit in
+`qemu/py32f071.c` are gone -- they fired on every keypad poll and buried the
+console. If you need them back while chasing the power save gap, they went in
+`py32_gpio_set_input`, `keypad_update_rows` and `keypad_col_changed`; `git log -p
+-- qemu/py32f071.c` has the exact lines. `grep -c 'keypad row0 -> 0'` on the
+captured stderr was how the matrix got confirmed working, and it is still the
+quickest check that a press reaches the model.
+
+Note the ELF at `uvk5-sat/build/CW/nr7y.cw.elf` carries no DWARF, so gdb reports
+`'gEeprom' has unknown type`. Scalars work if you cast through their address
+(`*(unsigned short*)&gDebounceCounter`); struct fields need manual offsets.
 
 ## What this cannot do
 

README.md

@@ -3,9 +3,18 @@
 Runs Quansheng UV-K5 V3 / UV-K1 firmware on a PC. The radio uses a Puya
 PY32F071 (Cortex-M0+), which QEMU has no machine for, so this adds one.
 
-The firmware boots to its main loop in about five seconds and the LCD contents
-are readable. Keypresses reach the firmware's scan but are not yet acted on --
-see [Status](#status).
+The firmware boots to its main loop in about five seconds, the LCD contents are
+readable, and the keypad drives the menus. See [Status](#status) for what is and
+is not modelled.
+
+| Main screen | Menu | Navigated with keys |
+| --- | --- | --- |
+| ![main VFO screen](docs/screenshots/main-vfo.png) | ![menu at Step](docs/screenshots/menu-step.png) | ![menu at BatSav](docs/screenshots/menu-batsav.png) |
+
+Real captures, not mock-ups: `tools/screenshot.py` reads the firmware's
+`gFrameBuffer` out of guest memory and renders it, so these are the pixels the
+LCD driver actually wrote. Left to right: the dual-watch main screen, the menu
+opened with `key.py MENU`, and entry 30/79 reached with keypresses.
 
 ## What it is for
 
@@ -28,10 +37,25 @@ has no public datasheet, so its driver is the only specification available.
 | Boot to main loop | works, ~5 s |
 | LCD contents | readable via `tools/screenshot.py` |
 | SPI flash, settings, calibration | works |
-| Keypad matrix | rows reach the firmware's scan (`KEYBOARD_Poll` returns the right key code) but the UI does not react — under investigation |
+| Keypad and menu navigation | works while the radio is awake; power save stops the scan, see below |
 | Timing accuracy | deliberately wrong, see [Timing](#timing) |
 | Radio/RF behaviour | not modelled |
 
+A short `tools/key.py MENU` opens the menu, UP/DOWN move through it, MENU enters
+a submenu, and typing a menu number jumps straight to that entry. Press duration
+decides short versus held, which the firmware treats as different events -- see
+[Timing](#timing).
+
+The limitation is power save. Around six seconds after boot the firmware enters
+it (`gCurrentFunction` becomes `FUNCTION_POWER_SAVE`) and stops scanning the
+keypad, so keys are ignored from then on. A real radio wakes on a keypress, so
+this is a gap in the machine model rather than firmware behaviour.
+
+In practice it is not much of an obstacle: keypad activity keeps the radio awake,
+and being in the menu blocks power save entirely. Open the menu within the first
+few seconds of boot and the session stays usable. `AGENTS.md` has the details,
+including two approaches that look like fixes and are not.
+
 ## Layout
 
     qemu/                    QEMU sources to be copied into a QEMU tree
@@ -39,6 +63,7 @@ has no public datasheet, so its driver is the only specification available.
       armv7m_systick.*.patched  SysTick with the poll-boost property added
     assets/
       calibration.bin        512-byte dump from a real radio
+    docs/screenshots/        LCD captures used in this README
     tools/                   run, screenshot, inject keys, probe state
     harness/, stubs/, shim/, tests/   host build of the CW timing chain (stage A)
 
@@ -148,6 +173,17 @@ re-anchor the count — the reported value stopped changing, the firmware's
 The consequence is that guest time runs fast during any delay. Fine for
 exercising menus and control flow; wrong for judging signal timing.
 
+`poll-boost` accelerates counter **reads** only. SysTick **interrupts** still
+fire at close to real time, and those are what drive `SysTick_Handler` ->
+`gNextTimeslice` -> `APP_TimeSlice10ms` -> `CheckKeys`. So the firmware's 10 ms
+timeslice thresholds hold in wall clock: a key must be down for 20 ms to
+register and 400 ms makes it a long press.
+
+Keeping those two apart matters. `tools/key.py` originally held keys for 2500 ms
+on the assumption that guest time ran fast here too, which turned every press
+into a long press. Handlers that act on a short release — `MAIN_Key_MENU` among
+them — ignored all of it, and the keypad looked broken when it was not.
+
 ## Stage A: the CW timing chain on the host
 
 `harness/`, `stubs/`, `shim/` and `tests/` compile `app/cwkeyer.c` and
@@ -161,6 +197,15 @@ on a host would let the tests drift from what the radio runs. The debounce in
 consecutive reads to register a press, immediate release) is part of the timing
 behaviour under test.
 
+## Licence
+
+Apache 2.0, see [LICENSE](LICENSE).
+
+One exception: `qemu/py32f071.c` is licensed GPL-2.0-or-later, as its header
+states. It is built into QEMU and derives from QEMU's device models, which are
+GPL-2.0, so it cannot be anything else. The tools, harness and documentation are
+Apache 2.0.
+
 ## Credits
 
 Base firmware: [armel/uv-k1-k5v3-firmware-custom](https://github.com/armel/uv-k1-k5v3-firmware-custom).

qemu/py32f071.c

@@ -314,8 +314,6 @@ static void py32_gpio_set_input(void *opaque, int line, int level)
     if (line < 0 || line >= PY32_GPIO_PINS) {
         return;
     }
-    fprintf(stderr, "TRACE gpio%s set_input pin=%d level=%d\n",
-            s->port_name ?: "?", line, level);
     if (level) {
         s->idr |= (1u << line);
     } else {
@@ -445,8 +443,6 @@ static void keypad_update_rows(UVK5KeypadState *s)
         if (all_cols_high && s->pressed[0][r]) {
             low = true;
         }
-        fprintf(stderr, "TRACE keypad row%d -> %d (irq=%p)\n",
-                r, low ? 0 : 1, (void *)s->row_out[r]);
         qemu_set_irq(s->row_out[r], low ? 0 : 1);
     }
 }
@@ -458,7 +454,6 @@ static void keypad_col_changed(void *opaque, int line, int level)
     if (line < 1 || line >= KEYPAD_COLS) {
         return;
     }
-    fprintf(stderr, "TRACE keypad col%d level=%d\n", line, level);
     s->col_high[line] = level != 0;
     keypad_update_rows(s);
 }
@@ -737,10 +732,8 @@ static void py32_spi_class_init(ObjectClass *klass, void *data)
  * Layout from py32f071xB.h: ISR 0x00, IFCR 0x04, then per-channel blocks of
  * 0x14 starting at 0x08 (CCR, CNDTR, CPAR, CMAR).
  */
-/* Forward declarations: the DMA model clocks bytes through an SPI controller,
- * whose definition appears earlier but whose accessor is declared there. */
-typedef struct PY32SpiState PY32SpiState;
-uint8_t py32_spi_xfer_byte(PY32SpiState *s, uint8_t out);
+/* The DMA model clocks bytes through an SPI controller. Both PY32SpiState and
+ * py32_spi_xfer_byte() are already defined above, so no redeclaration here. */
 
 #define TYPE_PY32_DMA "py32-dma"
 OBJECT_DECLARE_SIMPLE_TYPE(PY32DmaState, PY32_DMA)

tools/key.py

@@ -21,16 +21,20 @@ import time
 QMP_SOCKET = "/tmp/uvk5-qmp.sock"
 KEYPAD_PATH = "/machine/keypad"
 
-# App/app/app.c debounces with key_debounce_10ms = 2 and treats
-# key_repeat_delay_10ms = 40 as a long press. Guest time runs fast under
-# emulation, so these are generous rather than exact.
-# Guest time runs fast under emulation (SysTick reads are accelerated so busy-wait
-# delays converge), so a press has to be held far longer in wall-clock terms than
-# on real hardware for the firmware's debounce to complete. Measured: 400 ms was
-# too short to register at all.
-HOLD_MS = 2500
-LONG_HOLD_MS = 6000
-GAP_MS = 1200
+# App/app/app.c debounces in 10 ms timeslices driven by the SysTick interrupt:
+#   key_debounce_10ms     =  2  -> 20 ms to register a press
+#   key_repeat_delay_10ms = 40  -> 400 ms counts as a key *held*
+#
+# SysTick *interrupts* fire at close to real time here, so these thresholds apply
+# in wall clock as written. The `poll-boost` property accelerates SysTick counter
+# *reads* (so SYSTICK_DelayUs converges); it does not speed up interrupt delivery.
+# Do not conflate the two -- an earlier HOLD_MS of 2500 assumed it did, which put
+# every press ~250 ticks past the long-press threshold. Handlers like
+# MAIN_Key_MENU act only on a short release and return early when bKeyHeld is
+# set, so the UI appeared to ignore every key.
+HOLD_MS = 200          # ~20 ticks: past debounce, well short of the 40-tick hold
+LONG_HOLD_MS = 900     # ~90 ticks: comfortably past the hold threshold
+GAP_MS = 400           # let the release be debounced before the next press
 
 KEYS = [
     "MENU", "UP", "DOWN", "EXIT", "F", "STAR",