fix(ui): persist hardware-reported DPI so on-mouse changes survive restart#180
Open
hughesyadaddy wants to merge 1 commit into
Open
Conversation
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.
3507483 to
116e3ac
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the engine reads the device's current DPI on connect (or after a profile / connection event), the backend overwrites
settings.dpiin-memory but never callssave_config. A DPI change made on the mouse hardware (the on-device cycle button, Logi Options+ briefly stealing the device, or a parallel Mouser session) is visible to the running UI but reverts on the next launch.The unsaved mutation also leaves
engine.cfgand the on-disk config out of sync, so any later setter (setDpi,cycleDpiPreset,setDpiPreset) writes the same stale value back through.Fix
Route
_handleDpiReadthrough the same persistence path the user-driven setters use:clamp_dpiagainst the resolved connected device so the floor/ceiling stay device-aware (previously the raw HID value was stored without validation).save_configand propagate intoengine.cfgso subsequent_resolved_connected_device()reads observe the same value.settings.dpi-- the handler used to emitsettingsChangedon every poll, churning every QML binding that depends on thesettingskeypath.Crucially, no HID round-trip is added:
_handleDpiReadreacts to a value the device already reports, so echoing it back viaengine.set_dpiwould only generate a redundant HID++ call.Test plan
New
BackendHandleDpiReadTestsintests/test_backend.py(6 cases):test_persists_new_device_dpi_to_disk-- new value flushed viasave_configtest_clamps_overrange_dpi_to_device_max/test_clamps_underrange_dpi_to_device_min-- clamp covers both endstest_no_change_skips_save-- redundant reads do not churn disk or signalstest_syncs_engine_cached_config--engine.cfgmirrors the persisted valuetest_emits_dpi_from_device_with_clamped_value-- thedpiFromDevicesignal carries the clamped valuepytest tests/ -q-- 501 passed, 1 skipped, 170 subtests passedOut of scope
clamp_dpiitself against malformed config (e.g. stringified values from hand-edited JSON) -- separate fix.