-
Notifications
You must be signed in to change notification settings - Fork 2
Fix multi-monitor detection: periodic monitor list refresh and monito… #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,6 +33,8 @@ def __init__( | |||||||||||||
| """ | ||||||||||||||
| self.event_queue = event_queue | ||||||||||||||
| self._monitors = list(get_monitors()) | ||||||||||||||
| self._monitors_last_refresh = time.time() | ||||||||||||||
| self._monitors_refresh_interval = 5.0 | ||||||||||||||
| self.accessibility_enabled = accessibility | ||||||||||||||
| self.accessibility_handler = None | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -68,6 +70,14 @@ def _get_monitor(self, x: int, y: int) -> int: | |||||||||||||
| Returns: | ||||||||||||||
| Monitor index (0-based) | ||||||||||||||
| """ | ||||||||||||||
| now = time.time() | ||||||||||||||
| if now - self._monitors_last_refresh > self._monitors_refresh_interval: | ||||||||||||||
|
Comment on lines
70
to
+74
|
||||||||||||||
| try: | ||||||||||||||
| self._monitors = list(get_monitors()) | ||||||||||||||
| except Exception: | ||||||||||||||
| pass | ||||||||||||||
|
Comment on lines
+77
to
+78
|
||||||||||||||
| except Exception: | |
| pass | |
| except Exception as exc: | |
| # Keep using previously cached monitor information if refresh fails. | |
| print(f"Warning: Failed to refresh monitor information: {exc}") |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The monitor refresh can leave self._monitors empty (e.g., get_monitors() returns []). The rest of _get_monitor assumes at least one monitor exists (uses self._monitors[0] later), which can raise IndexError. Add a guard after refresh (and before the loop) to handle the empty-list case safely.
| # If no monitors are available (e.g., get_monitors() returned an empty list), | |
| # return a safe default monitor description to avoid IndexError. | |
| if not self._monitors: | |
| return 0, {"left": 0, "top": 0, "width": 0, "height": 0} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,7 +96,7 @@ def enqueue(self, event: InputEvent) -> None: | |
|
|
||
| queue = self.aggregations[agg_type] | ||
| config = self.configs[agg_type] | ||
| screenshots = self._collect_screenshots(event.timestamp) | ||
| screenshots = self._collect_screenshots(event.timestamp, event.monitor_index) | ||
|
|
||
| last_event, last_screenshots = queue[-1] if queue else (None, None) | ||
| first_event, first_screenshots = queue[0] if queue else (None, None) | ||
|
|
@@ -171,7 +171,7 @@ def _end_burst(self, agg_type: str, event: InputEvent, screenshot: Any) -> None: | |
| current_burst_id = self._get_burst_id_for_type(agg_type) | ||
|
|
||
| # Get screenshot with padding after | ||
| end_screenshot = self._collect_end_screenshot(event.timestamp) | ||
| end_screenshot = self._collect_end_screenshot(event.timestamp, event.monitor_index) | ||
|
|
||
| request = self._create_request( | ||
| event=event, | ||
|
|
@@ -255,21 +255,33 @@ def _get_burst_id_for_type(self, agg_type: str) -> int: | |
| self.next_burst_id | ||
| ) | ||
|
|
||
| def _collect_screenshots(self, timestamp: float) -> Any: | ||
| """Get screenshot before timestamp.""" | ||
| def _collect_screenshots(self, timestamp: float, monitor_index: int = None) -> Any: | ||
| """Get screenshot before timestamp, preferring the given monitor.""" | ||
| constants = constants_manager.get() | ||
|
Comment on lines
+258
to
260
|
||
| start_candidates = self.image_queue.get_entries_before( | ||
| timestamp, milliseconds=constants.PADDING_BEFORE | ||
| ) | ||
| return start_candidates[-1] if start_candidates else None | ||
|
|
||
| def _collect_end_screenshot(self, timestamp: float) -> Any: | ||
| """Get screenshot after timestamp with padding.""" | ||
| if not start_candidates: | ||
| return None | ||
| if monitor_index is not None: | ||
| matching = [s for s in start_candidates if s.monitor_index == monitor_index] | ||
| if matching: | ||
| return matching[-1] | ||
| return start_candidates[-1] | ||
|
Comment on lines
+266
to
+270
|
||
|
|
||
| def _collect_end_screenshot(self, timestamp: float, monitor_index: int = None) -> Any: | ||
| """Get screenshot after timestamp with padding, preferring the given monitor.""" | ||
| constants = constants_manager.get() | ||
| exact_candidates = self.image_queue.get_entries_after( | ||
| timestamp, milliseconds=constants.PADDING_AFTER | ||
| ) | ||
| return exact_candidates[-1] if exact_candidates else None | ||
| if not exact_candidates: | ||
| return None | ||
| if monitor_index is not None: | ||
| matching = [s for s in exact_candidates if s.monitor_index == monitor_index] | ||
| if matching: | ||
| return matching[-1] | ||
| return exact_candidates[-1] | ||
|
Comment on lines
+280
to
+284
|
||
|
|
||
| def _save_event_to_jsonl(self, event: InputEvent) -> None: | ||
| if self.session_dir: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_monitors()is called unguarded during initialization, but refreshes are wrapped in a try/except. Ifscreeninfofails or returns an empty list at startup, the handler can crash before it ever reaches the safer refresh logic. Consider wrapping the initialget_monitors()call similarly and ensuring_monitorsis non-empty (fallback to a single synthetic monitor or a safe default).