Skip to content

Commit 825a25a

Browse files
authored
Merge pull request #655 from kdmukai/hardwarebutton_refactor
[Refactor / bugfix] Simplify `HardwareButtons`
2 parents 24d688a + e972fc8 commit 825a25a

File tree

5 files changed

+136
-158
lines changed

5 files changed

+136
-158
lines changed

src/seedsigner/gui/screens/screen.py

+16-17
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
import time
23

34
from dataclasses import dataclass, field
@@ -15,6 +16,8 @@
1516
from seedsigner.models.settings import SettingsConstants
1617
from seedsigner.models.threads import BaseThread, ThreadsafeCounter
1718

19+
logger = logging.getLogger(__name__)
20+
1821

1922
# Must be huge numbers to avoid conflicting with the selected_button returned by the
2023
# screens with buttons.
@@ -71,6 +74,13 @@ def display(self) -> Any:
7174
for t in self.get_threads():
7275
t.stop()
7376

77+
for t in self.get_threads():
78+
# Wait for each thread to stop; equivalent to `join()` but gracefully
79+
# handles threads that were never run (necessary for screenshot generator
80+
# compatibility, perhaps other edge cases).
81+
while t.is_alive():
82+
time.sleep(0.01)
83+
7484

7585
def clear_screen(self):
7686
# Clear the whole canvas
@@ -226,11 +236,7 @@ def _run(self):
226236
time.sleep(0.1)
227237
continue
228238

229-
user_input = self.hw_inputs.wait_for(
230-
HardwareButtonsConstants.ALL_KEYS,
231-
check_release=True,
232-
release_keys=HardwareButtonsConstants.KEYS__ANYCLICK
233-
)
239+
user_input = self.hw_inputs.wait_for(HardwareButtonsConstants.ALL_KEYS)
234240

235241
with self.renderer.lock:
236242
if not self.top_nav.is_selected and user_input in [
@@ -447,6 +453,7 @@ def _run(self):
447453
while True:
448454
ret = self._run_callback()
449455
if ret is not None:
456+
logging.info("Exiting ButtonListScreen due to _run_callback")
450457
return ret
451458

452459
user_input = self.hw_inputs.wait_for(
@@ -455,9 +462,7 @@ def _run(self):
455462
HardwareButtonsConstants.KEY_DOWN,
456463
HardwareButtonsConstants.KEY_LEFT,
457464
HardwareButtonsConstants.KEY_RIGHT,
458-
] + HardwareButtonsConstants.KEYS__ANYCLICK,
459-
check_release=True,
460-
release_keys=HardwareButtonsConstants.KEYS__ANYCLICK
465+
] + HardwareButtonsConstants.KEYS__ANYCLICK
461466
)
462467

463468
with self.renderer.lock:
@@ -641,9 +646,7 @@ def swap_selected_button(new_selected_button: int):
641646
HardwareButtonsConstants.KEY_DOWN,
642647
HardwareButtonsConstants.KEY_LEFT,
643648
HardwareButtonsConstants.KEY_RIGHT
644-
] + HardwareButtonsConstants.KEYS__ANYCLICK,
645-
check_release=True,
646-
release_keys=HardwareButtonsConstants.KEYS__ANYCLICK
649+
] + HardwareButtonsConstants.KEYS__ANYCLICK
647650
)
648651

649652
with self.renderer.lock:
@@ -858,9 +861,7 @@ def _run(self):
858861
HardwareButtonsConstants.KEY_DOWN,
859862
HardwareButtonsConstants.KEY_LEFT,
860863
HardwareButtonsConstants.KEY_RIGHT,
861-
] + HardwareButtonsConstants.KEYS__ANYCLICK,
862-
check_release=True,
863-
release_keys=HardwareButtonsConstants.KEYS__ANYCLICK
864+
] + HardwareButtonsConstants.KEYS__ANYCLICK
864865
)
865866
if user_input == HardwareButtonsConstants.KEY_DOWN:
866867
# Reduce QR code background brightness
@@ -1192,9 +1193,7 @@ def _run(self):
11921193
# Start the interactive update loop
11931194
while True:
11941195
input = self.hw_inputs.wait_for(
1195-
HardwareButtonsConstants.KEYS__LEFT_RIGHT_UP_DOWN + [HardwareButtonsConstants.KEY_PRESS, HardwareButtonsConstants.KEY3],
1196-
check_release=True,
1197-
release_keys=[HardwareButtonsConstants.KEY_PRESS, HardwareButtonsConstants.KEY3]
1196+
HardwareButtonsConstants.KEYS__LEFT_RIGHT_UP_DOWN + [HardwareButtonsConstants.KEY_PRESS, HardwareButtonsConstants.KEY3]
11981197
)
11991198

12001199
with self.renderer.lock:

src/seedsigner/gui/screens/seed_screens.py

+15-20
Original file line numberDiff line numberDiff line change
@@ -243,11 +243,7 @@ def _render(self):
243243

244244
def _run(self):
245245
while True:
246-
input = self.hw_inputs.wait_for(
247-
HardwareButtonsConstants.ALL_KEYS,
248-
check_release=True,
249-
release_keys=[HardwareButtonsConstants.KEY_PRESS, HardwareButtonsConstants.KEY2]
250-
)
246+
input = self.hw_inputs.wait_for(HardwareButtonsConstants.ALL_KEYS)
251247

252248
with self.renderer.lock:
253249
if self.is_input_in_top_nav:
@@ -885,11 +881,7 @@ def _run(self):
885881

886882
# Start the interactive update loop
887883
while True:
888-
input = self.hw_inputs.wait_for(
889-
HardwareButtonsConstants.ALL_KEYS,
890-
check_release=True,
891-
release_keys=[HardwareButtonsConstants.KEY_PRESS, HardwareButtonsConstants.KEY1, HardwareButtonsConstants.KEY2, HardwareButtonsConstants.KEY3]
892-
)
884+
input = self.hw_inputs.wait_for(HardwareButtonsConstants.ALL_KEYS)
893885

894886
keyboard_swap = False
895887

@@ -1466,13 +1458,13 @@ def __post_init__(self):
14661458

14671459

14681460
def _run_callback(self):
1469-
# Exit the screen on success via a non-None value
1470-
logger.info(f"verified_index: {self.verified_index.cur_count}")
1461+
# Exit the screen on success via a non-None value.
1462+
# see: ButtonListScreen._run()
14711463
if self.verified_index.cur_count is not None:
1472-
logger.info("Screen callback returning success!")
1473-
self.threads[-1].stop()
1474-
while self.threads[-1].is_alive():
1475-
time.sleep(0.01)
1464+
# Note that the ProgressThread will have already exited on its own.
1465+
1466+
# Return a success value (anything other than None) to end the
1467+
# ButtonListScreen._run() loop.
14761468
return 1
14771469

14781470

@@ -1489,10 +1481,13 @@ def run(self):
14891481
while self.keep_running:
14901482
if self.verified_index.cur_count is not None:
14911483
# This thread will detect the success state while its parent Screen
1492-
# holds in its `wait_for`. Have to trigger a hw_input event to break
1493-
# the Screen._run out of the `wait_for` state. The Screen will then
1494-
# call its `_run_callback` and detect the success state and exit.
1495-
HardwareButtons.get_instance().trigger_override(force_release=True)
1484+
# blocks in its `wait_for`. Have to trigger a hw_input override event
1485+
# to break the Screen._run out of the `wait_for` state. The Screen
1486+
# will then call its `_run_callback` and detect the success state and
1487+
# exit.
1488+
HardwareButtons.get_instance().trigger_override()
1489+
1490+
# Exit the loop and thereby end this thread
14961491
return
14971492

14981493
textarea = TextArea(

src/seedsigner/gui/screens/settings_screens.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ def _run(self):
183183
screen_y=int((self.canvas_height - msg_height)/ 2),
184184
)
185185
while True:
186-
input = self.hw_inputs.wait_for(keys=HardwareButtonsConstants.ALL_KEYS, check_release=False)
186+
input = self.hw_inputs.wait_for(keys=HardwareButtonsConstants.ALL_KEYS)
187187

188188
if input == HardwareButtonsConstants.KEY1:
189189
# Note that there are three distinct screen updates that happen at

src/seedsigner/hardware/buttons.py

+50-69
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class HardwareButtons(Singleton):
3333
KEY2_PIN = 12
3434
KEY3_PIN = 8
3535

36+
3637
@classmethod
3738
def get_instance(cls):
3839
# This is the only way to access the one and only instance
@@ -53,8 +54,6 @@ def get_instance(cls):
5354
cls._instance.GPIO = GPIO
5455
cls._instance.override_ind = False
5556

56-
cls._instance.add_events([HardwareButtonsConstants.KEY_UP, HardwareButtonsConstants.KEY_DOWN, HardwareButtonsConstants.KEY_PRESS, HardwareButtonsConstants.KEY_LEFT, HardwareButtonsConstants.KEY_RIGHT, HardwareButtonsConstants.KEY1, HardwareButtonsConstants.KEY2, HardwareButtonsConstants.KEY3])
57-
5857
# Track state over time so we can apply input delays/ignores as needed
5958
cls._instance.cur_input = None # Track which direction or button was last pressed
6059
cls._instance.cur_input_started = None # Track when that input began
@@ -65,25 +64,30 @@ def get_instance(cls):
6564
return cls._instance
6665

6766

68-
6967
@classmethod
7068
def get_instance_no_hardware(cls):
7169
# This is the only way to access the one and only instance
7270
if cls._instance is None:
7371
cls._instance = cls.__new__(cls)
7472

7573

74+
def wait_for(self, keys=[]) -> int:
75+
"""
76+
Block execution until one of the target keys is pressed.
7677
77-
def wait_for(self, keys=[], check_release=True, release_keys=[]) -> int:
78+
Optionally override the wait by calling `trigger_override()`.
79+
"""
7880
# TODO: Refactor to keep control in the Controller and not here
7981
from seedsigner.controller import Controller
8082
controller = Controller.get_instance()
81-
82-
if not release_keys:
83-
release_keys = keys
8483
self.override_ind = False
8584

8685
while True:
86+
if self.override_ind:
87+
# Break out of the wait_for without waiting for user input
88+
self.override_ind = False
89+
return HardwareButtonsConstants.OVERRIDE
90+
8791
cur_time = int(time.time() * 1000)
8892
if cur_time - self.last_input_time > controller.screensaver_activation_ms and not controller.is_screensaver_running:
8993
# Start the screensaver. Will block execution until input detected.
@@ -99,48 +103,42 @@ def wait_for(self, keys=[], check_release=True, release_keys=[]) -> int:
99103
# Resume from a fresh loop
100104
continue
101105

106+
# Check each candidate key to see if it was pressed
102107
for key in keys:
103-
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):
104-
# when check release is False or the release lock is released (True)
105-
if self.GPIO.input(key) == GPIO.LOW or self.override_ind:
106-
HardwareButtonsConstants.release_lock = False
107-
if self.override_ind:
108-
self.override_ind = False
109-
return HardwareButtonsConstants.OVERRIDE
110-
111-
if self.cur_input != key:
112-
self.cur_input = key
113-
self.cur_input_started = int(time.time() * 1000) # in milliseconds
114-
self.last_input_time = self.cur_input_started
108+
if self.GPIO.input(key) == GPIO.LOW:
109+
if self.cur_input != key:
110+
self.cur_input = key
111+
self.cur_input_started = int(time.time() * 1000) # in milliseconds
112+
self.last_input_time = self.cur_input_started
113+
return key
114+
115+
else:
116+
# Still pressing the same input
117+
if cur_time - self.last_input_time > self.next_repeat_threshold:
118+
# Too much time has elapsed to consider this the same
119+
# continuous input. Treat as a new separate press.
120+
self.cur_input_started = cur_time
121+
self.last_input_time = cur_time
122+
return key
123+
124+
elif cur_time - self.cur_input_started > self.first_repeat_threshold:
125+
# We're good to relay this immediately as continuous
126+
# input.
127+
self.last_input_time = cur_time
115128
return key
116129

117130
else:
118-
# Still pressing the same input
119-
if cur_time - self.last_input_time > self.next_repeat_threshold:
120-
# Too much time has elapsed to consider this the same
121-
# continuous input. Treat as a new separate press.
122-
self.cur_input_started = cur_time
123-
self.last_input_time = cur_time
124-
return key
125-
126-
elif cur_time - self.cur_input_started > self.first_repeat_threshold:
127-
# We're good to relay this immediately as continuous
128-
# input.
129-
self.last_input_time = cur_time
130-
return key
131-
132-
else:
133-
# We're not yet at the first repeat threshold; triggering
134-
# a key now would be too soon and yields a bad user
135-
# experience when only a single click was intended but
136-
# a second input is processed because of race condition
137-
# against human response time to release the button.
138-
# So there has to be a delay before we allow the first
139-
# continuous repeat to register. So we'll ignore this
140-
# round's input and **won't update any of our
141-
# timekeeping vars**. But once we cross the threshold,
142-
# we let the repeats fly.
143-
pass
131+
# We're not yet at the first repeat threshold; triggering
132+
# a key now would be too soon and yields a bad user
133+
# experience when only a single click was intended but
134+
# a second input is processed because of race condition
135+
# against human response time to release the button.
136+
# So there has to be a delay before we allow the first
137+
# continuous repeat to register. So we'll ignore this
138+
# round's input and **won't update any of our
139+
# timekeeping vars**. But once we cross the threshold,
140+
# we let the repeats fly.
141+
pass
144142

145143
time.sleep(0.01) # wait 10 ms to give CPU chance to do other things
146144

@@ -149,29 +147,13 @@ def update_last_input_time(self):
149147
self.last_input_time = int(time.time() * 1000)
150148

151149

152-
def add_events(self, keys=[]):
153-
for key in keys:
154-
GPIO.add_event_detect(key, self.GPIO.RISING, callback=HardwareButtons.rising_callback)
155-
150+
def trigger_override(self) -> bool:
151+
""" Set the override flag to break out of the current `wait_for` loop """
152+
self.override_ind = True
156153

157-
def rising_callback(channel):
158-
HardwareButtonsConstants.release_lock = True
159-
160-
161-
def trigger_override(self, force_release = False) -> bool:
162-
if force_release:
163-
HardwareButtonsConstants.release_lock = True
164-
165-
if not self.override_ind:
166-
self.override_ind = True
167-
return True
168-
return False
169-
170-
def force_release(self) -> bool:
171-
HardwareButtonsConstants.release_lock = True
172-
return True
173154

174155
def check_for_low(self, key: int = None, keys: List[int] = None) -> bool:
156+
""" Returns True if one of the target keys/key is pressed """
175157
if key:
176158
keys = [key]
177159
for key in keys:
@@ -181,15 +163,16 @@ def check_for_low(self, key: int = None, keys: List[int] = None) -> bool:
181163
else:
182164
return False
183165

166+
184167
def has_any_input(self) -> bool:
168+
""" Returns True if any of the keys are pressed """
185169
for key in HardwareButtonsConstants.ALL_KEYS:
186170
if self.GPIO.input(key) == GPIO.LOW:
187171
return True
188172
return False
189173

174+
190175
# class used as short hand for static button/channel lookup values
191-
# TODO: Implement `release_lock` functionality as a global somewhere. Mixes up design
192-
# patterns to have a static constants class plus a settable global value.
193176
class HardwareButtonsConstants:
194177
if GPIO.RPI_INFO['P1_REVISION'] == 3: #This indicates that we have revision 3 GPIO
195178
KEY_UP = 31
@@ -227,5 +210,3 @@ class HardwareButtonsConstants:
227210

228211
KEYS__LEFT_RIGHT_UP_DOWN = [KEY_LEFT, KEY_RIGHT, KEY_UP, KEY_DOWN]
229212
KEYS__ANYCLICK = [KEY_PRESS, KEY1, KEY2, KEY3]
230-
231-
release_lock = True # released when True, locked when False

0 commit comments

Comments
 (0)