[Refactor / bugfix] Simplify HardwareButtons

[kdmukai]

Description

Two main issues are addressed:

• Eliminates lots of unnecessary complexity in HardwareButtons.
• Fixes Press OK one more time... after verifying scanned address #654 which seems to be some kind of race condition that is buried inside some of the above (now removed) complexity.

---

Death to release_lock

The main change is around eliminating the release_lock and related concepts in HardwareButtons. It's extremely difficult to reason through in its current (dev, i.e. before this PR) state. From what I can discern, it's meant to track the status of a button (or joystick direction) being physically released (press a button down, then release it to let it return to its normal resting position).

So think of it like javascript's onMouseUp event.

However, if you look at how our code reacts to user input, we only react on the PRESS, not on the release.

We only need HardwareButtons to react to individual inputs (joystick movement, button press) and be savvy about short-press vs long-press/sustained hold (e.g. holding the joystick to continuously scan across a keyboard). It doesn't care at all if we clicked some super-important OKAY button that we categorized as a release_key.

So if we start to unwind the release_lock concept, a number of methods can be totally deleted:

• The check_release and release_keys input params on wait_for()
• force_release()
• The rising_callback() handler
• The add_events() initializer to set that handler
• trigger_override() now doesn't need any reference to release_lock

---

Issue #654: In my testing, this was caused by this line:

seedsigner/src/seedsigner/hardware/buttons.py

Line 106 in 9bfb7b2

HardwareButtonsConstants.release_lock = False

If I moved that to after the override is handled, the issue was resolved (sort of -- I had a lot of changes in place by then, but moving that line was the big breakthrough as far as I could tell).

Completely eliminating the release_lock (thankfully) also fixed the bug.

---

Simplify trigger_override

The current (dev) version of wait_for() has an absolutely nightmarish if statement AND an incomprehensible follow up comment (that I think I wrote, unfortunately):

if not check_release or ((check_release and key in release_keys and HardwareButtonsConstants.release_lock) or check_release and key not in release_keys):
    # when check release is False or the release lock is released (True)

The above release_lock changes help us out a ton here. We can eliminate all of the if statement mess. The last complication is in how we process the trigger_override(). This now becomes simple: We triggered the override to break us out of the wait_for() loop. So move that check to the beginning of the loop and keep its logic completely separate.

---

Remaining changes

• Matching changes elsewhere to the wait_for() input params.
• Bit of thread cleanup assurances in BaseScreen.display().
• seed_views.py: the diff looks messy, but it's really just bumping everything in to wrap it in a try/finally to add guarantees about the BruteForceAddressVerificationThread being shut down.

---

Tips for testing

Any screen that involves user input, especially if there are other threads or atypical input handling:

• Issue Press OK one more time... after verifying scanned address #654 Address Verification
• The Address Verification brute force thread (Skip 10, Cancel).
• I/O Test
• Scan (from MainMenu) w/LEFT to cancel.
• Image entropy live preview (all navigation options on: live preview, review final pic)
• Seed word entry keyboard screen (joystick movement, joystick HOLD to scan across or up/down, KEY1/KEY3 press/hold to scroll candidate matches.
• BIP-39 passphrase entry, KEY1/KEY2 keyboard swaps
• Animated QR brightness up/down, exit

---

This pull request is categorized as a:

• Bug fix
• Code refactor

Checklist

• I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

• N/A (currently no way for the test suite to test real-time hardware input interactions)

I have tested this PR on the following platforms/os:

• Raspberry Pi OS Manual Build
• SeedSigner OS on a Pi0/Pi0W board

[jdlcdl]

Tested ACK (I think, at least it all seems to be working) as of 9efb700,
During code review, noting that thread cleanup that used to be handled during call-back of a seed screen is happening during a finally block of the base screen... it seems like a more sure way to clean-up so I like it, but I don't know where to expect other problems and would like to get to using this as my default "seedsigner" image asap, long before release.

Currently, this is:

• Passing all automated tests on my Ubuntu Desktop.
• Behaving well on RaspiOS RPi02w dev-signer (including Keith's detailed "check these too" list above).
• Behaving well on seedsigner-os RPi0 built like --pi0 --app-repo=https://github.com/kdmukai/seedsigner.git --app-commit-id=9efb700 --no-clean
  bded999c73e9ab1822a9dacedb4b0f024040dd9cc4948a9c402fa93519ca8c6f seedsigner_os.9efb700.pi0.img

I will be using this new pi0 image as my default (as I have done with 0.8.5-rc1 for the past weeks) until further prs (like pr_651 which was only awaiting this fix) have been merged for a new internal rc2.

[jdlcdl]

no longer a TODO?

[kdmukai]

Good eyes. Removed!