Skip to content

feat(mockui): shared on-screen keyboard via KeyboardManager#21

Merged
maggo83 merged 7 commits intok9ert:mainfrom
maggo83:feat/mockui-shared-keyboard
Mar 10, 2026
Merged

feat(mockui): shared on-screen keyboard via KeyboardManager#21
maggo83 merged 7 commits intok9ert:mainfrom
maggo83:feat/mockui-shared-keyboard

Conversation

@maggo83
Copy link
Copy Markdown
Collaborator

@maggo83 maggo83 commented Mar 10, 2026

Summary

Replaces three independent ad-hoc keyboard implementations in wallet_menu,
passphrase_menu, and generate_seedphrase_menu with a single shared
KeyboardManager owned by NavigationController.

Architecture

  • KeyboardManager holds one reusable lv.keyboard instance (lazy-created).
  • Menus activate it by calling bind() from a CLICKED event lambda on their
    textarea — no permanent owner reference is held.
  • Exactly one binding is active at a time. A new bind() silently replaces the
    previous one (restoring the old textarea text, suppressing on_cancel).
  • Lifecycle cleanup is driven by lv.EVENT.DELETE on the textarea, so menus
    never need to manually unbind on navigation.
  • Layout constants: Layout.ALNUM / Layout.FULL.
  • Menus own their own sanitizer callbacks (passphrase menu trims
    leading/trailing spaces via sanitize=str.strip).

Tests

  • 19 new unit tests in test_keyboard_manager.py covering: bind/unbind
    lifecycle, commit, cancel, sanitizer, DELETE path, rebind replacement,
    rebind on_cancel suppression, and no-crash guards.
  • New device test test_passphrase_keyboard_repeated_commits_no_reset:
    regression for crash observed after multiple passphrase edit cycles on device.
  • Added test_keyboard_device.py with open/commit/cancel flow coverage.
  • Keyboard helper utilities extracted to tests_device/conftest.py.

All tests pass: 119 unit tests, 9 device tests.

maggo83 added 5 commits March 9, 2026 15:42
…nt framework

- Remove generic .bmad/bmm, .bmad/bmb, .bmad/core upstream scaffolding
- Add custom agent team: orchestrator, pm, architect, ux-designer, developer,
  tester, i18n-specialist, micropython-specialist, lvgl-mockui-specialist,
  hw-bootloader-specialist, security, scrum-master, doc-writer
- Add workflows: feature-development (9 stages incl. device validation),
  bug-fix, refactor, release
- Add adapters: copilot-instructions, cursor-rules, continue-dev
- Add team-config.md and BMAD.md with full project context
- Propagate nix-develop prefix to all make targets across all agent files
- Document correct manifest system (freeze tree, no per-file listing)
- Document i18n pipeline direction: specter_ui_en.json → sync-i18n →
  translation_keys.py (auto-generated); keys are UPPERCASE_UNDERSCORES
- Document boot chain: main.py shared, boot.py hardware-only, platform
  detection via sys.platform, /flash mount differences
- Add auto-generated files rule: path-independent .gitignore patterns
- Add device validation stage to feature-development workflow with
  test-first triage loop (fix test before blaming implementation)
- Update .gitignore: lang_*.bin, language_config.json, translation_keys.py
- Update CLAUDE.md with corrected pipeline and key locations
Replaces three independent ad-hoc keyboard implementations
(wallet_menu, passphrase_menu, generate_seedphrase_menu) with a single
shared KeyboardManager owned by NavigationController.

Architecture
- KeyboardManager holds one reusable lv.keyboard instance (lazy-created).
- Menus activate it by calling bind() from a CLICKED event lambda on their
  textarea; no permanent owner reference needed.
- Exactly one binding is active at a time. A new bind() silently replaces the
  previous one (restoring the old textarea text, suppressing on_cancel).
- Lifecycle cleanup is driven by lv.EVENT.DELETE on the textarea, so menus
  never need to manually unbind on navigation.
- Layout constants live in Layout.ALNUM / Layout.FULL (enum-style class).
- Menus own their own accepted-char restrictions and sanitizer callbacks.
  The passphrase menu trims leading/trailing spaces via sanitize=str.strip.

Bug fixes
- Fix use-after-free crash in _commit(): textarea reference captured before
  _unbind() clears self.textarea.
- Fix set_textarea() never being called (keyboard was shown but not linked).
- Fix remove_event_cb() being called with extra args (MicroPython ABI).
- Fix lv.EVENT.DELETE handler calling set_text() on a dying object.
- Fix re-entrancy when replacing a binding: on_cancel suppressed via
  call_cb=False to prevent navigation callbacks mid-bind().
- Remove stray 'from portpicker import bind' that caused ImportError on boot.

Removed
- keyboard_text_rules.py and its unit test (rules logic folded into menus).

Tests
- 19 new unit tests in test_keyboard_manager.py covering bind/unbind
  lifecycle, commit, cancel, sanitizer, DELETE path, rebind replacement,
  rebind on_cancel suppression, and no-crash guards.
- New device test test_passphrase_keyboard_repeated_commits_no_reset:
  regression for the crash observed after multiple passphrase edit cycles.
- Extended test_keyboard_device.py with open/commit/cancel flow coverage.
- Keyboard helper utilities extracted to tests_device/conftest.py.

All tests pass: 119 unit tests, 9 device tests (build+flash default).
Replaces three independent ad-hoc keyboard implementations
(wallet_menu, passphrase_menu, generate_seedphrase_menu) with a single
shared KeyboardManager owned by NavigationController.

Architecture
- KeyboardManager holds one reusable lv.keyboard instance (lazy-created).
- Menus activate it by calling bind() from a CLICKED event lambda on their
  textarea; no permanent owner reference needed.
- Exactly one binding is active at a time. A new bind() silently replaces the
  previous one (restoring the old textarea text, suppressing on_cancel).
- Lifecycle cleanup is driven by lv.EVENT.DELETE on the textarea, so menus
  never need to manually unbind on navigation.
- Layout constants live in Layout.ALNUM / Layout.FULL (enum-style class).
- Menus own their own accepted-char restrictions and sanitizer callbacks.
  The passphrase menu trims leading/trailing spaces via sanitize=str.strip.

Bug fixes
- Fix use-after-free crash in _commit(): textarea reference captured before
  _unbind() clears self.textarea.
- Fix set_textarea() never being called (keyboard was shown but not linked).
- Fix remove_event_cb() being called with extra args (MicroPython ABI).
- Fix lv.EVENT.DELETE handler calling set_text() on a dying object.
- Fix re-entrancy when replacing a binding: on_cancel suppressed via
  call_cb=False to prevent navigation callbacks mid-bind().
- Remove stray 'from portpicker import bind' that caused ImportError on boot.

Removed
- keyboard_text_rules.py and its unit test (rules logic folded into menus).

Tests
- 19 new unit tests in test_keyboard_manager.py covering bind/unbind
  lifecycle, commit, cancel, sanitizer, DELETE path, rebind replacement,
  rebind on_cancel suppression, and no-crash guards.
- New device test test_passphrase_keyboard_repeated_commits_no_reset:
  regression for the crash observed after multiple passphrase edit cycles.
- Extended test_keyboard_device.py with open/commit/cancel flow coverage.
- Keyboard helper utilities extracted to tests_device/conftest.py.

All tests pass: 119 unit tests, 9 device tests (build+flash default).
Copy link
Copy Markdown
Contributor

@al-munazzim al-munazzim left a comment

Choose a reason for hiding this comment

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

Review: Clean, well-structured refactor 👍

+725 / -219 across 8 files, but net complexity drops significantly — three independent keyboard implementations replaced by one shared KeyboardManager. Test coverage is solid (19 unit + 3 device tests).

Strengths:

  • Single ownership model with bind()/_unbind() — one keyboard instance, one active binding, automatic cleanup via lv.EVENT.DELETE
  • Silent rebind with call_cb=False avoids spurious cancel callbacks during screen transitions
  • Sanitizer as a callback (sanitize=str.strip) keeps the manager generic
  • Commit/cancel ordering (unbind first, then callbacks) prevents reentrancy bugs
  • Device regression test (repeated_commits_no_reset) catches real crash scenarios

Nits / questions:

  1. _unbind double remove_event_cb (lines 80-81) — called twice. Intentional for DEFOCUSED + DELETE? Deserves a comment, looks like copy-paste at first glance.

  2. _original_name in generate_seedphrase_menu (line 37) — set but never read. Leftover?

  3. set_accepted_chars removed from wallet_menu — the old code restricted wallet name characters, now deleted. Passphrase menu still has it. Intentional? Without it, wallet names accept anything including newlines.

  4. set_wallet_bar_visible removed — old wallet_menu hid the wallet bar while keyboard was open. New KeyboardManager doesn't. Does the keyboard overlap the wallet bar now, or does LVGL layering handle it?

  5. Layout map duplication_build_full_layout and _build_alnum_layout share identical map_lower/map_upper rows; only map_special differs. Minor, but could share the text rows.

  6. on_cancel called on DELETE — if the textarea is being destroyed during navigation, calling on_cancel could trigger logic on a dying screen. Safe today (passphrase cancel is unset), but worth documenting the contract.

Verdict: Merge-ready after addressing the _original_name leftover and set_accepted_chars question. Double remove_event_cb deserves a comment at minimum.

maggo83 added 2 commits March 10, 2026 18:54
- _unbind: switch from remove_event_cb() to remove_event_dsc() using
  stored descriptors (defocus_cb / delete_cb); verified on device via
  REPL — add_event_cb returns lv_event_dsc_t, remove_event_dsc returns True

- wallet_menu: restore set_accepted_chars() to prevent newlines and other
  control characters in wallet names (passphrase menu already had it)

- keyboard_manager docstring: clarify on_cancel / text-restore contract —
  neither is triggered during lv.EVENT.DELETE because the textarea object
  is being destroyed by LVGL at that point (calling set_text or navigation
  callbacks on a dying object would be unsafe)

- test_keyboard_manager: fix MockTextarea to use descriptor-based
  remove_event_dsc() matching the real LVGL API; update
  test_cancel_on_delete_still_calls_on_cancel to
  test_cancel_on_delete_does_not_call_on_cancel to match the
  now-documented contract

All tests pass: 119 unit tests, 9 device tests.
@maggo83
Copy link
Copy Markdown
Collaborator Author

maggo83 commented Mar 10, 2026

Addressed below:

  1. _unbind double remove_event_cb — Fixed. Switched to remove_event_dsc() using the descriptor handles returned by add_event_cb (stored as self.defocus_cb / self.delete_cb). Verified on device via REPL: add_event_cb returns lv_event_dsc_t, remove_event_dsc(dsc) returns True. Not a copy-paste dup any more — each descriptor is distinct.

  2. _original_name in generate_seedphrase_menu — Fixed, removed the unused assignment.

  3. set_accepted_chars removed from wallet_menu — Deliberate omission was a mistake, thanks for catching it. Restored set_accepted_chars() to wallet_menu matching the same set used in passphrase_menu (no newlines, no control characters).

  4. set_wallet_bar_visible removed — Intentional. Tested with the overlay keyboard visible and the wallet bar is rendered underneath the keyboard without overlap (LVGL z-order handles it). Removing the hide/show calls simplifies the lifecycle without any visual regression.

  5. Layout map duplication — Accepted for now as a known nit. Can be cleaned up in a follow-up.

  6. on_cancel called on DELETE — Updated the on_cancel docstring to explicitly state the contract: neither text restore nor on_cancel are triggered during lv.EVENT.DELETE, because the textarea object is being destroyed by LVGL at that point — calling set_text() or navigation callbacks on a dying object would be unsafe. Test updated to assert this contract (test_cancel_on_delete_does_not_call_on_cancel).

@maggo83 maggo83 merged commit ddcdea6 into k9ert:main Mar 10, 2026
1 check passed
@maggo83 maggo83 deleted the feat/mockui-shared-keyboard branch March 10, 2026 19:13
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.

3 participants