Skip to content

fix: restart gamepad listener when its threads die#82

Open
fanxing11 wants to merge 1 commit into
ActivityWatch:masterfrom
fanxing11:fix/restart-gamepad-listener
Open

fix: restart gamepad listener when its threads die#82
fanxing11 wants to merge 1 commit into
ActivityWatch:masterfrom
fanxing11:fix/restart-gamepad-listener

Conversation

@fanxing11

Copy link
Copy Markdown

#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() 没找到设备时仍是 False
  • 起过线程然后全退出 → True
  • stop() 之后 → False(避免重启被用户主动停掉的 listener)
  • mouse/keyboard 都活着、只有 gamepad 死 → unix 现在会正确重建所有 listener

原有的 test_unix_reinitializes_dead_listenerstest_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 干净。

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-apps

greptile-apps Bot commented Jun 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a silent AFK mis-classification on gamepad-only gaming sessions by teaching _check_listeners() to detect and restart a dead GamepadListener. A _was_started boolean flag is introduced to distinguish "no gamepad ever attached" (no restart needed) from "gamepad threads died unexpectedly" (restart required), avoiding a restart-on-every-poll loop on gamepad-less systems.

  • GamepadListener gains needs_restart() — returns True only when start() previously spawned threads that have all since exited; stop() resets the flag to prevent re-triggering after intentional shutdown.
  • _check_listeners() in unix.py now includes gamepadListener.needs_restart() in its restart condition, with a separate log message for device-removal vs. X-server-restart scenarios.
  • Five new tests in test_listener_health.py cover the key state transitions; two existing mock-based tests are updated to pin needs_restart.return_value = False so MagicMock's default truthy behaviour doesn't interfere.

Confidence Score: 4/5

Safe to merge; the fix correctly handles the target scenario with no functional regressions introduced.

The _was_started flag approach is sound for the common single-gamepad case and is well-tested. Two narrow edge cases exist: the log message when gamepad and keyboard/mouse die at the same moment only surfaces the gamepad attribution, and a multi-gamepad setup that loses only one device will silently leave that device unmonitored until all threads die. Neither affects the primary bug being fixed.

The interaction between _stop_listeners() (which skips stop() when is_alive() is False) and needs_restart() in unix.py is the subtlest part of the change and worth a careful read.

Important Files Changed

Filename Overview
aw_watcher_afk/listeners.py Adds _was_started flag and needs_restart() to GamepadListener to distinguish "never ran" from "ran and died"; logic is correct for the single-gamepad case but needs_restart() stays False when a multi-gamepad system loses only one device.
aw_watcher_afk/unix.py Extends _check_listeners() to call gamepadListener.needs_restart() and trigger a full reinit when the gamepad dies; log message selection has an edge-case ambiguity when gamepad and keyboard/mouse die simultaneously.
tests/test_listener_health.py Adds 5 targeted tests for needs_restart() covering the pre-start, no-device, thread-died, post-stop, and gamepad-only-death scenarios; updates two existing mock-based tests to explicitly set needs_restart.return_value = False.

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]
Loading

Reviews (1): Last reviewed commit: "fix: restart gamepad listener when its t..." | Re-trigger Greptile

Comment thread aw_watcher_afk/unix.py
Comment on lines +56 to +63
if gamepad_died:
self.logger.warning(
"Gamepad listener died (device removed?), reinitializing..."
)
else:
self.logger.warning(
"Input listeners died (X server restart?), reinitializing..."
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Comment on lines 208 to +220
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()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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 TimeToBuildBob left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants