Fix check_for_low double-fire on button release#343
Conversation
…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
…ing (#341) * 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>
Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
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>
There was a problem hiding this comment.
Pull request overview
Fixes a GPIO button-state leakage bug that could trigger an immediate “phantom” capture when transitioning into the camera workflow, and expands display support/handling (ST7735 + display switching) with supporting docs and tests.
Changes:
- Consume
_low_since_ms[key]inHardwareButtons.wait_for()and add latching logic tocheck_for_low()to detect short presses between polling intervals; add unit tests covering held/released/noise/phantom scenarios. - Add a new ST7735 (128×128) display driver, wire it into display configuration/settings and the display-driver factory, and add driver-focused tests.
- Add a home-screen very-long-press shortcut to switch display configs, plus renderer/keyboard/QR adjustments for dynamic canvas sizing and updated documentation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_io_config_profiles.py | Adds test coverage for ST7735 driver construction, SPI mode/flags, lazy init, timing, and EINVAL fallback. |
| tests/test_buttons_gpio_resolution.py | Adds latching + phantom-press regression tests for check_for_low() / wait_for(). |
| src/seedsigner/views/view.py | Handles the new return code to re-render Main Menu after display switching. |
| src/seedsigner/models/settings_definition.py | Adds st7735_128x128 and desktop_128x128 display configuration constants/options. |
| src/seedsigner/hardware/displays/display_driver.py | Adds st7735 display type support and enforces 128×128 resolution. |
| src/seedsigner/hardware/displays/ST7735.py | Introduces the ST7735S 128×128 SPI display driver implementation. |
| src/seedsigner/hardware/buttons.py | Clears _low_since_ms on wait_for() returns and adds latching behavior to check_for_low(). |
| src/seedsigner/gui/screens/screen.py | Adds RET_CODE__DISPLAY_TOGGLE, updates QR rendering sizing, and implements Main Menu long-press display switching. |
| src/seedsigner/gui/renderer.py | Adds ST7735 canvas-vs-display resize path and resizes outgoing frames when needed. |
| src/seedsigner/gui/keyboard.py | Makes default keyboard rect derive from the active renderer canvas dimensions. |
| docs/io_config.md | Documents shared pinout and the config value for the 1.44" ST7735 HAT. |
| docs/hardware_platform_support.md | Documents supported displays and the very-long-press display switching shortcut. |
| 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) |
There was a problem hiding this comment.
QR images are now generated using (canvas_width, canvas_height). On non-square canvases (e.g. ILI9341 where canvas is 240x320), this will stretch the QR code via qrencode/PIL resize, which can reduce scan reliability. Consider generating a square QR image using a single size (e.g. min(canvas_width, canvas_height)) and centering/padding it instead of resizing to a rectangle.
| 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) |
There was a problem hiding this comment.
Same as above: next_part_image() is now called with (canvas_width, canvas_height), which will produce a non-square, stretched QR on portrait canvases. Use a square size (e.g. min dimension) and then composite onto the canvas to preserve module aspect ratio.
| image = self.qr_encoder.next_part_image(self.renderer.canvas_width, self.renderer.canvas_height, border=2, background_color=hex_color) | |
| canvas_width = self.renderer.canvas_width | |
| canvas_height = self.renderer.canvas_height | |
| qr_side = min(canvas_width, canvas_height) | |
| qr_image = self.qr_encoder.next_part_image( | |
| qr_side, | |
| qr_side, | |
| border=2, | |
| background_color=hex_color, | |
| ) | |
| # Center the square QR image on the full canvas to preserve aspect ratio | |
| if qr_side == canvas_width and qr_side == canvas_height: | |
| image = qr_image | |
| else: | |
| # Use the same background color as the QR image | |
| background = "#" + hex_color | |
| image = Image.new(qr_image.mode, (canvas_width, canvas_height), background) | |
| offset_x = (canvas_width - qr_side) // 2 | |
| offset_y = (canvas_height - qr_side) // 2 | |
| image.paste(qr_image, (offset_x, offset_y)) |
| # 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 |
There was a problem hiding this comment.
The PR title/description focus on the phantom camera button press fix, but this PR also adds a home-screen very-long-press shortcut for switching display drivers (and related display driver/settings/docs changes). Please update the PR description/title to reflect this added scope, or split into separate PRs to keep review risk isolated.
…eturn 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>
Description
check_for_low()double-fires: when a held button passes the debounce threshold, it returnsTruebut leaves_low_since_ms[key]populated. On subsequent release (pin high), the latch logic sees the stale timestamp and reports a second press for the same physical interaction. This causes the "create seed from camera" workflow to capture an image immediately on entry.Root cause: The debounced-low return path didn't consume
_low_since_ms[key], so the release-latch path (added to catch brief presses between slow polling calls) would re-fire.Fix: Clear
_low_since_ms[key]before returningTruefrom the debounced-low path:No screenshot — no UI changes; this is GPIO button debounce logic only.
This pull request is categorized as a:
Checklist
pytestand made sure all unit tests pass before sumbitting the PRIf you modified or added functionality/workflow, did you add new unit tests?
I have tested this PR on the following platforms/os:
📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.