Skip to content

Fix check_for_low double-fire on button release#343

Merged
3rdIteration merged 6 commits intodevfrom
copilot/fix-issue-342
Apr 7, 2026
Merged

Fix check_for_low double-fire on button release#343
3rdIteration merged 6 commits intodevfrom
copilot/fix-issue-342

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 23, 2026

Description

check_for_low() double-fires: when a held button passes the debounce threshold, it returns True but 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 returning True from the debounced-low path:

# Before (double-fires)
self.update_last_input_time()
return True

# After (consumes the press)
self._low_since_ms[key] = None
self.update_last_input_time()
return True

No screenshot — no UI changes; this is GPIO button debounce logic only.

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

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?

  • No, I'm a fool
  • Yes
  • N/A

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.

…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
Copilot AI changed the title [WIP] Fix issue related to seeds and voting Fix buttons unresponsive to short clicks during camera workflows Mar 23, 2026
Copilot AI requested a review from 3rdIteration March 23, 2026 17:41
Copilot AI and others added 2 commits March 30, 2026 02:17
…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>
Copilot AI changed the title Fix buttons unresponsive to short clicks during camera workflows Fix phantom button press when entering camera workflow Mar 30, 2026
@3rdIteration 3rdIteration marked this pull request as ready for review March 30, 2026 17:21
Copilot AI review requested due to automatic review settings March 30, 2026 17:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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] in HardwareButtons.wait_for() and add latching logic to check_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.

Comment on lines 869 to +870
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)
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
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)
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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))

Copilot uses AI. Check for mistakes.
Comment on lines +1372 to +1374
# 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
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
…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>
Copilot AI changed the title Fix phantom button press when entering camera workflow Fix check_for_low double-fire on button release Mar 31, 2026
@3rdIteration 3rdIteration merged commit b8f5b34 into dev Apr 7, 2026
9 checks passed
@3rdIteration 3rdIteration deleted the copilot/fix-issue-342 branch April 7, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants