diff --git a/AGENTS.md b/AGENTS.md index 3a0e9c4..a916734 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -461,6 +461,38 @@ The general lesson: when a firmware's mode is chosen from a byte in RAM, the tri input pin -- it is whatever wrote that byte before resetting. Find the writer in the source (`0x05DD` here) and the `#ifdef` around it, and you have the whole condition. +### Two pixels bugs behind "the other firmware looks shifted" + +Both were found by making the page *say where its picture came from*, and both had been +surviving because the wrong output looked plausible. + +**The fallback that quietly drew every frame.** `uvk5_stream.py` used `STATUS_BYTES` +without importing it, so the panel branch raised `NameError` on every frame and a bare +`except Exception: pass` swallowed it. Every screen the page drew came from guest RAM at +one firmware build's addresses: right-looking for that build, plausible and offset for any +other. Found by reporting the source and the reason (`/api/panel` answered +`source: framebuffer, note: NameError: name 'STATUS_BYTES' is not defined`). With the panel +path working, the page's `/frame.png` matches the controller's own memory 8192/8192; +before the fix it was 5594/8192 against the same memory. `tools/test_uvk5_stream.py` now +asserts that the panel wins when it is reachable, and that a fallback is announced with its +reason. + +**The column counter wrapped at 128 instead of 132.** The controller has 132 column +drivers and the glass shows 128 of them starting at column 4, which is why the model stores +pixels at `col - 4`. The counter was masked with `& 0x7f`, so addresses 128..131 came back +as 0..3, fell outside the `col >= 4` store, and were dropped: **every row lost its last four +pixels**. The battery icon lives in exactly those columns, so the symptom was a battery in +the wrong place and a picture that "looked shifted" on builds that draw to column 127 -- +while the localised build, whose rightmost four columns are blank anyway, looked fine. That +is why this read as a firmware-specific problem. Measured, before and after: filling a page +with `0xFF` left columns 124..127 blank; now they light (10/10/12/7 lit across them), and +the same firmware's frame matches the panel memory 8192/8192. + +The lesson in both cases is the same one this file keeps repeating: **a path that silently +substitutes a different source turns a hard error into a plausible wrong answer**, and a +byte that is off by four is invisible until something that matters lives in those four +columns. Report the source, and test that the preferred path is actually taken. + ## The keypad: two real bugs, both fixed The old note here said "keys reach the firmware but the UI does not react" and diff --git a/AGENTS.zh-CN.md b/AGENTS.zh-CN.md index 0593373..3a4e4cf 100644 --- a/AGENTS.zh-CN.md +++ b/AGENTS.zh-CN.md @@ -376,6 +376,30 @@ PTT+SIDE1/SIDE2 与 MENU（那是应用的几个特殊模式）、开机窗口 复位前写这个字节的那个程序。去源码里找到写入者（这里是 `0x05DD`）和它外面的 `#ifdef`，整个条件 就到手了。 +### 「换个固件就偏移」背后的两个像素 bug + +两个都是靠让**页面自己说出画面来自哪里**找到的；两个能长期存活，都是因为错的那份输出看起来"挺合理"。 + +**那条悄悄画出了每一帧的回落路径。** `uvk5_stream.py` 用了 `STATUS_BYTES` 却没导入它， +于是面板分支每一帧都抛 `NameError`，被一句裸的 `except Exception: pass` 吞掉。网页画出的每一幅 +画面都来自 guest RAM，用的是**某一份固件的地址**：装着那份固件时看着对，换别的就"貌似合理但整体 +偏移"。让它现形的办法是报出**来源与原因**（`/api/panel` 回答： +`source: framebuffer, note: NameError: name 'STATUS_BYTES' is not defined`）。面板路径通了以后， +网页的 `/frame.png` 与控制器自己的显存**逐像素相同（8192/8192）**；修复前对同一份显存只有 +**5594/8192**。`tools/test_uvk5_stream.py` 现在断言"面板可达时必须用面板"、"回落必须带原因被播报"。 + +**列计数器在 128 处回绕，而真实控制器是 132。** 控制器有 132 个列驱动，玻璃只显示其中 128 个、 +且从第 4 列开始 —— 这就是模型把像素存到 `col - 4` 的原因。但计数器被 `& 0x7f` 掩过，于是地址 +128..131 变成 0..3，落进 `col >= 4` 之外被丢弃：**每一行都丢掉了最右边 4 个像素**。而**电量图标 +恰好就在那几列**，所以症状是"电量位置不对"，以及在画到列 127 的固件上"画面看着偏了"——而那份汉化版 +最右 4 列本来就是空的，看着正常。这就是它被读成"固件相关问题"的原因。前后都量了：把一页填满 +`0xFF` 时，修复前列 124..127 全黑，修复后有内容（四列分别点亮 10/10/12/7），同一固件的画面与面板 +显存 8192/8192。 + +两次的教训是这份文件反复在说的那一句：**一条悄悄换成别的来源的路径，会把一个硬错误变成貌似合理的 +错答案**；而一个偏了四列的字节，在"重要的东西恰好住在那四列里"之前，是看不出来的。要报出来源， +并且要测"该走的那条优选路径确实被走了"。 + ## 键盘：两个真 bug，都已修复 这里原来的笔记写的是"按键到达了固件但界面不反应"，并且归咎于机器模型。结果发现有**两个 diff --git a/qemu/py32f071.c b/qemu/py32f071.c index e2e77a7..3da31c6 100644 --- a/qemu/py32f071.c +++ b/qemu/py32f071.c @@ -814,7 +814,7 @@ static uint8_t st7565_xfer(void *opaque, uint8_t out) { const char *panel_probe = g_getenv("UVK5_PANEL_PROBE"); static unsigned panel_probe_n; - if (panel_probe && panel_probe_n < 600) { + if (panel_probe && panel_probe_n < 40000) { FILE *f = fopen(panel_probe, "a"); if (f) { fprintf(f, "PANEL a0=%d cs=%d page=%d col=%d byte=%02x\n", @@ -833,7 +833,17 @@ static uint8_t st7565_xfer(void *opaque, uint8_t out) if (s->col >= 4 && s->col < 132) { s->gram[s->page & 7][s->col - 4] = out; } - s->col = (s->col + 1) & 0x7f; + /* + * The controller's column counter runs 0..131 -- it has 132 column drivers, + * and the glass shows 128 of them starting at 4, which is why the store above + * subtracts 4. Masking the counter to seven bits made it wrap at 128 instead: + * the four bytes addressed 128..131 were then re-read as 0..3, fell outside + * the store, and were dropped. Every row lost its last four pixels -- and the + * battery icon lives in the rightmost columns, so the symptom was a wrong + * battery and a picture that looked shifted, on builds that draw to column + * 127. Measured: filling a whole page with 0xFF left columns 124..127 blank. + */ + s->col = (s->col + 1) % 132; return 0xff; } diff --git a/tools/test_uvk5_stream.py b/tools/test_uvk5_stream.py index b72e172..8a62472 100644 --- a/tools/test_uvk5_stream.py +++ b/tools/test_uvk5_stream.py @@ -5,6 +5,7 @@ import threading import time import unittest +from uvk5_lcd import LCD_HEIGHT, LCD_WIDTH, STATUS_BYTES, TOTAL_ROWS, unpack from uvk5_stream import FramePump @@ -130,5 +131,66 @@ class TestFramePump(unittest.TestCase): self.assertTrue(wait_for_frame(pump), "did not resume after rebind") +class FakePanel: + """A grabber whose panel works, and whose guest RAM must never be touched. + + read() raises, so any test that silently takes the fallback fails loudly instead of + comparing two wrong pictures -- which is how the bug below stayed invisible: the + fake client used by the tests above answers memsave and nothing else, so the panel + branch failed there too, the bare except swallowed it, and every one of them was + really exercising the fallback. + """ + + def __init__(self): + self.gram = bytes((i * 7) & 0xFF for i in range(TOTAL_ROWS * LCD_WIDTH)) + + def panel_gram(self): + return self.gram + + def panel_pixels(self): + return unpack(self.gram[:STATUS_BYTES], self.gram[STATUS_BYTES:]) + + def raw(self): + raise AssertionError("guest RAM was read while the panel was available") + + +class TestFrameSource(unittest.TestCase): + """Which memory the page draws is not a detail -- it decides whose screen is right. + + The panel is the only firmware-independent source: every build pushes pixels through + the same controller, while guest RAM is correct only for the build whose buffer + addresses were passed in. So when the panel is reachable it must be used. + + uvk5_stream.py used STATUS_BYTES without importing it. The panel branch therefore + raised NameError on every frame, a bare except swallowed it, and every screen the + page drew came from guest RAM at one firmware's addresses -- right-looking for that + firmware, plausible and offset for any other. These two tests fail on that code. + """ + + def test_the_panel_is_used_when_it_is_there(self): + pump = FramePump(None, 0, 0) + status, frame, pixels = pump._grab(FakePanel()) + self.assertEqual(pump.source()[0], "panel", + "the panel is reachable, so guest RAM must not be used") + self.assertIsNone(pump.source()[1]) + self.assertEqual(len(status), STATUS_BYTES) + self.assertEqual(len(frame), TOTAL_ROWS * LCD_WIDTH - STATUS_BYTES) + # unpack() returns one entry per LCD line, not per page: eight pages of eight. + self.assertEqual(len(pixels), LCD_HEIGHT) + self.assertEqual(len(pixels[0]), LCD_WIDTH) + + def test_a_broken_panel_falls_back_and_says_why(self): + notes = [] + pump = FramePump(None, 0, 0, on_fallback=notes.append) + broken = FakePanel() + broken.panel_gram = lambda: (_ for _ in ()).throw(RuntimeError("no panel model")) + broken.raw = lambda: (b"\x00" * STATUS_BYTES, + b"\x00" * (TOTAL_ROWS * LCD_WIDTH - STATUS_BYTES)) + pump._grab(broken) + self.assertEqual(pump.source()[0], "framebuffer") + self.assertIn("no panel model", pump.source()[1]) + self.assertEqual(len(notes), 1, "the fallback is announced once, with its reason") + + if __name__ == "__main__": unittest.main() diff --git a/tools/uvk5_stream.py b/tools/uvk5_stream.py index 0b7ff2b..9d1bcaa 100644 --- a/tools/uvk5_stream.py +++ b/tools/uvk5_stream.py @@ -17,12 +17,24 @@ looks live. import threading import time -from uvk5_lcd import FrameGrabber, default_spool_dir, encode_png, unpack +# STATUS_BYTES is not decoration: _grab() splits the controller's memory into the +# status line and the frame with it. It was missing from this import for the whole +# life of the panel path, so the panel branch raised NameError on every frame, the +# bare except below swallowed it, and every picture the page ever drew came from the +# guest-RAM fallback instead -- which needs *that build's* buffer addresses. With the +# CN addresses and a CN firmware that looked right; with any other firmware the screen +# was plausible and offset, which is exactly how it was reported. +from uvk5_lcd import STATUS_BYTES, FrameGrabber, default_spool_dir, encode_png, unpack class FramePump: def __init__(self, client, frame_addr: int, status_addr: int, - fps: int = 15, scale: int = 4, spool_dir: str = None): + fps: int = 15, scale: int = 4, spool_dir: str = None, + on_fallback=None): + # Called once with the reason the first time a frame has to come from guest RAM. + # It used to be swallowed whole, which is how a NameError in the panel branch + # went unnoticed for as long as the fallback kept producing plausible pictures. + self._on_fallback = on_fallback self._frame_addr = frame_addr self._status_addr = status_addr # Resolved by FrameGrabber: /dev/shm on Linux, the temp directory on @@ -36,6 +48,13 @@ class FramePump: self._png = None self._raw = None self._generation = 0 + # Which memory the pixels came from. The panel is preferred and is the only + # source that is firmware-independent; the guest-RAM fallback needs that + # build's own buffer addresses, so getting it wrong shows up as a picture + # that is plausible but offset. Reporting it is what turns "the screen looks + # wrong" into "it came from the fallback, and here is why". + self._source = None + self._note = None self._stop = threading.Event() self._thread = None @@ -91,6 +110,11 @@ class FramePump: self._stop.wait(slack) + def source(self): + """(which memory the last frame came from, why the fallback happened).""" + with self._lock: + return self._source, self._note + def _grab(self, grabber): """One frame: (status, frame, pixels), from the panel if it is there. @@ -105,8 +129,21 @@ class FramePump: try: pixels = grabber.panel_pixels() gram = grabber.panel_gram() + self._source = "panel" + self._note = None return gram[:STATUS_BYTES], gram[STATUS_BYTES:], pixels - except Exception: + except Exception as exc: + # Remember the first reason rather than overwriting it every frame: the + # cause does not change while the emulator runs, and the first one is + # the informative one. + if self._source != "framebuffer": + self._note = "%s: %s" % (type(exc).__name__, exc) + if self._on_fallback is not None: + try: + self._on_fallback(self._note) + except Exception: + pass + self._source = "framebuffer" status, frame = grabber.raw() return status, frame, unpack(status, frame) def latest(self): diff --git a/tools/webui.py b/tools/webui.py index bb704b1..4a99f1b 100644 --- a/tools/webui.py +++ b/tools/webui.py @@ -154,7 +154,10 @@ def create_app(client, frame_addr: int, status_addr: int, scale: int = 4, # One background grabber for every client. client may be None: the emulator # can be powered off, and the page still has to load. - pump = FramePump(client, frame_addr, status_addr, fps=TARGET_FPS, scale=scale) + pump = FramePump(client, frame_addr, status_addr, fps=TARGET_FPS, scale=scale, + on_fallback=lambda note: log.add( + "qemu", "panel unavailable, drawing from guest RAM at " + f"0x{frame_addr:08X}: {note}")) pump.start() app.config["PUMP"] = pump app.config["SUPERVISOR"] = supervisor @@ -270,7 +273,34 @@ def create_app(client, frame_addr: int, status_addr: int, scale: int = 4, # The emulator can die under us; that is a state to report, not a 500. return jsonify(powered=False, status="unreachable", error=str(exc)) return jsonify(powered=True, speaker=speaker_on(), - panel=panel_state(), firmware=firmware_info(), **info) + panel=panel_state(), firmware=firmware_info(), + frame_source=pump.source()[0], **info) + + @app.get("/api/panel") + def api_panel(): + """The display controller's own memory, and where /frame.png came from. + + /frame.png prefers the panel and falls back to guest RAM when the panel model + is not there. The fallback needs the addresses of *that* build's buffers, so + it is the path that can look right and be offset at the same time -- and a + caller cannot tell which it got from the picture. This says which, gives the + reason for a fallback, and hands back the controller's own bytes so the two + can be compared without guessing. + """ + source, note = pump.source() + body = {"source": source, "note": note, + "frame_addr": frame_addr, "status_addr": status_addr} + target = active_client() + if target is None: + return jsonify(body, powered=False) + try: + body["gram"] = target.command("qom-get", path=PANEL_PATH, property="gram") + body["invert"] = bool(target.command("qom-get", path=PANEL_PATH, + property="invert")) + body["bytes"] = len(body["gram"]) // 2 + except Exception as exc: + body["panel_error"] = str(exc) + return jsonify(body) @app.get("/api/firmware") def api_firmware():