From 8cf1e9a1f0c9bf78d1237139638c10abf4f7d328 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:27:12 +0000 Subject: [PATCH 1/6] Initial plan From 429199cdb85c3f42cd86e1b42279c77eb59dc0cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:39:29 +0000 Subject: [PATCH 2/6] Fix check_for_low() to latch button presses released between polling calls When check_for_low() sees a pin transition from LOW to HIGH between calls, and the debounce threshold was met, treat it as a valid press instead of silently discarding it. This fixes brief button clicks being missed in slow polling loops like the camera entropy preview (issue #342). Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> Agent-Logs-Url: https://github.com/3rdIteration/seedsigner/sessions/a5b1eaa9-e012-4b01-849c-8bb6c92d9ab0 --- src/seedsigner/hardware/buttons.py | 13 ++- tests/test_buttons_gpio_resolution.py | 133 ++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/src/seedsigner/hardware/buttons.py b/src/seedsigner/hardware/buttons.py index 18ace3301..c225d2060 100644 --- a/src/seedsigner/hardware/buttons.py +++ b/src/seedsigner/hardware/buttons.py @@ -413,7 +413,18 @@ def check_for_low(self, key=None, keys: List = None) -> bool: continue self.update_last_input_time() return True - self._low_since_ms[key] = None + else: + # Pin is high. If it was previously observed as low, the + # button was pressed and released between polling calls. + # Treat this as a valid press provided the debounce + # interval was met, so that brief clicks are not lost in + # slow polling loops (e.g. camera preview). + low_since = self._low_since_ms.get(key) + if low_since is not None: + self._low_since_ms[key] = None + if cur_time - low_since >= self.debounce_threshold_ms: + self.update_last_input_time() + return True return False pygame.event.pump() diff --git a/tests/test_buttons_gpio_resolution.py b/tests/test_buttons_gpio_resolution.py index de2000439..3943deff8 100644 --- a/tests/test_buttons_gpio_resolution.py +++ b/tests/test_buttons_gpio_resolution.py @@ -293,3 +293,136 @@ def read(self): assert not instance.check_for_low(key="KEY1") # has_any_input should not crash when some buttons are disabled assert not instance.has_any_input() + + +# --------------------------------------------------------------------------- +# check_for_low latching tests – verifies that a brief button press (pressed +# and released between two slow polling calls) is still detected. +# --------------------------------------------------------------------------- + +def _make_instance_with_controllable_pins(monkeypatch): + """Create a HardwareButtons instance whose GPIO pins can be individually + controlled via a ``pin_states`` dict (True = high / not pressed, + False = low / pressed).""" + + pin_states = {} + + button_map = { + "KEY_UP": ["/dev/gpiochip1", 25, "pull_up"], + "KEY_DOWN": ["/dev/gpiochip1", 27, "pull_up"], + "KEY_LEFT": ["/dev/gpiochip1", 24, "pull_up"], + "KEY_RIGHT": ["/dev/gpiochip1", 22, "pull_up"], + "KEY_PRESS": ["/dev/gpiochip1", 26, "pull_up"], + "KEY1": ["/dev/gpiochip1", 17, "pull_up"], + "KEY2": ["/dev/gpiochip0", 4, "pull_up"], + "KEY3": ["/dev/gpiochip1", 21, "pull_up"], + } + + class ControllableGPIO: + def __init__(self, *args, **kwargs): + self._name = None + + def close(self): + pass + + def read(self): + if self._name is not None: + return pin_states.get(self._name, True) + return True + + monkeypatch.setattr(buttons_module, "USING_GPIO", True) + monkeypatch.setattr(buttons_module.Settings, "get_platform_default_hardware_config", lambda: "FOX_22") + monkeypatch.setattr(buttons_module, "get_hardware_pin_mapping", lambda _: {"buttons": button_map}) + monkeypatch.setattr(buttons_module, "GPIO", ControllableGPIO) + + instance = buttons_module.HardwareButtons.get_instance() + + # Map each pin object to its button name so we can control read() values + for name, pin_obj in instance._gpio_pins.items(): + pin_obj._name = name + pin_states[name] = True # all buttons start high (not pressed) + + return instance, pin_states + + +def test_check_for_low_detects_held_button(monkeypatch): + """A button held across two check_for_low calls is detected.""" + instance, pin_states = _make_instance_with_controllable_pins(monkeypatch) + + # First call: pin goes low → records timestamp, returns False + pin_states["KEY_PRESS"] = False # pressed + assert not instance.check_for_low(key="KEY_PRESS") + + # Simulate time passing beyond debounce threshold + instance._low_since_ms["KEY_PRESS"] = int(time.time() * 1000) - 20 + + # Second call: pin still low, debounce met → returns True + assert instance.check_for_low(key="KEY_PRESS") + + +def test_check_for_low_detects_released_button(monkeypatch): + """A button pressed and released between two slow polling calls is detected + (the latching fix for issue #342).""" + instance, pin_states = _make_instance_with_controllable_pins(monkeypatch) + + # First call: pin is low → records timestamp, returns False + pin_states["KEY_PRESS"] = False # pressed + assert not instance.check_for_low(key="KEY_PRESS") + + # Simulate time passing beyond debounce threshold + instance._low_since_ms["KEY_PRESS"] = int(time.time() * 1000) - 20 + + # Button is released before the next call + pin_states["KEY_PRESS"] = True # released + + # Second call: pin is high but was previously low → latched press detected + assert instance.check_for_low(key="KEY_PRESS") + + # State is cleared after detection + assert instance._low_since_ms["KEY_PRESS"] is None + + # Subsequent call with pin still high returns False (no double-fire) + assert not instance.check_for_low(key="KEY_PRESS") + + +def test_check_for_low_ignores_noise_within_debounce(monkeypatch): + """A very brief low (within debounce window) that goes high is not + treated as a valid press.""" + instance, pin_states = _make_instance_with_controllable_pins(monkeypatch) + + # Pin goes low briefly + pin_states["KEY_PRESS"] = False + assert not instance.check_for_low(key="KEY_PRESS") + + # Do NOT advance the timestamp – debounce window not met + # (_low_since_ms stays at current time, so next check is within debounce) + + # Pin goes high immediately + pin_states["KEY_PRESS"] = True + + # Should NOT return True because debounce threshold was not met + assert not instance.check_for_low(key="KEY_PRESS") + + # State should be cleared + assert instance._low_since_ms["KEY_PRESS"] is None + + +def test_check_for_low_latching_works_with_keys_list(monkeypatch): + """Latching works when checking multiple keys at once (the ANYCLICK + pattern used in camera preview).""" + instance, pin_states = _make_instance_with_controllable_pins(monkeypatch) + + anyclick = ["KEY_PRESS", "KEY1", "KEY2", "KEY3"] + + # KEY1 is briefly pressed + pin_states["KEY1"] = False + assert not instance.check_for_low(keys=anyclick) + + # Simulate time passing beyond debounce threshold + instance._low_since_ms["KEY1"] = int(time.time() * 1000) - 20 + + # KEY1 is released + pin_states["KEY1"] = True + + # Should detect the latched press + assert instance.check_for_low(keys=anyclick) From 30081c9a270c65558efd868cbce0b0e6d9f0b9a5 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 22:15:07 -0400 Subject: [PATCH 3/6] =?UTF-8?q?Add=20ST7735=20128=C3=97128=20display=20sup?= =?UTF-8?q?port=20with=20very-long-press=20driver=20switching=20(#341)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * Add ST7735S display driver for Waveshare 1.44" LCD HAT (128x128) - New ST7735.py driver with full init sequence, offsets, lazy init, SPI_NO_CS - Updated display_driver.py factory with ST7735 type - Added st7735_128x128 and desktop_128x128 display configurations - Updated renderer.py to handle ST7735 (same canvas orientation as ST7789) - 7 new tests for ST7735 driver (dimensions, SPI, lazy init, SLPOUT timing) - Updated docs/io_config.md with 1.44" HAT documentation Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> * Fix hardcoded 240x240 values in QRDisplayScreen and Keyboard for resolution independence - QRDisplayScreen now uses renderer.canvas_width/height instead of 240, 240 - Keyboard default rect now computed from Renderer dimensions instead of hardcoded (0,40,240,240) Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> * Fix em-dash to ASCII hyphen in test comment * Fix ST7735 128x128 display: correct color order and add UI scaling - ST7735.py: Change MADCTL from 0x60 to 0x68 (add BGR bit) to fix red/blue channel swap that caused orange buttons to appear blue - renderer.py: Render UI at 240x240 (native design resolution) and downscale to 128x128 for the ST7735 display, so all UI elements are properly scaled instead of being clipped Agent-Logs-Url: https://github.com/3rdIteration/seedsigner/sessions/570ee10b-35d2-41d6-a050-f7000f5a7992 Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> * Add very-long-press (5s) joystick display driver switch on homescreen Hold joystick direction for 5+ seconds on the main menu to switch display configuration without navigating to Settings: - UP → ST7789-240x240 - RIGHT → ST7789-320x240 - DOWN → ST7735-128x128 The display setting is persisted (if persistence is enabled) and the screen re-renders itself with the new display dimensions. Agent-Logs-Url: https://github.com/3rdIteration/seedsigner/sessions/663adbf5-59b2-43e4-a99c-c03ca26196da Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> * Fix comment clarity: '5 s' → '5 seconds' per code review Agent-Logs-Url: https://github.com/3rdIteration/seedsigner/sessions/663adbf5-59b2-43e4-a99c-c03ca26196da Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> * Document supported displays and very-long-press display switching Add "Supported displays" section to hardware_platform_support.md listing all display modules (ST7789 240x240, ST7789 320x240, ST7735S 128x128, ILI9341 320x240) with config values, drivers, and notes. Add "Display switching shortcut" section documenting the 5-second joystick hold feature on the home screen: UP → ST7789 240×240 RIGHT → ST7789 320×240 DOWN → ST7735 128×128 Agent-Logs-Url: https://github.com/3rdIteration/seedsigner/sessions/2253b284-331c-46ce-a73e-f6c81356ece6 Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> --- docs/hardware_platform_support.md | 32 ++ docs/io_config.md | 14 +- src/seedsigner/gui/keyboard.py | 6 +- src/seedsigner/gui/renderer.py | 30 +- src/seedsigner/gui/screens/screen.py | 56 ++- src/seedsigner/hardware/displays/ST7735.py | 328 ++++++++++++++++++ .../hardware/displays/display_driver.py | 8 + src/seedsigner/models/settings_definition.py | 5 + src/seedsigner/views/view.py | 7 +- tests/test_io_config_profiles.py | 220 ++++++++++++ 10 files changed, 697 insertions(+), 9 deletions(-) create mode 100644 src/seedsigner/hardware/displays/ST7735.py diff --git a/docs/hardware_platform_support.md b/docs/hardware_platform_support.md index 7e5999902..dafab0484 100644 --- a/docs/hardware_platform_support.md +++ b/docs/hardware_platform_support.md @@ -33,6 +33,38 @@ Current profiles in `io_config.json`: - `FOX_PI` (Luckfox Pico Pi) - `LC_LAFRITE` (Libre Computer La Frite AML-S805X-AC, USB camera) +## Supported displays + +SeedSigner supports several SPI display modules. The active display driver is +selected via **Settings → Hardware → Display type** (or via a SettingsQR). + +| Display | Config value | Resolution | Driver | Notes | +|---------|-------------|------------|--------|-------| +| Waveshare 1.3" LCD HAT (ST7789) | `st7789_240x240` | 240×240 | `ST7789.py` | Default; original SeedSigner display | +| ST7789 320×240 (e.g. 2.0" IPS) | `st7789_320x240` | 320×240 | `st7789_mpy.py` | Natively portrait; 90° rotation applied | +| Waveshare 1.44" LCD HAT (ST7735S) | `st7735_128x128` | 128×128 | `ST7735.py` | UI renders at 240×240 and downscales | +| ILI9341 320×240 | `ili9341_320x240` | 320×240 | `ili9341.py` | Beta support | + +The Waveshare 1.3" and 1.44" LCD HATs share the same GPIO40 header pinout and +use the same `RPI_40` hardware profile — only the display driver setting differs. +See `docs/io_config.md` for wiring details. + +## Display switching shortcut (very-long-press) + +While on the **home screen**, holding the joystick in one direction for +**5 seconds or more** will switch the display driver without navigating to +Settings. This is useful when the current display setting doesn't match the +physical hardware (e.g. after swapping HATs) and the screen is unreadable. + +| Joystick direction | Switches to | +|--------------------|-------------| +| **Up** (hold 5 s) | ST7789 240×240 | +| **Right** (hold 5 s) | ST7789 320×240 | +| **Down** (hold 5 s) | ST7735 128×128 | + +The setting is persisted if **Persistent Settings** is enabled. After switching, +the home screen re-renders automatically with the new display dimensions. + ## Button GPIO mapping format Button mappings live under each model's `buttons` object. diff --git a/docs/io_config.md b/docs/io_config.md index 34e117c05..de75ff673 100644 --- a/docs/io_config.md +++ b/docs/io_config.md @@ -40,7 +40,8 @@ The table below uses standard 40-pin physical numbering and highlights: ## Waveshare SPI display pin notes -For the Waveshare 1.3" LCD HAT on a GPIO40 header: +The Waveshare 1.3" LCD HAT (ST7789, 240×240) and 1.44" LCD HAT (ST7735S, +128×128) share the same GPIO40 header pinout: - `SPI0_MOSI`: pin `19` (`GPIO10`) - `SPI0_SCLK`: pin `23` (`GPIO11`) - `CS` / `LCD-CS` (`SPI0_CE0`): pin `24` (`GPIO8`) @@ -50,7 +51,16 @@ For the Waveshare 1.3" LCD HAT on a GPIO40 header: - Power: pin `1` (`3V3`) - Ground: e.g. pin `6` (`GND`) -These are the standard Waveshare/RPi-style assignments that the `RPI_40` profile follows. +Both HATs use the same `RPI_40` hardware profile — the only difference is the +display driver setting: + +| HAT | Display setting | Driver | +|-----|----------------|--------| +| 1.3" LCD HAT (240×240) | `st7789_240x240` (default) | `ST7789.py` | +| 1.44" LCD HAT (128×128) | `st7735_128x128` | `ST7735.py` | + +To use the 1.44" HAT, change the **Display type** setting to `st7735 128x128` +(or use a SettingsQR with `disp_conf=st7735_128x128`). ### CS and the three wiring options diff --git a/src/seedsigner/gui/keyboard.py b/src/seedsigner/gui/keyboard.py index bc679ec5a..6d97abbaf 100644 --- a/src/seedsigner/gui/keyboard.py +++ b/src/seedsigner/gui/keyboard.py @@ -179,7 +179,7 @@ def __init__(self, selected_char="a", rows=4, cols=10, - rect=(0,40, 240,240), + rect=None, additional_keys=[KEY_BACKSPACE], auto_wrap=[WRAP_TOP, WRAP_BOTTOM, WRAP_LEFT, WRAP_RIGHT], render_now=True, @@ -192,6 +192,10 @@ def __init__(self, self.charset = charset self.rows = rows self.cols = cols + if rect is None: + from seedsigner.gui.renderer import Renderer + renderer = Renderer.get_instance() + rect = (0, GUIConstants.TOP_NAV_HEIGHT, renderer.canvas_width, renderer.canvas_height) self.rect = rect self.font = Fonts.get_font(font_name, font_size) diff --git a/src/seedsigner/gui/renderer.py b/src/seedsigner/gui/renderer.py index 344626871..18222a14f 100644 --- a/src/seedsigner/gui/renderer.py +++ b/src/seedsigner/gui/renderer.py @@ -6,6 +6,7 @@ DISPLAY_TYPE__ILI9341, DISPLAY_TYPE__ILI9486, DISPLAY_TYPE__ST7789, + DISPLAY_TYPE__ST7735, DISPLAY_TYPE__DESKTOP, DisplayDriver, ) @@ -23,6 +24,8 @@ class Renderer(ConfigurableSingleton): draw: ImageDraw.ImageDraw = None disp = None lock = Lock() + _needs_resize = False + _display_size = (0, 0) @classmethod @@ -60,21 +63,42 @@ def initialize_display(self): self.canvas_width = self.disp.width self.canvas_height = self.disp.height + elif self.display_type == DISPLAY_TYPE__ST7735: + # The UI is designed for 240×240; render at that resolution + # and downscale to the physical 128×128 display in + # show_image(). + self.canvas_width = 240 + self.canvas_height = 240 + elif self.display_type in [DISPLAY_TYPE__ILI9341, DISPLAY_TYPE__ILI9486]: # Swap for the natively portrait-oriented displays self.canvas_width = self.disp.height self.canvas_height = self.disp.width + self._needs_resize = ( + self.canvas_width != self.disp.width + or self.canvas_height != self.disp.height + ) + self._display_size = (self.disp.width, self.disp.height) + self.canvas = Image.new('RGB', (self.canvas_width, self.canvas_height)) self.draw = ImageDraw.Draw(self.canvas) finally: self.lock.release() + def _resize_for_display(self, image): + """Downscale *image* to the physical display size when the canvas is + larger than the display (e.g. 240×240 canvas on a 128×128 ST7735).""" + if self._needs_resize: + return image.resize(self._display_size, Image.LANCZOS) + return image + + def show_image(self, image=None, alpha_overlay=None, show_direct=False): if show_direct: # Use the incoming image as the canvas and immediately render - self.disp.show_image(image, 0, 0) + self.disp.show_image(self._resize_for_display(image), 0, 0) return if alpha_overlay: @@ -86,7 +110,7 @@ def show_image(self, image=None, alpha_overlay=None, show_direct=False): # Always write to the current canvas, rather than trying to replace it self.canvas.paste(image) - self.disp.show_image(self.canvas, 0, 0) + self.disp.show_image(self._resize_for_display(self.canvas), 0, 0) def show_image_pan(self, image, start_x, start_y, end_x, end_y, rate, alpha_overlay=None): @@ -120,7 +144,7 @@ def show_image_pan(self, image, start_x, start_y, end_x, end_y, rate, alpha_over # Always keep a copy of the current display in the canvas self.canvas.paste(crop) - self.disp.show_image(crop, 0, 0) + self.disp.show_image(self._resize_for_display(crop), 0, 0) diff --git a/src/seedsigner/gui/screens/screen.py b/src/seedsigner/gui/screens/screen.py index 68e8f66f9..949b72301 100644 --- a/src/seedsigner/gui/screens/screen.py +++ b/src/seedsigner/gui/screens/screen.py @@ -25,6 +25,7 @@ # screens with buttons. RET_CODE__BACK_BUTTON = 1000 RET_CODE__POWER_BUTTON = 1001 +RET_CODE__DISPLAY_TOGGLE = 1002 @@ -866,7 +867,7 @@ def run(self): # Display the brightness tips toast duration = 10 ** 9 * 1.2 # 1.2 seconds if is_brightness_tip_enabled and time.time_ns() - self.tips_start_time.cur_count < duration: - image = self.qr_encoder.part_to_image(self.qr_encoder.cur_part(), 240, 240, border=2, background_color=hex_color) + image = self.qr_encoder.part_to_image(self.qr_encoder.cur_part(), self.renderer.canvas_width, self.renderer.canvas_height, border=2, background_color=hex_color) self.render_brightness_tip(image) pending_encoder_restart = True else: @@ -876,7 +877,7 @@ def run(self): # brightness tip is stowed. self.qr_encoder.restart() pending_encoder_restart = False - image = self.qr_encoder.next_part_image(240, 240, border=2, background_color=hex_color) + image = self.qr_encoder.next_part_image(self.renderer.canvas_width, self.renderer.canvas_height, border=2, background_color=hex_color) with self.renderer.lock: self.renderer.show_image(image) @@ -1368,6 +1369,10 @@ class MainMenuScreen(LargeButtonScreen): show_back_button: bool = False show_power_button: bool = True + # Very-long-press (5 seconds) on a joystick direction switches the display driver. + VERY_LONG_PRESS_MS = 5000 + _DIRECTION_TO_DISPLAY = None # built lazily in _run_callback + def __post_init__(self): super().__post_init__() from seedsigner.hardware.battery_hat import BatteryHat @@ -1380,7 +1385,54 @@ def __post_init__(self): self.components.append(self.battery_indicator) self.threads.append(MainMenuScreen.UpdateThread(self)) + # State for tracking the very-long-press + self._hold_key = None + self._hold_start_ms = None + def _run_callback(self): + from seedsigner.models.settings import Settings + + # Lazy-build the direction→display map (avoids import-time reference to + # SettingsConstants values that are plain strings). + if MainMenuScreen._DIRECTION_TO_DISPLAY is None: + MainMenuScreen._DIRECTION_TO_DISPLAY = { + HardwareButtonsConstants.KEY_UP: SettingsConstants.DISPLAY_CONFIGURATION__ST7789__240x240, + HardwareButtonsConstants.KEY_RIGHT: SettingsConstants.DISPLAY_CONFIGURATION__ST7789__320x240, + HardwareButtonsConstants.KEY_DOWN: SettingsConstants.DISPLAY_CONFIGURATION__ST7735__128x128, + } + + cur_input = self.hw_inputs.cur_input + cur_time = int(time.time() * 1000) + + if cur_input in self._DIRECTION_TO_DISPLAY: + if cur_input != self._hold_key: + # New direction; start tracking + self._hold_key = cur_input + self._hold_start_ms = cur_time + elif cur_time - self._hold_start_ms >= self.VERY_LONG_PRESS_MS: + new_config = self._DIRECTION_TO_DISPLAY[cur_input] + settings = Settings.get_instance() + current_config = settings.get_value( + SettingsConstants.SETTING__DISPLAY_CONFIGURATION + ) + if new_config != current_config: + logger.info( + "Very-long-press detected (%s): switching display %s → %s", + cur_input, current_config, new_config, + ) + settings.set_value( + SettingsConstants.SETTING__DISPLAY_CONFIGURATION, + new_config, + ) + self.renderer.initialize_display() + return RET_CODE__DISPLAY_TOGGLE + # Same config already active; reset so we don't keep triggering + self._hold_key = None + self._hold_start_ms = None + else: + self._hold_key = None + self._hold_start_ms = None + return None class UpdateThread(BaseThread): diff --git a/src/seedsigner/hardware/displays/ST7735.py b/src/seedsigner/hardware/displays/ST7735.py new file mode 100644 index 000000000..589f782ad --- /dev/null +++ b/src/seedsigner/hardware/displays/ST7735.py @@ -0,0 +1,328 @@ +"""Driver for the ST7735S-based 128×128 1.44" Waveshare LCD HAT. + +The ST7735S controller has an internal 132×162 frame-buffer but only 128×128 +pixels are visible. Column and row address offsets are applied in +:meth:`SetWindows` to centre the active region. + +Pin wiring, SPI bus, and chip-select handling are identical to the +ST7789-based 1.3" HAT — see the ``RPI_40`` profile in ``io_config.json``. +""" + +import array +import errno +import logging +import time + +from periphery import GPIO, SPI + +from seedsigner.hardware.io_config import get_hardware_pin_mapping +from seedsigner.models.settings import Settings + +logger = logging.getLogger(__name__) + +# The ST7735S internal RAM is 132×162 but only 128×128 are visible. +# Offsets shift the address window to the correct region. These values +# correspond to MADCTL = 0x60 (MX + MV — landscape orientation) where +# the row/column exchange means X maps to physical rows and Y to +# physical columns. +_X_OFFSET = 1 +_Y_OFFSET = 2 + + +class ST7735: + """Driver for the ST7735S 128×128 1.44-inch LCD HAT.""" + + def __init__(self): + self.width = 128 + self.height = 128 + # Keep SPI transfers within conservative per-message kernel limits. + self.CHUNK_SIZE = 4096 + + hardware_config = Settings.get_platform_default_hardware_config() + pin_mapping = get_hardware_pin_mapping(hardware_config)["display"] + + self._dc = GPIO(*pin_mapping["dc"], "out") + self._rst = GPIO(*pin_mapping["rst"], "out") + bl_config = pin_mapping["bl"] + if bl_config == "disabled": + self._bl = None + else: + self._bl = GPIO(*bl_config, "out") + self._bl.write(True) + + spi_bus = f"/dev/spidev{pin_mapping['spi_bus']}.{pin_mapping['spi_device']}" + # ST7735S maximum SPI clock is ~16 MHz (datasheet typ. 15.15 MHz + # write cycle). Use 16 MHz for reliability. + spi_hz = 16_000_000 + + spi_extra_flags = 0 + if pin_mapping.get("cs") == "disabled": + spi_extra_flags = 0x40 # SPI_NO_CS + spi_mode = 3 + logger.info( + "SPI CS mode: SPI_NO_CS (0x40), SPI Mode 3 — LCD CS tied to GND." + ) + else: + spi_mode = 0 + spi_device = pin_mapping.get("spi_device", 0) + logger.warning( + "SPI CS mode: kernel-managed CE%d GPIO — LCD CS must be " + "asserted (wired to CE%d or tied to GND).", + spi_device, + spi_device, + ) + + logger.info("Initializing SPI: bus=%s at %.1f MHz", spi_bus, spi_hz / 1_000_000) + try: + self._spi = SPI(spi_bus, spi_mode, spi_hz, extra_flags=spi_extra_flags) + except OSError as e: + if e.errno == errno.EINVAL and spi_extra_flags != 0: + logger.warning( + "SPI extra_flags=0x%02x rejected (EINVAL); retrying without. " + "SPI Mode %d still active.", + spi_extra_flags, + spi_mode, + ) + self._spi = SPI(spi_bus, spi_mode, spi_hz, extra_flags=0) + else: + raise + + self._display_initialized = False + + # ------------------------------------------------------------------ + # Low-level SPI helpers + # ------------------------------------------------------------------ + + def _chunked_transfer(self, data): + """Transfer *data* in chunks to avoid kernel buffer limits.""" + if isinstance(data, list): + data = bytes(data) + + i = 0 + chunk_size = self.CHUNK_SIZE + while i < len(data): + chunk = data[i:i + chunk_size] + try: + self._spi.transfer(chunk) + i += len(chunk) + except Exception as e: + if getattr(e, "errno", None) == errno.EMSGSIZE and chunk_size > 256: + chunk_size = max(256, chunk_size // 2) + self.CHUNK_SIZE = chunk_size + logger.warning( + "SPI message too long; reducing chunk size to %d bytes", + chunk_size, + ) + continue + raise + + def command(self, cmd): + """Write a command byte.""" + self._dc.write(False) + self._spi.transfer([cmd]) + + def data(self, val): + """Write a data byte.""" + self._dc.write(True) + self._spi.transfer([val]) + + # ------------------------------------------------------------------ + # Initialisation + # ------------------------------------------------------------------ + + def _ensure_initialized(self): + if not self._display_initialized: + self.init() + self._display_initialized = True + + def reset(self): + """Pulse the hardware reset line.""" + self._rst.write(True) + time.sleep(0.01) + self._rst.write(False) + time.sleep(0.01) + self._rst.write(True) + time.sleep(0.01) + + def init(self): + """Send the ST7735S register initialisation sequence.""" + self.reset() + + # ------ Frame-rate control ------ + self.command(0xB1) # FRMCTR1 — Normal mode + self.data(0x01) + self.data(0x2C) + self.data(0x2D) + + self.command(0xB2) # FRMCTR2 — Idle mode + self.data(0x01) + self.data(0x2C) + self.data(0x2D) + + self.command(0xB3) # FRMCTR3 — Partial mode (dot + line inversion) + self.data(0x01) + self.data(0x2C) + self.data(0x2D) + self.data(0x01) + self.data(0x2C) + self.data(0x2D) + + # ------ Display inversion control ------ + self.command(0xB4) # INVCTR — column inversion + self.data(0x07) + + # ------ Power control ------ + self.command(0xC0) # PWCTR1 + self.data(0xA2) + self.data(0x02) + self.data(0x84) + + self.command(0xC1) # PWCTR2 + self.data(0xC5) + + self.command(0xC2) # PWCTR3 — normal mode + self.data(0x0A) + self.data(0x00) + + self.command(0xC3) # PWCTR4 — idle mode + self.data(0x8A) + self.data(0x2A) + + self.command(0xC4) # PWCTR5 — partial mode + self.data(0x8A) + self.data(0xEE) + + # ------ VCOM control ------ + self.command(0xC5) # VMCTR1 + self.data(0x0E) + + # ------ Memory access (orientation) ------ + # 0x68 = MX | MV | BGR → landscape, same wiring as 1.3" HAT. + # The ST7735S panel has physically BGR-ordered sub-pixels, so the + # BGR bit (0x08) must be set for correct colour rendering. + # Without it, red and blue channels are swapped (orange appears + # blue, etc.). + self.command(0x36) # MADCTL + self.data(0x68) + + # ------ Gamma correction ------ + self.command(0xE0) # GAMCTRP1 — positive gamma + self.data(0x0F) + self.data(0x1A) + self.data(0x0F) + self.data(0x18) + self.data(0x2F) + self.data(0x28) + self.data(0x20) + self.data(0x22) + self.data(0x1F) + self.data(0x1B) + self.data(0x23) + self.data(0x37) + self.data(0x00) + self.data(0x07) + self.data(0x02) + self.data(0x10) + + self.command(0xE1) # GAMCTRN1 — negative gamma + self.data(0x0F) + self.data(0x1B) + self.data(0x0F) + self.data(0x17) + self.data(0x33) + self.data(0x2C) + self.data(0x29) + self.data(0x2E) + self.data(0x30) + self.data(0x30) + self.data(0x39) + self.data(0x3F) + self.data(0x00) + self.data(0x07) + self.data(0x03) + self.data(0x10) + + # ------ Enable test command / disable RAM power-save ------ + self.command(0xF0) + self.data(0x01) + self.command(0xF6) + self.data(0x00) + + # ------ Colour mode: 16-bit (65k colours) ------ + self.command(0x3A) # COLMOD + self.data(0x05) + + # ------ Sleep out + display on ------ + self.command(0x11) # SLPOUT + time.sleep(0.150) # ≥120 ms required by datasheet + + self.command(0x29) # DISPON + + # ------------------------------------------------------------------ + # Drawing helpers + # ------------------------------------------------------------------ + + def SetWindows(self, Xstart, Ystart, Xend, Yend): + """Set the column / row address window with ST7735S offsets.""" + self.command(0x2A) # CASET + self.data(0x00) + self.data((Xstart + _X_OFFSET) & 0xFF) + self.data(0x00) + self.data((Xend - 1 + _X_OFFSET) & 0xFF) + + self.command(0x2B) # RASET + self.data(0x00) + self.data((Ystart + _Y_OFFSET) & 0xFF) + self.data(0x00) + self.data((Yend - 1 + _Y_OFFSET) & 0xFF) + + self.command(0x2C) # RAMWR + + def show_image(self, Image, Xstart, Ystart): + """Write a PIL Image to the display.""" + self._ensure_initialized() + imwidth, imheight = Image.size + if imwidth != self.width or imheight != self.height: + raise ValueError( + f"Image must be same dimensions as display " + f"({self.width}x{self.height})." + ) + # Same 24-bit RGB → 16-bit RGB565 conversion as ST7789: + # convert to gBRG-3:5:5:3 then per-pixel byte-swap. + arr = array.array("H", Image.convert("BGR;16").tobytes()) + arr.byteswap() + pix = arr.tobytes() + self.SetWindows(0, 0, self.width, self.height) + self._dc.write(True) + self._chunked_transfer(pix) + + def clear(self): + """Fill the display with white.""" + self._ensure_initialized() + _buffer = [0xFF] * (self.width * self.height * 2) + self.SetWindows(0, 0, self.width, self.height) + self._dc.write(True) + self._chunked_transfer(_buffer) + + def invert(self, enabled: bool = True): + """Toggle display colour inversion.""" + self._ensure_initialized() + self.command(0x21 if enabled else 0x20) + + # ------------------------------------------------------------------ + # Cleanup + # ------------------------------------------------------------------ + + def __del__(self): + self.close() + + def close(self): + for attr_name in ("_dc", "_rst", "_bl", "_spi"): + resource = getattr(self, attr_name, None) + if resource is None: + continue + try: + resource.close() + except Exception: + pass + setattr(self, attr_name, None) diff --git a/src/seedsigner/hardware/displays/display_driver.py b/src/seedsigner/hardware/displays/display_driver.py index 158267394..0d3b3010f 100644 --- a/src/seedsigner/hardware/displays/display_driver.py +++ b/src/seedsigner/hardware/displays/display_driver.py @@ -1,12 +1,14 @@ """Factory for selecting the appropriate display backend.""" DISPLAY_TYPE__ST7789 = "st7789" +DISPLAY_TYPE__ST7735 = "st7735" DISPLAY_TYPE__ILI9341 = "ili9341" DISPLAY_TYPE__ILI9486 = "ili9486" DISPLAY_TYPE__DESKTOP = "desktop" ALL_DISPLAY_TYPES = [ DISPLAY_TYPE__ST7789, + DISPLAY_TYPE__ST7735, DISPLAY_TYPE__ILI9341, DISPLAY_TYPE__ILI9486, DISPLAY_TYPE__DESKTOP, @@ -37,6 +39,12 @@ def __init__(self, display_type: str = DISPLAY_TYPE__ST7789, width: int = None, # Have to swap width and height; screen is natively 240x320 self.display = ST7789(width=height, height=width) + elif self.display_type == DISPLAY_TYPE__ST7735: + if width != 128 or height != 128: + raise ValueError("ST7735 display only supports 128x128 resolution") + from seedsigner.hardware.displays.ST7735 import ST7735 + self.display = ST7735() + elif self.display_type == DISPLAY_TYPE__ILI9341: from seedsigner.hardware.displays.ili9341 import ILI9341 self.display = ILI9341() diff --git a/src/seedsigner/models/settings_definition.py b/src/seedsigner/models/settings_definition.py index 17d1237f6..ce8ef706a 100644 --- a/src/seedsigner/models/settings_definition.py +++ b/src/seedsigner/models/settings_definition.py @@ -484,23 +484,28 @@ def map_network_to_embit(cls, network) -> str: # Hardware config settings DISPLAY_CONFIGURATION__ST7789__240x240 = "st7789_240x240" # default; original Waveshare 1.3" display hat DISPLAY_CONFIGURATION__ST7789__320x240 = "st7789_320x240" # natively portrait dimensions; we apply a 90° rotation + DISPLAY_CONFIGURATION__ST7735__128x128 = "st7735_128x128" # Waveshare 1.44" LCD HAT; ST7735S controller DISPLAY_CONFIGURATION__ILI9341__320x240 = "ili9341_320x240" # natively portrait dimensions; we apply a 90° rotation DISPLAY_CONFIGURATION__ILI9486__480x320 = "ili9486_480x320" # natively portrait dimensions; we apply a 90° rotation DISPLAY_CONFIGURATION__DESKTOP__240x240 = "desktop_240x240" # pygame-based desktop simulation DISPLAY_CONFIGURATION__DESKTOP__320x240 = "desktop_320x240" + DISPLAY_CONFIGURATION__DESKTOP__128x128 = "desktop_128x128" if USING_MOCK_GPIO: ALL_DISPLAY_CONFIGURATIONS = [ (DISPLAY_CONFIGURATION__ST7789__240x240, "st7789 240x240"), (DISPLAY_CONFIGURATION__ST7789__320x240, "st7789 320x240"), + (DISPLAY_CONFIGURATION__ST7735__128x128, "st7735 128x128"), (DISPLAY_CONFIGURATION__ILI9341__320x240, "ili9341 320x240 (beta)"), (DISPLAY_CONFIGURATION__DESKTOP__240x240, "desktop 240x240"), (DISPLAY_CONFIGURATION__DESKTOP__320x240, "desktop 320x240"), + (DISPLAY_CONFIGURATION__DESKTOP__128x128, "desktop 128x128"), # (DISPLAY_CONFIGURATION__ILI9486__320x480, "ili9486 480x320"), # TODO: Enable when ili9486 driver performance is improved ] else: ALL_DISPLAY_CONFIGURATIONS = [ (DISPLAY_CONFIGURATION__ST7789__240x240, "st7789 240x240"), (DISPLAY_CONFIGURATION__ST7789__320x240, "st7789 320x240"), + (DISPLAY_CONFIGURATION__ST7735__128x128, "st7735 128x128"), (DISPLAY_CONFIGURATION__ILI9341__320x240, "ili9341 320x240 (beta)"), # (DISPLAY_CONFIGURATION__ILI9486__320x480, "ili9486 480x320"), # TODO: Enable when ili9486 driver performance is improved ] diff --git a/src/seedsigner/views/view.py b/src/seedsigner/views/view.py index 6925b4e6c..afae5febe 100644 --- a/src/seedsigner/views/view.py +++ b/src/seedsigner/views/view.py @@ -4,7 +4,7 @@ from seedsigner.helpers.l10n import mark_for_translation as _mft from seedsigner.gui.components import SeedSignerIconConstants -from seedsigner.gui.screens import RET_CODE__POWER_BUTTON, RET_CODE__BACK_BUTTON +from seedsigner.gui.screens import RET_CODE__POWER_BUTTON, RET_CODE__BACK_BUTTON, RET_CODE__DISPLAY_TOGGLE from seedsigner.gui.screens.screen import BaseScreen, ButtonOption, LargeButtonScreen, WarningScreen, ErrorScreen from seedsigner.models.settings import Settings, SettingsConstants from seedsigner.models.settings_definition import SettingsDefinition @@ -246,6 +246,11 @@ def run(self): if selected_menu_num == RET_CODE__POWER_BUTTON: return Destination(PowerOptionsView) + if selected_menu_num == RET_CODE__DISPLAY_TOGGLE: + # Display driver was switched via very-long-press; re-render the + # home screen with the new display dimensions. + return Destination(MainMenuView) + if button_data[selected_menu_num] == self.SCAN: from seedsigner.views.scan_views import ScanView return Destination(ScanView) diff --git a/tests/test_io_config_profiles.py b/tests/test_io_config_profiles.py index 109dba0d6..f344a8d7d 100644 --- a/tests/test_io_config_profiles.py +++ b/tests/test_io_config_profiles.py @@ -575,3 +575,223 @@ def test_st7789_does_not_swallow_non_einval_oserror(): # Only one SPI() call should have been made — no retry on non-EINVAL errors. assert mock_spi_cls.call_count == 1 + +# --------------------------------------------------------------------------- +# ST7735 display driver tests (Waveshare 1.44" LCD HAT, 128x128) +# --------------------------------------------------------------------------- + +def _make_st7735_pin_mapping(cs=None): + """Return a minimal display pin mapping for ST7735 tests.""" + mapping = { + "display": { + "dc": ["/dev/gpiochip0", 25], + "rst": ["/dev/gpiochip0", 27], + "bl": ["/dev/gpiochip0", 24], + "spi_bus": 0, + "spi_device": 0, + } + } + if cs is not None: + mapping["display"]["cs"] = cs + return mapping + + +def _import_st7735_with_mocked_periphery(): + """Import ST7735 with mocked periphery, analogous to _import_st7789_...""" + import sys + import types + from unittest.mock import MagicMock + + if "periphery" not in sys.modules: + fake_periphery = types.ModuleType("periphery") + fake_periphery.GPIO = MagicMock() + fake_periphery.SPI = MagicMock() + sys.modules["periphery"] = fake_periphery + + sys.modules.pop("seedsigner.hardware.displays.ST7735", None) + import seedsigner.hardware.displays.ST7735 as st7735_module + return st7735_module + + +def test_st7735_dimensions(): + """ST7735 must report 128×128 resolution.""" + from unittest.mock import patch + + st7735_module = _import_st7735_with_mocked_periphery() + pin_mapping = _make_st7735_pin_mapping() + + with patch.object(st7735_module, "GPIO"), \ + patch.object(st7735_module, "SPI"), \ + patch.object(st7735_module, "Settings") as mock_settings, \ + patch.object(st7735_module, "get_hardware_pin_mapping", return_value=pin_mapping): + mock_settings.get_platform_default_hardware_config.return_value = "RPI_40" + display = st7735_module.ST7735() + + assert display.width == 128 + assert display.height == 128 + + +def test_st7735_spi_speed(): + """ST7735 must use 16 MHz SPI speed (datasheet max ~16 MHz).""" + from unittest.mock import patch + + st7735_module = _import_st7735_with_mocked_periphery() + pin_mapping = _make_st7735_pin_mapping() + + with patch.object(st7735_module, "GPIO"), \ + patch.object(st7735_module, "SPI") as mock_spi_cls, \ + patch.object(st7735_module, "Settings") as mock_settings, \ + patch.object(st7735_module, "get_hardware_pin_mapping", return_value=pin_mapping): + mock_settings.get_platform_default_hardware_config.return_value = "RPI_40" + st7735_module.ST7735() + + mock_spi_cls.assert_called_once_with( + "/dev/spidev0.0", + 0, + 16_000_000, + extra_flags=0, + ) + + +def test_st7735_spi_cs_disabled(): + """ST7735 must use SPI Mode 3 + SPI_NO_CS when cs='disabled'.""" + from unittest.mock import patch + + st7735_module = _import_st7735_with_mocked_periphery() + pin_mapping = _make_st7735_pin_mapping(cs="disabled") + + with patch.object(st7735_module, "GPIO"), \ + patch.object(st7735_module, "SPI") as mock_spi_cls, \ + patch.object(st7735_module, "Settings") as mock_settings, \ + patch.object(st7735_module, "get_hardware_pin_mapping", return_value=pin_mapping): + mock_settings.get_platform_default_hardware_config.return_value = "RPI_40" + st7735_module.ST7735() + + mock_spi_cls.assert_called_once_with( + "/dev/spidev0.0", + 3, + 16_000_000, + extra_flags=0x40, + ) + + +def test_st7735_init_not_called_during_construction(): + """init() must be deferred (lazy init), same as ST7789.""" + from unittest.mock import patch + + st7735_module = _import_st7735_with_mocked_periphery() + pin_mapping = _make_st7735_pin_mapping() + + with patch.object(st7735_module, "GPIO"), \ + patch.object(st7735_module, "SPI"), \ + patch.object(st7735_module, "Settings") as mock_settings, \ + patch.object(st7735_module, "get_hardware_pin_mapping", return_value=pin_mapping), \ + patch.object(st7735_module.ST7735, "init") as mock_init: + mock_settings.get_platform_default_hardware_config.return_value = "RPI_40" + display = st7735_module.ST7735() + + mock_init.assert_not_called() + assert display._display_initialized is False + + +def test_st7735_init_called_on_first_show_image(): + """_ensure_initialized() must call init() exactly once.""" + from unittest.mock import patch + + st7735_module = _import_st7735_with_mocked_periphery() + pin_mapping = _make_st7735_pin_mapping() + + with patch.object(st7735_module, "GPIO"), \ + patch.object(st7735_module, "SPI"), \ + patch.object(st7735_module, "Settings") as mock_settings, \ + patch.object(st7735_module, "get_hardware_pin_mapping", return_value=pin_mapping), \ + patch.object(st7735_module.ST7735, "init") as mock_init: + mock_settings.get_platform_default_hardware_config.return_value = "RPI_40" + display = st7735_module.ST7735() + + display._ensure_initialized() + assert mock_init.call_count == 1 + assert display._display_initialized is True + + display._ensure_initialized() + assert mock_init.call_count == 1 + + +def test_st7735_init_sleeps_after_slpout(): + """init() must sleep ≥120 ms between SLPOUT and DISPON.""" + import time + from unittest.mock import patch + + ST7735_SLPOUT = 0x11 + ST7735_DISPON = 0x29 + + st7735_module = _import_st7735_with_mocked_periphery() + pin_mapping = _make_st7735_pin_mapping() + + commands_in_order = [] + + def fake_command(self_inner, cmd): + commands_in_order.append(("cmd", cmd, time.monotonic())) + + def fake_data(self_inner, val): + commands_in_order.append(("data", val, None)) + + def fake_reset(self_inner): + pass + + with patch.object(st7735_module, "GPIO"), \ + patch.object(st7735_module, "SPI"), \ + patch.object(st7735_module, "Settings") as mock_settings, \ + patch.object(st7735_module, "get_hardware_pin_mapping", return_value=pin_mapping), \ + patch.object(st7735_module.ST7735, "reset", fake_reset), \ + patch.object(st7735_module.ST7735, "command", fake_command), \ + patch.object(st7735_module.ST7735, "data", fake_data): + mock_settings.get_platform_default_hardware_config.return_value = "RPI_40" + display = st7735_module.ST7735() + display.init() + + slpout_t = next( + (e[2] for e in commands_in_order if e[0] == "cmd" and e[1] == ST7735_SLPOUT), + None, + ) + dispon_t = next( + (e[2] for e in commands_in_order if e[0] == "cmd" and e[1] == ST7735_DISPON), + None, + ) + + assert slpout_t is not None, "SLPOUT (0x11) not found in init() sequence" + assert dispon_t is not None, "DISPON (0x29) not found in init() sequence" + assert dispon_t > slpout_t, "DISPON must be sent after SLPOUT" + + delay_ms = (dispon_t - slpout_t) * 1000 + assert delay_ms >= 120, ( + f"Must sleep ≥120 ms between SLPOUT and DISPON; actual {delay_ms:.1f} ms" + ) + + +def test_st7735_falls_back_on_einval(): + """ST7735 must retry without SPI_NO_CS on EINVAL, keeping Mode 3.""" + import errno as _errno + from unittest.mock import patch, call, MagicMock + + st7735_module = _import_st7735_with_mocked_periphery() + pin_mapping = _make_st7735_pin_mapping(cs="disabled") + + mock_spi_instance = MagicMock() + einval_error = OSError(_errno.EINVAL, "Invalid argument") + einval_error.errno = _errno.EINVAL + mock_spi_cls = MagicMock(side_effect=[einval_error, mock_spi_instance]) + + with patch.object(st7735_module, "GPIO"), \ + patch.object(st7735_module, "SPI", mock_spi_cls), \ + patch.object(st7735_module, "Settings") as mock_settings, \ + patch.object(st7735_module, "get_hardware_pin_mapping", return_value=pin_mapping): + mock_settings.get_platform_default_hardware_config.return_value = "RPI_40" + display = st7735_module.ST7735() + + assert mock_spi_cls.call_count == 2 + first_call, second_call = mock_spi_cls.call_args_list + assert first_call == call("/dev/spidev0.0", 3, 16_000_000, extra_flags=0x40) + assert second_call == call("/dev/spidev0.0", 3, 16_000_000, extra_flags=0) + assert display._spi is mock_spi_instance + From 8927ddf166806f8f50b341d2c83943dc1602fcc4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:27:12 +0000 Subject: [PATCH 4/6] Initial plan Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> From 71229ddb3a1f76dc93ab8703e79e86e28f9811de Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 30 Mar 2026 02:29:15 +0000 Subject: [PATCH 5/6] Fix phantom button press when entering camera workflow wait_for() now clears _low_since_ms[key] before returning a successful press. This prevents stale button state from carrying over to the next screen's check_for_low() calls, which caused an immediate image capture when entering the camera entropy workflow. Agent-Logs-Url: https://github.com/3rdIteration/seedsigner/sessions/9be6c08f-f815-4873-9d17-db802755f015 Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> --- src/seedsigner/hardware/buttons.py | 3 +++ tests/test_buttons_gpio_resolution.py | 37 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/seedsigner/hardware/buttons.py b/src/seedsigner/hardware/buttons.py index c225d2060..711539093 100644 --- a/src/seedsigner/hardware/buttons.py +++ b/src/seedsigner/hardware/buttons.py @@ -344,14 +344,17 @@ def wait_for(self, keys: List = []) -> int: self.cur_input = key self.cur_input_started = cur_time self.last_input_time = cur_time + self._low_since_ms[key] = None return key else: if cur_time - self.last_input_time > self.next_repeat_threshold: self.cur_input_started = cur_time self.last_input_time = cur_time + self._low_since_ms[key] = None return key elif cur_time - self.cur_input_started > self.first_repeat_threshold: self.last_input_time = cur_time + self._low_since_ms[key] = None return key else: self._low_since_ms[key] = None diff --git a/tests/test_buttons_gpio_resolution.py b/tests/test_buttons_gpio_resolution.py index 3943deff8..502fef6a6 100644 --- a/tests/test_buttons_gpio_resolution.py +++ b/tests/test_buttons_gpio_resolution.py @@ -426,3 +426,40 @@ def test_check_for_low_latching_works_with_keys_list(monkeypatch): # Should detect the latched press assert instance.check_for_low(keys=anyclick) + + +def test_wait_for_clears_low_state_preventing_phantom_press(monkeypatch): + """After wait_for() returns a key press, check_for_low() must not see + stale _low_since_ms state and report a phantom press (the 'camera + captures immediately' bug).""" + instance, pin_states = _make_instance_with_controllable_pins(monkeypatch) + + # wait_for() does ``from seedsigner.controller import Controller`` at + # runtime. Provide a lightweight mock module so we don't pull in heavy + # dependencies (embit, etc.) that may not be installed in the test env. + mock_controller = SimpleNamespace( + screensaver_activation_ms=999_999_999, + is_screensaver_running=False, + ) + mock_controller_module = SimpleNamespace( + Controller=SimpleNamespace(get_instance=lambda: mock_controller), + ) + monkeypatch.setitem(sys.modules, "seedsigner.controller", mock_controller_module) + + # Pre-set the pin LOW and back-date _low_since_ms so wait_for() will + # detect the press immediately (no threading needed). + pin_states["KEY_PRESS"] = False + instance._low_since_ms["KEY_PRESS"] = int(time.time() * 1000) - 20 + + result = instance.wait_for(keys=["KEY_PRESS"]) + assert result == "KEY_PRESS" + + # wait_for() must have cleared _low_since_ms for the returned key + assert instance._low_since_ms["KEY_PRESS"] is None + + # Now simulate the user releasing the button (as happens during screen + # transition to the camera preview) + pin_states["KEY_PRESS"] = True + + # check_for_low must NOT report a phantom press + assert not instance.check_for_low(key="KEY_PRESS") From 9660a19e2f2cb7d4c4f3dc1d3b32e711d4465ee5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 31 Mar 2026 21:21:05 +0000 Subject: [PATCH 6/6] Fix check_for_low double-fire: clear _low_since_ms on debounced-low return When check_for_low() returned True for a held button (debounced low path), _low_since_ms[key] was left populated. On the subsequent release (pin high), the latch logic would see the stale timestamp and report a second press for the same physical interaction. Clear _low_since_ms[key] before returning True from the debounced-low path so the press is consumed and release cannot double-fire. Agent-Logs-Url: https://github.com/3rdIteration/seedsigner/sessions/f3fbb813-a06c-4a5a-a965-f99a9883327c Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com> --- src/seedsigner/hardware/buttons.py | 1 + tests/test_buttons_gpio_resolution.py | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/seedsigner/hardware/buttons.py b/src/seedsigner/hardware/buttons.py index 711539093..23e3c8b3d 100644 --- a/src/seedsigner/hardware/buttons.py +++ b/src/seedsigner/hardware/buttons.py @@ -414,6 +414,7 @@ def check_for_low(self, key=None, keys: List = None) -> bool: continue if cur_time - low_since < self.debounce_threshold_ms: continue + self._low_since_ms[key] = None self.update_last_input_time() return True else: diff --git a/tests/test_buttons_gpio_resolution.py b/tests/test_buttons_gpio_resolution.py index 502fef6a6..1ea0ad71e 100644 --- a/tests/test_buttons_gpio_resolution.py +++ b/tests/test_buttons_gpio_resolution.py @@ -360,6 +360,32 @@ def test_check_for_low_detects_held_button(monkeypatch): assert instance.check_for_low(key="KEY_PRESS") +def test_check_for_low_no_double_fire_on_release(monkeypatch): + """A button held long enough to return True on the debounced-low path + must NOT fire again when it is subsequently released (pin goes high). + This is the double-fire bug described in the issue.""" + instance, pin_states = _make_instance_with_controllable_pins(monkeypatch) + + # First call: pin goes low → records timestamp, returns False + pin_states["KEY_PRESS"] = False + assert not instance.check_for_low(key="KEY_PRESS") + + # Simulate time passing beyond debounce threshold + instance._low_since_ms["KEY_PRESS"] = int(time.time() * 1000) - 20 + + # Second call: pin still low, debounce met → returns True (first fire) + assert instance.check_for_low(key="KEY_PRESS") + + # _low_since_ms must be cleared after the debounced-low return + assert instance._low_since_ms["KEY_PRESS"] is None + + # Button is released + pin_states["KEY_PRESS"] = True + + # Must NOT return True again – the press was already consumed + assert not instance.check_for_low(key="KEY_PRESS") + + def test_check_for_low_detects_released_button(monkeypatch): """A button pressed and released between two slow polling calls is detected (the latching fix for issue #342)."""