From 116e3ac28eeaccc078e248e6de18f5f8caf054ae Mon Sep 17 00:00:00 2001 From: hughesyadaddy Date: Mon, 18 May 2026 10:22:40 -0400 Subject: [PATCH] fix(ui): persist and clamp device-reported DPI through _handleDpiRead When the engine read the device's current DPI on connect, the backend overwrote ``settings.dpi`` in-memory but never called ``save_config`` -- so a DPI change made on the mouse hardware (via the on-device cycle button, Logi Options+ briefly stealing the device, or another Mouser session) was visible to the running session but reverted on the next launch. The unsaved mutation also left ``engine.cfg`` and disk out of sync, so any later setter (``setDpi`` itself, ``cycleDpiPreset``) wrote the same stale value back. Three concrete changes: * Route the device-reported value through ``clamp_dpi`` against the resolved connected device so the floor/ceiling stay device-aware. The previous handler stored the raw HID value with no validation. * Persist via ``save_config`` and propagate the new config into ``engine.cfg`` so subsequent ``_resolved_connected_device()`` reads observe the same value. * Skip the write entirely when the device-reported value matches the current ``settings.dpi`` -- the handler used to emit ``settingsChanged`` on every poll, churning every QML binding that depends on the ``settings`` keypath. No HID round-trip is added: this handler reacts to a value the device already reports, so calling ``engine.set_dpi`` here would only echo it back. Tests - ``test_persists_new_device_dpi_to_disk`` -- new value flushed via ``save_config``. - ``test_clamps_overrange_dpi_to_device_max`` / ``test_clamps_underrange_dpi_to_device_min`` -- clamp covers both ends of the range. - ``test_no_change_skips_save`` -- redundant reads do not churn disk or signals. - ``test_syncs_engine_cached_config`` -- ``engine.cfg`` mirrors the persisted value. - ``test_emits_dpi_from_device_with_clamped_value`` -- the ``dpiFromDevice`` signal carries the clamped value, not the raw one. --- tests/test_backend.py | 75 +++++++++++++++++++++++++++++++++++++++++++ ui/backend.py | 27 ++++++++++++++-- 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/tests/test_backend.py b/tests/test_backend.py index 1310276..46a0dec 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -4,6 +4,7 @@ import sys import tempfile import unittest +import unittest.mock from types import SimpleNamespace from unittest.mock import patch @@ -1273,5 +1274,79 @@ def test_set_start_minimized_does_not_call_apply_login_startup(self): self.assertFalse(backend.startMinimized) +@unittest.skipIf(Backend is None, "PySide6 not installed in test environment") +class BackendHandleDpiReadTests(unittest.TestCase): + """Device-reported DPI must persist to ``config.json`` so a hardware + DPI change taken on the mouse survives the next Mouser restart, and + must clamp into the connected device's range to defend against + bogus reports.""" + + def setUp(self) -> None: + self._save_mock = unittest.mock.MagicMock() + self._patches = ( + patch("ui.backend.save_config", self._save_mock), + patch("ui.backend.supports_login_startup", return_value=False), + ) + for p in self._patches: + p.start() + self.addCleanup(self._stop_patches) + + def _stop_patches(self) -> None: + for p in self._patches: + p.stop() + + def _build(self, *, cfg=None, engine=None): + loaded = copy.deepcopy(cfg or DEFAULT_CONFIG) + with patch("ui.backend.load_config", return_value=loaded): + backend = Backend(engine=engine) + self._save_mock.reset_mock() + return backend + + def test_persists_new_device_dpi_to_disk(self): + backend = self._build() + backend._handleDpiRead(2400) + self.assertEqual(backend._cfg["settings"]["dpi"], 2400) + self._save_mock.assert_called_once_with(backend._cfg) + + def test_clamps_overrange_dpi_to_device_max(self): + device = SimpleNamespace(dpi_min=200, dpi_max=4000) + engine = _FakeEngine(device_connected=True, connected_device=device) + backend = self._build(engine=engine) + backend._handleDpiRead(99999) + self.assertEqual(backend._cfg["settings"]["dpi"], 4000) + self._save_mock.assert_called_once_with(backend._cfg) + + def test_clamps_underrange_dpi_to_device_min(self): + device = SimpleNamespace(dpi_min=400, dpi_max=8000) + engine = _FakeEngine(device_connected=True, connected_device=device) + backend = self._build(engine=engine) + backend._handleDpiRead(50) + self.assertEqual(backend._cfg["settings"]["dpi"], 400) + self._save_mock.assert_called_once_with(backend._cfg) + + def test_no_change_skips_save(self): + cfg = copy.deepcopy(DEFAULT_CONFIG) + cfg["settings"]["dpi"] = 1500 + backend = self._build(cfg=cfg) + backend._handleDpiRead(1500) + self._save_mock.assert_not_called() + + def test_syncs_engine_cached_config(self): + engine = _FakeEngine(device_connected=True) + backend = self._build(engine=engine) + backend._handleDpiRead(1800) + self.assertEqual(engine.cfg["settings"]["dpi"], 1800) + + def test_emits_dpi_from_device_with_clamped_value(self): + _ensure_qapp() + device = SimpleNamespace(dpi_min=200, dpi_max=4000) + engine = _FakeEngine(device_connected=True, connected_device=device) + backend = self._build(engine=engine) + seen = [] + backend.dpiFromDevice.connect(seen.append) + backend._handleDpiRead(9999) + QCoreApplication.processEvents() + self.assertEqual(seen, [4000]) + if __name__ == "__main__": unittest.main() diff --git a/ui/backend.py b/ui/backend.py index db48f74..d3d825d 100644 --- a/ui/backend.py +++ b/ui/backend.py @@ -1640,10 +1640,31 @@ def _handleProfileSwitch(self, profile_name): @Slot(int) def _handleDpiRead(self, dpi): - """Runs on Qt main thread.""" - self._cfg.setdefault("settings", {})["dpi"] = dpi + """Runs on Qt main thread. + + A device-reported DPI is authoritative for "what the hardware is + currently set to" -- the user expects Mouser to keep showing the + same value across restarts rather than reverting to a stale + preference whenever the engine reads the device. Clamp the + incoming value, persist it, and keep the engine's cached config + in sync so subsequent reads do not loop through a stale picture. + + Skip the engine push (``set_dpi``) here: this handler is reacting + to a value the device already reports, so echoing it back would + be a redundant HID round-trip. + """ + device = self._resolved_connected_device() + clamped = clamp_dpi(dpi, device) + settings = self._cfg.setdefault("settings", {}) + if settings.get("dpi") == clamped: + self.dpiFromDevice.emit(clamped) + return + settings["dpi"] = clamped + save_config(self._cfg) + if self._engine: + self._engine.cfg = self._cfg self.settingsChanged.emit() - self.dpiFromDevice.emit(dpi) + self.dpiFromDevice.emit(clamped) @Slot(bool) def _handleConnectionChange(self, connected):