From e7f3f9da762483e35cdf34f5f0f17b8b99eb3b6a Mon Sep 17 00:00:00 2001 From: MCKero Date: Fri, 28 Aug 2026 05:35:37 +0100 Subject: [PATCH] Hold keys for a server-side duration, not a network round trip POST /api/key now accepts {"key": "MENU", "hold_ms": 120} and holds the key for exactly that long, locally. The old design sent down and up as two requests so the browser would own press duration. That is correct on loopback and broken over a real link: the round trip between the two requests *is* the press duration. Measured against this server at 400 ms RTT, an intended tap arrived as a 407 ms hold, and since the firmware reads 400 ms as held (key_repeat_delay_10ms = 40), every short press was dispatched to the hold path where MAIN_Key_MENU does nothing. Jitter either side of that threshold is why it felt intermittent rather than simply broken. Verified at 400 ms simulated RTT: one request 531 ms total, firmware saw 120 ms -> short press two requests 816 ms total, firmware saw 408 ms -> held (the bug) hold_ms is clamped to MAX_HOLD_MS, rejects negatives and non-numbers, and defaults to TAP_MS. A deliberate 900 ms hold is preserved, so long-press events still work. down and up stay for scripting on a fast link. --- tools/test_webui.py | 50 +++++++++++++++++++++++++++++++++++++++++++++ tools/webui.py | 36 ++++++++++++++++++++++++++++---- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/tools/test_webui.py b/tools/test_webui.py index 6fc40be..4a42641 100644 --- a/tools/test_webui.py +++ b/tools/test_webui.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 """Unit tests for the web UI. Stubs the QMP client, so no emulator needed.""" +import time import unittest import webui @@ -157,5 +158,54 @@ class TestFrontEnd(unittest.TestCase): self.assertNotIn('data-key="PTT"', self.body) +class TestHoldMs(unittest.TestCase): + """Press duration must be produced by the server, not by request timing. + + At 400 ms RTT the gap between a `down` request and an `up` request is itself + ~400 ms, which the firmware reads as a held key (key_repeat_delay_10ms = 40). + Measured against the real server: an intended tap arrived as 407 ms. Holding + server-side is what makes a short press possible over a slow link. + """ + + def setUp(self): + self.client, self.http = make_app() + + def test_hold_ms_presses_and_releases(self): + resp = self.http.post("/api/key", json={"key": "MENU", "hold_ms": 120}) + self.assertEqual(resp.status_code, 200) + self.assertEqual(self.client.presses(), ["MENU", ""]) + + def test_hold_ms_is_honoured_server_side(self): + start = time.monotonic() + self.http.post("/api/key", json={"key": "MENU", "hold_ms": 150}) + elapsed = (time.monotonic() - start) * 1000 + self.assertGreaterEqual(elapsed, 140) + self.assertLess(elapsed, 400) + + def test_hold_ms_defaults_to_a_short_press(self): + resp = self.http.post("/api/key", json={"key": "UP"}) + self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.get_json()["hold_ms"], webui.TAP_MS) + + def test_hold_ms_is_clamped(self): + resp = self.http.post("/api/key", json={"key": "UP", "hold_ms": 99999}) + self.assertEqual(resp.status_code, 200) + self.assertLessEqual(resp.get_json()["hold_ms"], webui.MAX_HOLD_MS) + + def test_hold_ms_rejects_nonsense(self): + resp = self.http.post("/api/key", json={"key": "UP", "hold_ms": "soon"}) + self.assertEqual(resp.status_code, 400) + self.assertEqual(self.client.presses(), []) + + def test_hold_ms_rejects_negative(self): + resp = self.http.post("/api/key", json={"key": "UP", "hold_ms": -5}) + self.assertEqual(resp.status_code, 400) + + def test_long_hold_is_preserved_not_clamped_to_a_tap(self): + """A deliberate long press must stay long, or hold events break.""" + resp = self.http.post("/api/key", json={"key": "MENU", "hold_ms": 900}) + self.assertEqual(resp.get_json()["hold_ms"], 900) + + if __name__ == "__main__": unittest.main() diff --git a/tools/webui.py b/tools/webui.py index 7c5591e..9b2885d 100644 --- a/tools/webui.py +++ b/tools/webui.py @@ -31,10 +31,23 @@ KEYPAD_PATH = "/machine/keypad" # Firmware thresholds, from App/misc.c: # key_debounce_10ms = 2 -> 20 ms to register a press # key_repeat_delay_10ms = 40 -> 400 ms counts as HELD, a different event -# A tap has to sit between those. Real down/up events from the browser carry -# their own duration, which is why they are preferred over tap. +# +# The browser sends the duration it measured and the server holds the key for +# exactly that long. It must not be reproduced by sending `down` and `up` as two +# requests: over a slow link the round trip between them *becomes* the press +# duration. Measured against this server at 400 ms RTT, an intended tap arrived as +# a 407 ms hold, so every short press was dispatched as a held key and handlers +# like MAIN_Key_MENU did nothing. Jitter either side of the threshold is what made +# it look intermittent rather than simply broken. TAP_MS = 200 +# Below the 20 ms debounce nothing registers at all, so even a very fast click has +# to ask for at least this long. +MIN_HOLD_MS = 60 + +# A hold longer than this is a stuck key or a typo, not intent. +MAX_HOLD_MS = 5000 + BOUNDARY = "uvk5frame" TARGET_FPS = 15 @@ -86,15 +99,30 @@ def create_app(client, frame_addr: int, status_addr: int, scale: int = 4): return jsonify(error=f"unknown action {action!r}", valid=["down", "up", "tap"]), 400 + hold_raw = body.get("hold_ms") + if hold_raw is None: + hold_ms = TAP_MS + else: + try: + hold_ms = int(hold_raw) + except (TypeError, ValueError): + return jsonify( + error=f"hold_ms must be a number, got {hold_raw!r}"), 400 + if hold_ms < 0: + return jsonify(error="hold_ms must not be negative"), 400 + hold_ms = min(hold_ms, MAX_HOLD_MS) + if action == "down": set_press(key) elif action == "up": set_press("") else: + # Hold here, locally. See the note on TAP_MS: doing this as two + # requests puts the network round trip inside the press duration. set_press(key) - time.sleep(TAP_MS / 1000) + time.sleep(hold_ms / 1000) set_press("") - return jsonify(ok=True, key=key, action=action) + return jsonify(ok=True, key=key, action=action, hold_ms=hold_ms) @app.post("/api/release-all") def api_release_all():