fix: restart gamepad listener when its threads die#82
Conversation
The gamepad listener added in ActivityWatch#80 wasn't covered by the listener health check in unix.py — only the mouse and keyboard listeners were. If a gamepad disappears (e.g. unplugged, USB error) while mouse/keyboard stay alive, all gamepad reader threads exit and the watcher silently stops tracking gamepad input. A reconnected gamepad is never picked up either, so users get incorrectly marked AFK while playing games. Add a needs_restart() helper on GamepadListener that returns True only after start() actually spawned threads and they have all died. This avoids the obvious naive fix (just check is_alive()) turning into an infinite restart loop on systems without a gamepad (where is_alive() is False from the start because start() returned early). unix._check_listeners() now reinitializes all listeners when the gamepad has died too, with a distinct log message for easier debugging.
Greptile SummaryThis PR fixes a silent AFK mis-classification on gamepad-only gaming sessions by teaching
Confidence Score: 4/5Safe to merge; the fix correctly handles the target scenario with no functional regressions introduced. The The interaction between Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[seconds_since_last_input] --> B[_check_listeners]
B --> C{gamepadListener\n.needs_restart?}
C -- "_was_started AND\nnot is_alive()" --> D[gamepad_died = True]
C -- "not started OR\nstill alive" --> E[gamepad_died = False]
D --> F{mouse alive?\nkeyboard alive?\nor gamepad_died?}
E --> F
F -- any False / True --> G[_stop_listeners]
G --> H{gamepadListener\n.is_alive?}
H -- True --> I[gamepadListener.stop]
H -- False --> J[skip stop\nthreads already dead]
I --> K[_start_listeners\ncreate new instances]
J --> K
K --> L[reset last_activity]
F -- all OK --> M[check for events]
Reviews (1): Last reviewed commit: "fix: restart gamepad listener when its t..." | Re-trigger Greptile |
| if gamepad_died: | ||
| self.logger.warning( | ||
| "Gamepad listener died (device removed?), reinitializing..." | ||
| ) | ||
| else: | ||
| self.logger.warning( | ||
| "Input listeners died (X server restart?), reinitializing..." | ||
| ) |
There was a problem hiding this comment.
Log message swallows keyboard/mouse death when gamepad also dies
The current branching logs only the gamepad message when gamepad_died is True, even if mouseListener.is_alive() or keyboardListener.is_alive() are also False at the same time (e.g., the X server crashed right as the gamepad was unplugged). In that scenario the "X server restart?" attribution is silently dropped, making diagnosis harder. The two warnings aren't mutually exclusive — consider logging both conditions when both are true.
| def is_alive(self) -> bool: | ||
| return any(t.is_alive() for t in self._threads) | ||
|
|
||
| def needs_restart(self) -> bool: | ||
| """Return True if start() previously launched threads but they have all died. | ||
|
|
||
| Used to distinguish a system that simply has no gamepad attached (in | ||
| which case start() returned early and no restart is needed) from one | ||
| where all reader threads have exited unexpectedly (e.g. device removed | ||
| or USB error). In the latter case the caller should reinitialize the | ||
| listener so a reconnected gamepad starts being tracked again. | ||
| """ | ||
| return self._was_started and not self.is_alive() |
There was a problem hiding this comment.
Multi-gamepad partial disconnection is not detected
is_alive() uses any(t.is_alive() for t in self._threads), so when a system has two gamepads and only one is unplugged, one thread dies but is_alive() stays True. needs_restart() therefore returns False, and the disconnected gamepad is never re-discovered on reconnect. The watcher silently tracks one fewer device until the remaining gamepad is also unplugged. This is a narrow edge case but worth noting, especially since the PR description specifically calls out multi-device USB scenarios.
TimeToBuildBob
left a comment
There was a problem hiding this comment.
CI green across all platforms. The needs_restart() gate is exactly right — it prevents the common false-negative case (systems without evdev or no connected gamepads) while still catching the real failure: threads that were started but have since died.
The five new tests in test_listener_health.py cover the tricky edge cases (never-started, started-but-no-devices, running-threads-died, stopped-explicitly, only-gamepad-dead). Code is clean and well-commented.
This addresses the regression where gamepad disconnection during a gaming session permanently breaks AFK detection until the watcher is restarted. Merging this is a quality improvement over the current state.
看 #80 加进来的 gamepad listener 时,发现
unix.py的_check_listeners只盯着 mouse / keyboard,gamepad 根本没人管。如果手柄运行中被拔了(或者 USB 抽风),所有 reader 线程都会退出,但 watcher 不会重启它,再插回去也不会被发现。结果就是:玩游戏的时候只有手柄输入,被一路标成 AFK。
直接的修法
if not gamepad.is_alive()不行 —— 没装 evdev 或者没手柄的机器上start()直接 return,is_alive()一开始就是 False,会变成每个 poll 都 restart 一遍。所以加了个
needs_restart():只在「真的起过线程、现在全死了」时才返回 True。靠一个_was_started标志区分两种情况。测试在
tests/test_listener_health.py里加了 5 个:needs_restart()起手就是 False(gamepad-less 系统不会乱重启)start()没找到设备时仍是 Falsestop()之后 → False(避免重启被用户主动停掉的 listener)原有的
test_unix_reinitializes_dead_listeners和test_unix_gamepad_event_resets_afk_timer同步更新(把 mock 的needs_restart.return_value显式设成 False,不然 MagicMock 默认 truthy 会触发新逻辑)。本地跑
pytest tests/test_listener_health.py全过(18/18),mypy aw_watcher_afk --ignore-missing-imports干净。