mirror of
https://github.com/saphid/frame-control.git
synced 2026-10-06 06:00:33 +02:00
Control: fixes from the SWE-2 Max review
- The Frame side tracks keys as well as buttons and lets go of both when the session ends. - A stale tap tells the page, which re-reads the panels at once. - A paused input device waits instead of ending the session; only a disconnect does. An OS error on one event skips it. - Presses check focus with two property reads and do the full lookup only when it changed. - Writes to an agent's stdin are serialized, so two devices sending at once can't tear a line (the keyboard agent too). - Connecting gives up with a message after 15 s instead of hanging on "Connecting…"; text goes in 100-character pieces so releases don't wait behind a long paste; a cancelled mouse gesture releases what's held. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
ac07efa8f1
commit
597f98af6d
5 files changed
+72
-16
No files matched your search
@@ -217,6 +217,10 @@ How (**verified 2026-09-29**, SteamOS 0.4.1, build 20260925.6191901):
|
||||
gets a connector of its own and doesn't hold focus.
|
||||
- **Keys in a burst can arrive out of order**, so the helper paces them (8 ms
|
||||
apart).
|
||||
- **Known limit:** if focus moves to another panel in the middle of a drag, the
|
||||
release goes to the panel that has focus then. Whether gamescope hands it to
|
||||
the window that got the press isn't known yet. When the session ends, the
|
||||
helper lets go of every button and key it still holds.
|
||||
- **The Desktop picture is the window's own pixels**: `ffmpeg -f x11grab
|
||||
-window_id <window> -i :<display>` works on gamescope's redirected windows,
|
||||
while grabbing the root gives black. It streams as H.264 like the headset view
|
||||
|
||||
+25
-3
@@ -89,11 +89,13 @@ class Apply(unittest.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
self.focused = {"window": 7, "display": ":1", "root": [1280, 720], "width": 1280, "height": 720, "name": "x"}
|
||||
self.old_focus, self.old_say = self.t.focus, self.t.say
|
||||
saved = self.t.focus, self.t.say, self.t.focus_now
|
||||
self.said = []
|
||||
self.t.focus = lambda: dict(self.focused)
|
||||
self.t.focus_now = lambda: (self.focused["window"], self.focused["display"])
|
||||
self.t.say = lambda state, **more: self.said.append((state, more))
|
||||
self.addCleanup(lambda: (setattr(self.t, "focus", self.old_focus), setattr(self.t, "say", self.old_say)))
|
||||
self.addCleanup(lambda: (setattr(self.t, "focus", saved[0]), setattr(self.t, "say", saved[1]),
|
||||
setattr(self.t, "focus_now", saved[2])))
|
||||
|
||||
def test_tap_moves_then_clicks_in_order(self):
|
||||
gs, panel = FakeGamescope(), None
|
||||
@@ -120,10 +122,12 @@ class Apply(unittest.TestCase):
|
||||
panel = self.t.apply(gs, {"fx": 0.5, "fy": 0.5, "window": 7, "display": ":1"}, None)
|
||||
panel = self.t.apply(gs, {"fx": 0.6, "fy": 0.5, "window": 7, "display": ":1"}, panel)
|
||||
self.assertEqual(len(calls), 1)
|
||||
self.t.apply(gs, {"button": "left", "down": True, "window": 7, "display": ":1"}, panel)
|
||||
self.assertEqual(len(calls), 1) # same panel still: the quick check was enough
|
||||
self.focused["window"] = 8
|
||||
self.t.apply(gs, {"button": "left", "down": True, "window": 7, "display": ":1"}, panel)
|
||||
self.assertEqual(len(calls), 2)
|
||||
self.assertEqual([c[0] for c in gs.calls], ["move_to", "move_to"])
|
||||
self.assertEqual([c[0] for c in gs.calls], ["move_to", "move_to", "button"])
|
||||
|
||||
def test_same_window_id_on_the_other_display_is_another_panel(self):
|
||||
gs = FakeGamescope()
|
||||
@@ -137,6 +141,24 @@ class Apply(unittest.TestCase):
|
||||
self.t.apply(gs, e, None)
|
||||
self.assertEqual([c[0] for c in gs.calls], ["move_by", "scroll", "key", "text"])
|
||||
|
||||
def test_everything_held_is_let_go(self):
|
||||
class Held(self.t.Gamescope):
|
||||
def __init__(self):
|
||||
self.held, self.keys, self.log = set(), set(), []
|
||||
|
||||
def frame(self):
|
||||
pass
|
||||
|
||||
g = Held()
|
||||
g.L = type("L", (), {"ei_device_button_button": lambda *a: g.log.append(("button",) + a[2:]),
|
||||
"ei_device_keyboard_key": lambda *a: g.log.append(("key",) + a[2:])})()
|
||||
g.device = object()
|
||||
g.button("left", True)
|
||||
g.key(42, True)
|
||||
g.release_all()
|
||||
self.assertEqual(g.log[-2:], [("button", 0x110, False), ("key", 42, False)])
|
||||
self.assertEqual((g.held, g.keys), (set(), set()))
|
||||
|
||||
def test_text_uses_shift_for_capitals(self):
|
||||
class Keys(self.t.Gamescope):
|
||||
def __init__(self):
|
||||
|
||||
+28
-7
@@ -140,6 +140,12 @@ def focus_display():
|
||||
return name if name[:1] == ":" and name[1:].isdigit() else None
|
||||
|
||||
|
||||
def focus_now():
|
||||
"""Just which window and display have focus: two property reads, for checking a press."""
|
||||
window = (xprop_root(":0", "GAMESCOPE_FOCUSED_WINDOW") or [0])[0]
|
||||
return (window or None, focus_display() if window else None)
|
||||
|
||||
|
||||
def focus():
|
||||
"""The panel that has focus in the headset: window, display, name and sizes (gamescope
|
||||
publishes the window and its display on :0's root)."""
|
||||
@@ -222,7 +228,7 @@ class Gamescope:
|
||||
if L.ei_setup_backend_socket(self.ei, SOCKET.encode()) != 0:
|
||||
raise RuntimeError("Couldn't reach gamescope's input socket. Is the headset on?")
|
||||
self.fd = L.ei_get_fd(self.ei)
|
||||
self.device, self.sequence, self.held = None, 0, set()
|
||||
self.device, self.sequence, self.held, self.keys, self.alive = None, 0, set(), set(), True
|
||||
|
||||
def pump(self, wait=0.0):
|
||||
"""Handle gamescope's events; False once it has disconnected."""
|
||||
@@ -249,7 +255,7 @@ class Gamescope:
|
||||
if self.L.ei_event_get_device(ev) == self.device:
|
||||
self.device = None
|
||||
elif kind == EV_DISCONNECT:
|
||||
self.device, alive = None, False
|
||||
self.device, alive, self.alive = None, False, False
|
||||
self.L.ei_event_unref(ev)
|
||||
|
||||
def wait_ready(self, timeout=5):
|
||||
@@ -258,7 +264,8 @@ class Gamescope:
|
||||
if not self.pump(0.1):
|
||||
break
|
||||
if self.device is None:
|
||||
raise RuntimeError("gamescope didn't offer an input device")
|
||||
raise RuntimeError("gamescope closed its input socket" if not self.alive
|
||||
else "gamescope didn't offer an input device")
|
||||
|
||||
def frame(self):
|
||||
self.L.ei_device_frame(self.device, self.L.ei_now(self.ei))
|
||||
@@ -287,6 +294,7 @@ class Gamescope:
|
||||
def key(self, code, down):
|
||||
self.L.ei_device_keyboard_key(self.device, code, down)
|
||||
self.frame()
|
||||
(self.keys.add if down else self.keys.discard)(code)
|
||||
# Paced: a burst of keys can reach the app out of order (seen 2026-09-29).
|
||||
time.sleep(0.008)
|
||||
|
||||
@@ -303,9 +311,12 @@ class Gamescope:
|
||||
self.key(SHIFT, False)
|
||||
|
||||
def release_all(self):
|
||||
"""Let go of every button and key still down, so nothing stays held in the headset."""
|
||||
for code in list(self.held):
|
||||
name = next(n for n, c in BUTTONS.items() if c == code)
|
||||
self.button(name, False)
|
||||
for code in list(self.keys):
|
||||
self.key(code, False)
|
||||
|
||||
|
||||
# ---- events from the server --------------------------------------------------------
|
||||
@@ -341,8 +352,11 @@ def apply(gs, event, panel):
|
||||
# Moves may use a focus reading up to a second old; anything that acts (a press, key,
|
||||
# text or scroll) reads it afresh, so it can't land on a panel that took focus since.
|
||||
acts = any(k in event for k in ("button", "key", "text", "scroll")) and event.get("down") is not False
|
||||
if "window" in event and (acts or not panel or time.time() - panel.get("_at", 0) > 1 or aimed_elsewhere(event, panel)):
|
||||
panel = {**focus(), "_at": time.time()}
|
||||
if "window" in event:
|
||||
if panel and acts and focus_now() == (panel.get("window"), panel.get("display")):
|
||||
panel = {**panel, "_at": time.time()} # still the same panel: keep its geometry
|
||||
elif acts or not panel or time.time() - panel.get("_at", 0) > 1 or aimed_elsewhere(event, panel):
|
||||
panel = {**focus(), "_at": time.time()}
|
||||
stale = aimed_elsewhere(event, panel) if "window" in event else False
|
||||
if stale and not (event.get("down") is False and ("button" in event or "key" in event)):
|
||||
say("ready", focus=panel.get("window"), display=panel.get("display"), stale=True) # the page re-syncs
|
||||
@@ -392,10 +406,17 @@ def main():
|
||||
for line in lines:
|
||||
for event in events(line):
|
||||
if gs.device is None:
|
||||
gs.wait_ready(2)
|
||||
# Paused (gamescope can pause the device): wait a moment; drop this
|
||||
# event if it doesn't come back. Only a disconnect ends the session.
|
||||
try:
|
||||
gs.wait_ready(2)
|
||||
except RuntimeError:
|
||||
if not gs.alive:
|
||||
raise
|
||||
continue
|
||||
try:
|
||||
panel = apply(gs, event, panel)
|
||||
except (ValueError, KeyError, TypeError):
|
||||
except (ValueError, KeyError, TypeError, OSError):
|
||||
continue # the server checks events; skip anything odd
|
||||
except RuntimeError as e:
|
||||
say("error", message=str(e))
|
||||
|
||||
+10
-3
@@ -1508,8 +1508,12 @@ async function ctrlPoll(n) {
|
||||
ctrl.state = r.state; ctrl.message = r.message || "";
|
||||
} catch (e) { ctrl.state = "error"; ctrl.message = e.message; }
|
||||
ctrlShow();
|
||||
if (ctrl.state !== "ready" && ctrl.state !== "error" && n < 30) setTimeout(() => ctrlPoll(n + 1), 500);
|
||||
else if (ctrl.state === "ready") ctrlFlush();
|
||||
if (ctrl.state !== "ready" && ctrl.state !== "error" && n < 30) return setTimeout(() => ctrlPoll(n + 1), 500);
|
||||
if (ctrl.state === "ready") return ctrlFlush();
|
||||
if (ctrl.state !== "error") {
|
||||
ctrl.state = "error"; ctrl.message = "The Frame didn't answer. Turn Control off and on to try again.";
|
||||
ctrlShow();
|
||||
}
|
||||
}
|
||||
$("viewer").tabIndex = 0;
|
||||
$("viewer").addEventListener("focus", () => { if (ctrl.on && window.frameApp?.captureKeys) window.frameApp.captureKeys(true); });
|
||||
@@ -1561,6 +1565,7 @@ async function ctrlFlush() {
|
||||
try {
|
||||
const r = await api("/api/touch", { events: batch });
|
||||
ctrl.state = r.state; ctrl.message = r.message || "";
|
||||
if (r.stale && typeof loadPanels === "function") loadPanels(); // focus moved on: catch up now
|
||||
if (!r.sent && r.state === "error") {
|
||||
// Broken: presses would be stale by the time it's back; releases still matter.
|
||||
ctrl.queue.unshift(...batch.filter(isRelease));
|
||||
@@ -1738,6 +1743,7 @@ canvasEl.addEventListener("pointermove", e => {
|
||||
for (const type of ["pointerup", "pointercancel"]) canvasEl.addEventListener(type, e => {
|
||||
if (!ctrl.on) return;
|
||||
if (e.pointerType === "touch") return type === "pointercancel" ? ctrlTouchCancel(e) : ctrlTouchUp(e);
|
||||
if (type === "pointercancel") return [...ctrl.held].forEach(b => ctrlButton(b, false)); // no button is named
|
||||
const name = CTRL_BUTTONS[e.button] || "left";
|
||||
if (ctrl.held.has(name)) ctrlButton(name, false);
|
||||
});
|
||||
@@ -1772,7 +1778,8 @@ function ctrlText(text) {
|
||||
ctrlWarned = true;
|
||||
toast("Control types plain letters, numbers and symbols. For accents and emoji, use Keyboard and trackpad below.");
|
||||
}
|
||||
for (const part of plain.match(/[\s\S]{1,500}/g) || []) ctrlSend([{ text: part }]);
|
||||
// Small pieces: the Frame types them key by key, and a release queued behind mustn't wait long.
|
||||
for (const part of plain.match(/[\s\S]{1,100}/g) || []) ctrlSend([{ text: part }]);
|
||||
}
|
||||
$("ctrlType").addEventListener("beforeinput", e => {
|
||||
if (e.isComposing) return;
|
||||
|
||||
+5
-3
@@ -616,7 +616,7 @@ class InputAgent:
|
||||
"""
|
||||
|
||||
def __init__(self, source=HERE / "frame_input_agent.py", packages=None):
|
||||
self.source, self.proc, self.lock = source, None, threading.Lock()
|
||||
self.source, self.proc, self.lock, self.write_lock = source, None, threading.Lock(), threading.Lock()
|
||||
self.packages = kdeconnect_packages() if packages is None else packages
|
||||
# generation counts stop()s; launching is the generation a launch is under way for.
|
||||
self.status, self.launching, self.generation = {"state": "off"}, None, 0
|
||||
@@ -789,8 +789,10 @@ class InputAgent:
|
||||
self.start()
|
||||
elif ready and events:
|
||||
try:
|
||||
proc.stdin.write((json.dumps(events) + "\n").encode())
|
||||
proc.stdin.flush()
|
||||
# One writer at a time: two devices sending at once mustn't tear a line.
|
||||
with self.write_lock:
|
||||
proc.stdin.write((json.dumps(events) + "\n").encode())
|
||||
proc.stdin.flush()
|
||||
sent = True
|
||||
except (BrokenPipeError, OSError, ValueError):
|
||||
pass # _watch reports how it ended
|
||||
|
||||
Reference in new issue
Block a user