Skip to content

Protect settings during device refresh#95

Open
IgorArkhipov wants to merge 3 commits into
ergohaven:mainfrom
IgorArkhipov:igor/settings-safe-device-refresh-fix
Open

Protect settings during device refresh#95
IgorArkhipov wants to merge 3 commits into
ergohaven:mainfrom
IgorArkhipov:igor/settings-safe-device-refresh-fix

Conversation

@IgorArkhipov

@IgorArkhipov IgorArkhipov commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

EN

Problem

Background device scans could clear or switch an active keyboard when HID enumeration briefly changed, even while firmware settings still had pending writes. A reconnect also returned module and list editors to their first row, making a refresh look like the user's selected controller or setting had disappeared. Connected users had no explicit refresh command to control when device enumeration was applied.

Fix

  • Run automatic device discovery only while disconnected, and discard a late automatic result if a keyboard connected while that scan was in flight.
  • Defer scan results and device switches while layer, macro, tap-dance, combo, tap-hold, or settings-import work is pending.
  • Add an explicit Refresh devices command to the connected-device menu in English and Russian; manual results may update the active device list when no write is pending.
  • Preserve the selected module group by stable kind/title and retain key-override and alternate-repeat rows across reconnects, clamped to the refreshed entry count.
  • Add focused tests for pending-write deferral, automatic/manual scan policy, and reconnect selection restoration.

Verification

  • cargo test: 196 passed.
  • Focused device refresh/reconnect tests: 6 passed.
  • cargo clippy --all-targets: passed with existing repository warnings.
  • python3 scripts/check_i18n.py: passed.
  • Scoped rustfmt --check and git diff --check: passed. Repository-wide cargo fmt --check remains blocked by pre-existing formatting differences in src/keycode.rs.
  • TARGET=aarch64-apple-darwin scripts/build_macos_app.sh: arm64 app, DMG, and ZIP built; architecture and code-signature validation passed.

RU

Проблема

Фоновое сканирование устройств могло сбросить или переключить активную клавиатуру при кратковременном изменении результатов HID-поиска, даже если запись настроек прошивки еще не завершилась. После переподключения редакторы модулей и списков также возвращались к первой строке, поэтому выбранный контроллер или параметр выглядел потерянным. Для подключенного устройства не было явной команды обновления, позволяющей пользователю выбрать момент применения результатов поиска.

Исправление

  • Автоматический поиск устройств выполняется только без подключенной клавиатуры; поздний результат автоматического поиска отбрасывается, если подключение завершилось во время сканирования.
  • Результаты поиска и переключение устройств откладываются, пока выполняется запись слоев, макросов, tap dance, combos, tap-hold или импорт настроек.
  • В меню подключенного устройства добавлена явная команда обновления списка на английском и русском языках; ручной результат применяется только при отсутствии незавершенной записи.
  • Выбранная группа модулей восстанавливается по стабильным типу и названию, а строки Key Overrides и Alt Repeat сохраняются с ограничением по новому числу записей.
  • Добавлены целевые тесты ожидания незавершенной записи, политики автоматического/ручного поиска и восстановления выбора после переподключения.

Проверка

  • cargo test: прошли 196 тестов.
  • Целевые тесты обновления устройств и переподключения: прошли 6 тестов.
  • cargo clippy --all-targets: прошел с существующими предупреждениями репозитория.
  • python3 scripts/check_i18n.py: прошел.
  • Scoped rustfmt --check и git diff --check: прошли. Общий cargo fmt --check по-прежнему блокируется существующими отличиями форматирования в src/keycode.rs.
  • TARGET=aarch64-apple-darwin scripts/build_macos_app.sh: собраны arm64-приложение, DMG и ZIP; архитектура и подпись прошли проверку.

@IgorArkhipov
IgorArkhipov force-pushed the igor/settings-safe-device-refresh-fix branch from 27e2dee to 02ff978 Compare July 17, 2026 21:04

@kissetfall kissetfall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for working on protecting pending settings during device refresh. The current implementation still has several lifecycle blockers that need to be resolved before merge:

  1. Automatic device polling stops whenever a layout or HID handle is present. As a result, a normal USB disconnect is no longer detected and Entropy can remain in a false “connected” state.
  2. Manual refresh does not recover the same device after unplug/replug: when the device key is unchanged, the stale HID handle is retained and reconnect is not started.
  3. The “Refresh data” flow can delete the device cache before device_refresh_blocked() rejects reconnect because of pending Tap Hold, Macro, or Combo state. Please run one shared preflight before any destructive cache action or reconnect transition.
  4. The empty manual-scan status uses the hardcoded English text No device detected; it needs to use the existing i18n path.

Please keep lightweight disconnect detection active while protecting pending writes, reopen the HID handle when the same device is replugged, make the cache/reconnect transition atomic, and add production-lifecycle regression coverage for unplug/replug of the same device and “Refresh data” with pending editor state. The current helper-only tests do not exercise these paths.

@IgorArkhipov
IgorArkhipov force-pushed the igor/settings-safe-device-refresh-fix branch from 02ff978 to ad11ada Compare July 20, 2026 00:46
@IgorArkhipov

Copy link
Copy Markdown
Contributor Author

Addressed:

  • Automatic USB polling now remains active for connected HID sessions, including macOS; scan enumeration remains serialized through the existing macOS HID operation lock.
  • Empty scan results clear stale connected state. Non-empty results received while pending editor/HID work is blocked are consumed and discarded, not deferred for later application.
  • Manual refresh of the same device now forces start_connect, and re-enumerated devices are matched by serial or, for empty-serial boards, unambiguous VID/PID + manufacturer/name identity.
  • device_refresh_blocked() is shared before cache deletion and all reconnect/switch paths, covering active HID work plus pending Tap Hold, Macro, Combo, and Key Override changes.
  • Empty scan status now uses Key::NoDevicesFound.

Added production lifecycle regressions for connected scan scheduling, unplug with pending Combo state, stale scan discard, same-device reconnect, and empty-serial re-enumeration.

@kissetfall kissetfall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed head ad11ada. Manual same-device reopen, unique re-enumeration matching, refresh-data preflight, and localization are improved. All four hosted builds are green and local tests pass 262/262. Two lifecycle blockers remain.

  1. An empty automatic scan is applied even when device_refresh_blocked is true. The new pending-Combo test demonstrates this path but only asserts that HID is cleared; clear_connected_keyboard_state also clears combo_dirty and the pending edit. A physical unplug or transient empty enumeration therefore loses the exact settings this PR is meant to protect. Route disconnect through the shared deferred-state handoff, then reconnect/reconcile without dropping Macro/Combo/Tap Dance/Tap Hold/picker work.

  2. On macOS this removes the hid_session_active guard and enumerates every second while HidDevice remains open. The repository's own hid.rs comment says concurrent enumeration while another thread holds an open device can crash on macOS 26. The mutex only serializes individual HID operations; it does not cover the lifetime of the open handle. Keep a macOS-safe lightweight liveness path or close/reopen through one owner instead of concurrent HidApi enumeration.

Also treat a scan worker/channel failure separately from a confirmed physical disconnect; it currently becomes an empty list and clears the live session.

@IgorArkhipov
IgorArkhipov force-pushed the igor/settings-safe-device-refresh-fix branch from ad11ada to be973c7 Compare July 20, 2026 15:56
@IgorArkhipov

Copy link
Copy Markdown
Contributor Author

Addressed:

  • Automatic scan results are now discarded whenever refresh is blocked, including empty results, so pending Combo/Macro/Tap Dance/Tap Hold and picker edits remain intact.
  • Scan worker errors and channel disconnects now preserve current device session instead of being treated as an empty physical-device scan.
  • macOS no longer enumerates while an HID handle is open. Connected-device liveness uses the existing handle, while manual scans are blocked until ownership is released.

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