feat(input): SteamOS Game Mode support, Steam Controller exit chord, and controller grouping improvements - #53
Conversation
…tion to helper daemon
hikaps
left a comment
There was a problem hiding this comment.
The security-critical part is solid: raw-HID reads bounded (uint8_t buf[64], every access guarded by bytesRead >= 10), isValidDevicePath rejects .. + enforces /dev/input|/dev/hidraw\d+ + char-device check, WatchDevice polkit-gated like ChangeDeviceOwner, and the VirtualDeviceWatcher removal is clean (CMake/includes/members/call-sites all gone, suite updated). None of the inline items are architectural — requesting changes for those.
Cross-PR (with #46): install.sh merges cleanly (disjoint regions). But both PRs add a SteamOS update script — update-sysext.sh here vs update-nonroot.sh in #46 — divergent; keep one (this one's the faster loop; fold #46's Flatpak-source rebuild in as a flag) and drop the other. Suggested order: #46 first, then this rebases + dedups + adds the gamemode launcher to packaging.
| echo "Launching CouchPlay..." | ||
|
|
||
| # Set environment so CouchPlay connects to the nested KWin's Wayland display | ||
| # WAYLAND_DISPLAY is inherited from kwin_wayland's nested output |
There was a problem hiding this comment.
The nested kwin is never actually used. kwin_wayland … & is backgrounded, then only export QT_QPA_PLATFORM=wayland runs — WAYLAND_DISPLAY is never set to kwin's socket (a backgrounded child can't export env back to its parent; the "inherited" comment here is impossible). CouchPlay lands on the parent gamescope socket, so the KWin window-positioning this script exists for never applies. Capture kwin's socket (--wayland-display <name> + export WAYLAND_DISPLAY=<name>) before launching CouchPlay.
|
|
||
| [Desktop Action GameMode] | ||
| Name=CouchPlay (Game Mode) | ||
| Exec=couchplay-gamemode |
There was a problem hiding this comment.
This desktop action ships nothing: beta.yml/release.yml only package scripts/install-helper.sh from scripts/, so couchplay-gamemode.sh is never in the tarball, and it's not on PATH inside the Flatpak (the primary distribution). The Exec=couchplay-gamemode is a dead action. Either add the script to CI packaging and provide a Flatpak-spawning Exec, or drop the action until then.
| return; | ||
| } | ||
|
|
||
| QFile logFile(QStringLiteral("/home/deck/couchplay.log")); |
There was a problem hiding this comment.
Hardcoded /home/deck/couchplay.log opened unbounded-append on every message; in the Flatpak that path isn't the real home (open silently fails), and where it works it grows without bound + persists runtime detail. Looks like leftover debug — move to QStandardPaths::GenericDataLocation and gate behind a debug flag.
| } | ||
|
|
||
| // 1. Write the temporary udev rule assigning this device/subsystem to the target user | ||
| if (writeTempUdevRule(hid, QString::fromLocal8Bit(pw->pw_name))) { |
There was a problem hiding this comment.
pw->pw_name is passed to writeTempUdevRule without the project's s_validUsername regex (^[a-z][a-z0-9_-]{0,31}$). The ChangeDeviceOwner path never calls validateUserAndAuth, unlike the username-taking slots. A username with a quote/comma/newline (feasible via NSS/LDAP) could corrupt the rule or inject a RUN+= key parsed by the privileged udev daemon. Low likelihood on a gaming box, but cheap + correct — sanitize before it reaches the rule.
| watcher->notifier = new QSocketNotifier(fd, QSocketNotifier::Read, this); | ||
| watcher->chordTimer = new QTimer(this); | ||
| watcher->chordTimer->setSingleShot(true); | ||
| watcher->chordTimer->setInterval(3000); // 3.0 seconds hold duration |
There was a problem hiding this comment.
Hold is 3s (and logs "held for 3s"), but the README, "Exiting a Session" section, and exitChordTriggered doxygen all say 2s — users release at ~2s and it won't fire. Make it 2000 (or fix all the docs to 3s).
| void SessionRunner::onDeviceReconnected(const QString &stableId, int eventNumber, int instanceIndex) | ||
| { | ||
| if (!isRunning()) { | ||
| if (m_status != QStringLiteral("Starting session...") && m_status != QStringLiteral("Session running")) { |
There was a problem hiding this comment.
isRunning() was replaced with a guard against magic status-string literals. That couples reconnection to exact strings set elsewhere (a rename silently disables it) and flips semantics (reconnects whenever the string says running, even if all instances died). Prefer a semantic predicate.
- S1: validate username (s_validUsername) in writeTempUdevRule before it reaches a root-written udev rule (was only checked in other slots). - C2: replace hardcoded /home/deck/couchplay.log with QStandardPaths + gate behind COUCHPLAY_LOG (was always-on unbounded append). - C4: chord hold 3000ms -> 2000ms to match README (was 3s in code, 2s in docs). - C5: restore semantic isRunning() guard in onDeviceReconnected (status-string literals coupled to exact strings + flipped semantics). - C1: wire the nested kwin socket (--wayland-display) + export WAYLAND_DISPLAY so CouchPlay connects to kwin, not the parent gamescope socket. - C3: include couchplay-gamemode.sh in the beta/release tarballs so the GameMode desktop action resolves (install.sh already installs it). Note: full Game Mode behavior (nested kwin rendering) needs SteamOS validation.
Summary
This PR adds SteamOS Game Mode integration, migrates input device monitoring into the privileged helper daemon, adds Steam Controller exit chord detection via raw HID, and improves Bluetooth controller grouping. This adds full support for steamos game mode and allows couchplay to run while the steam client is also running in desktop mode. I plan to come through with some improvements / documentation on how to set this up to run it in game mode and big picture mode. I could have include all of those changes into this bransch / MR but this was already a monster so I wanted to send it over as it is since it works fully
Changes
refactor: Migrate input device watching to helper daemon
VirtualDeviceWatcherGUI-side manager entirely/dev/inputfor new devices, grants uinput write access, and monitors assigned nodes for the Start+Select exit chordSessionRunnerdelegates device watching to the helper via a newWatchDeviceD-Bus call; the helper emitsexitChordTriggered(username)back when the chord is heldfeat: Steam Controller exit chord via hidraw
/dev/hidrawNendpoint and parses raw HID reports (64-byte SC format and 16-byte puck format) to detect Start+Select independently of the evdev node, which Steam Input may be holding open exclusivelyfeat: SteamOS Game Mode support & robust controller isolation
scripts/couchplay-gamemode.sh— a Game Mode hook script that re-runs CouchPlay's device isolation setup on Steam'sgamemoderunentry/exit, ensuring udev seat stripping and uinput ACLs survive Steam's device resetsscripts/update-sysext.shto rebuild the SquashFS system extension image for immutable SteamOS deploymentswriteTempUdevRule,removeTempUdevRule,findHidrawPathForDeviceId) toCouchPlayHelper.desktopfile to invokecouchplay-gamemode.shwhen launched via Steamfix: Controller detection and grouping improvements
DeviceManagerfor controllers that expose multiple input nodes (e.g. motion + gamepad)test_devicemanagercovering the new grouping and BT detection pathsTesting
ctest --test-dir build)