trezor-firmware icon indicating copy to clipboard operation
trezor-firmware copied to clipboard

RustLayout rework, Global Layout object

Open matejcik opened this issue 1 year ago • 6 comments

Ladies and gentlemen, this is what you've been waiting for

fixes #2299 fixes #3280

Main features:

  1. unified ui.Layout and ui.ProgressLayout classes
    • instead of per-model RustLayout that is kind-of-but-not-really a copypaste job
  2. Global ui.CURRENT_LAYOUT that always unambiguously refers to the layout that is on the screen, or None if the screen is dark
  3. Total overhaul of debuglink internals, leading, hopefully, to greater test stability and less races
    • client.debug.wait_layout() is now almost never needed, except in very special cases. read_layout() Does The Right Thing.
    • deadlock detection when you miss a ButtonAck
    • it "worked on my PC" but i'm sure you guys will figure out a way to break it anyway :woman_shrugging:

Minor niceties

  • timeout for loop.wait() so you usually don't need to use loop.race(xyz, loop.wait(...)) e8990ce5db4a138df0670e8fe79aec73b164f4bb
    • paving way to fixing #3117
  • loop.chan is unused now. I left it in just in case but we can probably remove it 9f01c9f896e9268142e63a134642432579890a77
  • nice simplification of handle_session now that it does not need to support debuglink 06cad6fa6635593bb4503dbda2d520bd71ecf523
  • bug with make test_emu_ui in zsh not being random is fixed eb65cf669234f3ed1110a865d831dd08cd8d9b99
  • WipeDevice will show a progress screen for the 2 seconds it takes to wipe storage. c98e54ba5ea1731e27dd56cba04e6ee4a03530f4
    • it's "stuck" because we can't animate, but at least the user is not staring at a black screen
  • if a timeout kills your test, you'll see where in the input flow it froze 1a9d022da11f46953164faf789b5fad52e0d037e
  • detection of invalid input flows that don't run all the way through fd914bd18f9cf3819b46229886780fd849d48f9d
  • reduces chattiness of error output when your UI tests are failing (e15c4e84f0a250265440940f5a230a0e98d87304, same as #3665)

Supersedes

#2335 #3083 #3665

matejcik avatar Apr 07 '24 12:04 matejcik

legacy UI changes device test(screens) main(screens)

github-actions[bot] avatar Apr 07 '24 12:04 github-actions[bot]

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

github-actions[bot] avatar Apr 07 '24 12:04 github-actions[bot]

UI changes summary:

  • all progress steps of all loaders are removed, we only screenshot the first time a loader is shown
    • except for "hold to lock", because the loader starts in progress already
  • some "loading transaction" screens are gone
    • previously, a "loading transaction" screen would blink inbetween every Bitcoin confirmation dialog
    • now it only shows up if there is in fact a loading step (so e.g., between the last output and the total confirmation screen there is nothing happening to warrant a loader)
  • some screens are now recorded that are immediately canceled
    • in test_cancel, in cancellation versions of various reset/backup/recovery flows
  • WipeDevice now starts progress earlier

Learn more

https://github.com/trezor/trezor-firmware/blob/matejcik/global-layout-only3/docs/core/misc/layout-lifecycle.md

Reviewer instructions

I strongly recommend going by commits. There's a big one that does most of the things (b75dbe18e4c04a7c73f4c7ae9c5160d3cd17f4b8), the other ones are more reasonable.

Please also point out if you feel that something is not documented that should be.

matejcik avatar Apr 07 '24 12:04 matejcik

TODO: this definitely wants a changelog entry

matejcik avatar Apr 07 '24 13:04 matejcik

seems that there's a progress related piece of code that I didn't manage to commit. stay tuned for fixes.

matejcik avatar Apr 07 '24 13:04 matejcik

also this does not feel like I added 11k of code, not sure what is going on...

matejcik avatar Apr 07 '24 13:04 matejcik