Add a frame grabber that reads the LCD over QMP memsave

memsave, not pmemsave. The plan specified pmemsave on the strength of a timing
measurement that never checked the contents; it turns out pmemsave takes a
*physical* address and silently returns zeros for gFrameBuffer's virtual
address. No error, no warning -- just a permanently blank screen.

Caught by rendering a real frame and finding 0 lit pixels where the gdb path
reported 1693. With memsave the count matches exactly, and the image reads
correctly: both VFOs at 18.00000 MHz, PS/DWR/CL status bar.

Two tests guard the decisions rather than the current text: the stub client
raises if pmemsave is ever used, and a source check rejects subprocess/popen so
frame reads cannot regress onto gdb, which would halt the guest.

Measured 1.35 ms per frame with the guest still reporting status running.

tools/test_uvk5_lcd.py

@@ -1,10 +1,13 @@
 #!/usr/bin/env python3
 """Unit tests for LCD unpacking and PNG encoding. No emulator needed."""
+import os
 import struct
+import tempfile
 import unittest
 import zlib
 
-from uvk5_lcd import LCD_HEIGHT, LCD_WIDTH, encode_png, unpack
+from uvk5_lcd import (FRAME_BYTES, LCD_HEIGHT, LCD_WIDTH, STATUS_BYTES,
+                      FrameGrabber, encode_png, unpack)
 
 
 class TestUnpack(unittest.TestCase):
@@ -78,5 +81,95 @@ class TestEncodePng(unittest.TestCase):
         self.assertEqual(raw[2], 0xFF)       # neighbour -> white
 
 
+class StubClient:
+    """Stands in for QmpClient; writes the files memsave would write.
+
+    Rejects pmemsave on purpose. pmemsave takes a *physical* address and returns
+    zeros for the framebuffer's virtual address -- a blank screen with no error
+    reported anywhere, which is exactly the bug this stub is here to catch.
+    """
+
+    def __init__(self):
+        self.calls = []
+
+    def command(self, name, **args):
+        self.calls.append((name, args))
+        if name == "pmemsave":
+            raise AssertionError(
+                "pmemsave reads physical addresses and silently returns zeros "
+                "for gFrameBuffer; use memsave")
+        if name != "memsave":
+            raise AssertionError(f"unexpected command {name}")
+        with open(args["filename"], "wb") as fh:
+            fh.write(bytes(args["size"]))
+        return {}
+
+
+class TestFrameGrabber(unittest.TestCase):
+    def test_uses_memsave_and_returns_png(self):
+        client = StubClient()
+        tmp = tempfile.mkdtemp()
+        grabber = FrameGrabber(client, frame_addr=0x200013DC,
+                               status_addr=0x2000175C, spool_dir=tmp)
+        png = grabber.png(scale=2)
+
+        self.assertTrue(png.startswith(b"\x89PNG\r\n\x1a\n"))
+        self.assertEqual([c[0] for c in client.calls], ["memsave", "memsave"])
+
+    def test_reads_the_right_addresses_and_sizes(self):
+        client = StubClient()
+        grabber = FrameGrabber(client, frame_addr=0x200013DC,
+                               status_addr=0x2000175C,
+                               spool_dir=tempfile.mkdtemp())
+        grabber.raw()
+
+        by_addr = {args["val"]: args["size"] for _, args in client.calls}
+        self.assertEqual(by_addr[0x200013DC], FRAME_BYTES)
+        self.assertEqual(by_addr[0x2000175C], STATUS_BYTES)
+
+    def test_raw_returns_status_then_frame(self):
+        client = StubClient()
+        grabber = FrameGrabber(client, frame_addr=0x1000, status_addr=0x2000,
+                               spool_dir=tempfile.mkdtemp())
+        status, frame = grabber.raw()
+        self.assertEqual(len(status), STATUS_BYTES)
+        self.assertEqual(len(frame), FRAME_BYTES)
+
+    def test_never_uses_pmemsave(self):
+        """Regression guard: pmemsave silently reads the wrong memory.
+
+        The framebuffer symbols are CPU virtual addresses. pmemsave treats its
+        argument as physical and returns zeros, so the screen renders blank with
+        no error raised. Verified against a live emulator: pmemsave gave 0 lit
+        bits, memsave gave 1693, matching the gdb path exactly.
+        """
+        source = open(os.path.join(os.path.dirname(os.path.abspath(__file__)),
+                                  "uvk5_lcd.py")).read()
+        code = "\n".join(line for line in source.splitlines()
+                          if not line.lstrip().startswith("#")
+                          and not line.lstrip().startswith("*"))
+        # Allow the word inside the explanatory docstring, forbid a real call.
+        self.assertNotIn('command("pmemsave"', code)
+
+    def test_does_not_invoke_gdb(self):
+        """Guard the design decision, not just the current behaviour.
+
+        A gdb attach halts the guest, which would both stutter a live stream and
+        perturb key debounce timing (see AGENTS.md). pmemsave leaves the guest
+        running -- measured ~1.3 ms per frame with status still "running".
+
+        Checks for the machinery needed to shell out, not for the word "gdb":
+        the module mentions gdb in prose precisely to explain why it is avoided.
+        """
+        source = open(os.path.join(os.path.dirname(os.path.abspath(__file__)),
+                                  "uvk5_lcd.py")).read()
+        code = "\n".join(line for line in source.splitlines()
+                         if not line.lstrip().startswith("#"))
+        for forbidden in ("subprocess", "os.system", "popen", "gdb-multiarch"):
+            self.assertNotIn(forbidden, code.lower(),
+                             f"{forbidden} must not appear: reading frames has "
+                             "to stay on the QMP path")
+
+
 if __name__ == "__main__":
     unittest.main()

tools/uvk5_lcd.py

@@ -6,6 +6,7 @@ The firmware keeps the display in gStatusLine (page 0) and gFrameBuffer
 the layout the ST7565 expects. Extracted from tools/screenshot.py so the web UI
 and the CLI screenshotter cannot drift apart.
 """
+import os
 import struct
 import zlib
 
@@ -56,3 +57,44 @@ def encode_png(pixels, scale: int = 4) -> bytes:
             + chunk(b"IHDR", struct.pack(">IIBBBBB", width, height, 8, 0, 0, 0, 0))
             + chunk(b"IDAT", zlib.compress(bytes(raw), 6))
             + chunk(b"IEND", b""))
+
+
+class FrameGrabber:
+    """Reads the LCD out of guest memory over QMP.
+
+    memsave, not pmemsave. The framebuffer symbols are CPU virtual addresses;
+    pmemsave interprets its argument as a *physical* address and silently returns
+    a block of zeros for these, which renders as a blank screen with no error
+    anywhere. memsave takes the virtual address and returns the real contents --
+    verified against the gdb path, both reporting 1693 lit bits on the same frame.
+
+    QMP, not gdb: measured ~1.35 ms per frame with the guest still reporting
+    status "running". The gdb path used by screenshot.py halts the guest on every
+    attach, which is unusable for a live stream and also perturbs key debounce
+    timing (see AGENTS.md). Do not reintroduce gdb here.
+    """
+
+    def __init__(self, client, frame_addr: int, status_addr: int,
+                 spool_dir: str = "/dev/shm"):
+        self._client = client
+        self._frame_addr = frame_addr
+        self._status_addr = status_addr
+        # pmemsave writes to a path, so a tmpfs avoids disk I/O every frame.
+        self._frame_path = os.path.join(spool_dir, "uvk5-frame.bin")
+        self._status_path = os.path.join(spool_dir, "uvk5-status.bin")
+
+    def raw(self) -> tuple[bytes, bytes]:
+        """Return (status, frame) exactly as the firmware holds them."""
+        self._client.command("memsave", val=self._frame_addr,
+                             size=FRAME_BYTES, filename=self._frame_path)
+        self._client.command("memsave", val=self._status_addr,
+                             size=STATUS_BYTES, filename=self._status_path)
+        with open(self._frame_path, "rb") as fh:
+            frame = fh.read(FRAME_BYTES)
+        with open(self._status_path, "rb") as fh:
+            status = fh.read(STATUS_BYTES)
+        return status, frame
+
+    def png(self, scale: int = 4) -> bytes:
+        status, frame = self.raw()
+        return encode_png(unpack(status, frame), scale)