Feature/custom_button #223

2025-12-16T18:07:22+08:00

fbuirey commented

2025-12-16 18:07:22 +08:00

(Migrated from github.com)

Voici une proposition de descriptif de Pull Request en anglais, rédigée dans un style attendu pour un projet open-source structuré comme pico-keys / pico-fido, en restant factuel, non intrusif et conforme à l’architecture existante.

Tu peux la copier telle quelle dans ta PR.

Summary

This PR improves button handling on Pico platforms by adding clean short-press and long-press detection while preserving the existing architecture and platform abstractions.

Previously, when using a custom GPIO button (CUST_BUTTON_PIN) instead of the BOOTSEL button, only long presses were reliably detected, and short presses could be miscounted or missed. This change ensures consistent and correct behavior between BOOTSEL and custom buttons.

This is a bug fix and behavioral improvement, not a new API.

Details / Impact

What changed
- picok_board_button_read() remains a pure input function (no callbacks, no policy).
- Short-press vs long-press detection is handled centrally in core0_loop(), using existing timing logic.
- Avoids double counting (short+long or multiple shorts).
- Maintains identical behavior across:
  - BOOTSEL button
  - Custom GPIO button (CUST_BUTTON_PIN)
  - Platform variants (PICO / ESP / emulation)
Hardware / board(s) tested
- Raspberry Pi Pico / RP2040
- RP2350-based board with custom button on GPIO 2
Firmware / base version
- pico-keys-sdk (current main branch at time of submission)
Security impact
- None
- No changes to PIN handling, key storage, attestation, or crypto paths
Behavior changes
- Short press (< threshold) and long press (≥ threshold) are now reliably distinguished
- No new commands or APIs
- No change in defaults or configuration options

Testing

Manual testing
1. Build firmware with default BOOTSEL button configuration
2. Verify:
  - Short press → single short-press event
  - Long press → single long-press event
3. Build firmware with CUST_BUTTON_PIN defined (GPIO 2)
4. Repeat the same validation steps
Expected result
- Short press triggers exactly one short-press action
- Long press triggers exactly one long-press action
- No duplicate or spurious events
Actual result
- Behavior matches expectations on all tested configurations
Logs / traces
- No additional logs added
- No sensitive data involved

Licensing confirmation (required)

By checking the box below, you confirm ALL of the following:

You are the author of this contribution, or you have the right to contribute it.
You have read CONTRIBUTING.md.
You agree that this contribution may be merged, used, modified, and redistributed:
- under the AGPLv3 Community Edition, and
- under any proprietary / commercial / Enterprise editions of this project,
  now or in the future.
You understand that submitting this PR does not create any support obligation,
SLA, or guarantee of merge.

I confirm the above licensing terms:

[ x] Yes, I agree

Anything else?

This PR intentionally avoids introducing platform-specific logic or callbacks in picok_board_button_read() to keep the hardware abstraction layer minimal and consistent.
Future work could include configurable press thresholds or multi-press patterns, but those are out of scope for this change.

Voici une **proposition de descriptif de Pull Request en anglais**, rédigée dans un style attendu pour un projet open-source structuré comme *pico-keys / pico-fido*, en restant factuel, non intrusif et conforme à l’architecture existante. Tu peux la copier telle quelle dans ta PR. --- ## Summary This PR improves button handling on Pico platforms by adding **clean short-press and long-press detection** while preserving the existing architecture and platform abstractions. Previously, when using a custom GPIO button (`CUST_BUTTON_PIN`) instead of the BOOTSEL button, only long presses were reliably detected, and short presses could be miscounted or missed. This change ensures consistent and correct behavior between BOOTSEL and custom buttons. This is a **bug fix and behavioral improvement**, not a new API. --- ## Details / Impact * **What changed** * `picok_board_button_read()` remains a *pure input function* (no callbacks, no policy). * Short-press vs long-press detection is handled centrally in `core0_loop()`, using existing timing logic. * Avoids double counting (short+long or multiple shorts). * Maintains identical behavior across: * BOOTSEL button * Custom GPIO button (`CUST_BUTTON_PIN`) * Platform variants (PICO / ESP / emulation) * **Hardware / board(s) tested** * Raspberry Pi Pico / RP2040 * RP2350-based board with custom button on GPIO 2 * **Firmware / base version** * pico-keys-sdk (current `main` branch at time of submission) * **Security impact** * None * No changes to PIN handling, key storage, attestation, or crypto paths * **Behavior changes** * Short press (< threshold) and long press (≥ threshold) are now reliably distinguished * No new commands or APIs * No change in defaults or configuration options ## Testing * **Manual testing** 1. Build firmware with default BOOTSEL button configuration 2. Verify: * Short press → single short-press event * Long press → single long-press event 3. Build firmware with `CUST_BUTTON_PIN` defined (GPIO 2) 4. Repeat the same validation steps * **Expected result** * Short press triggers exactly one short-press action * Long press triggers exactly one long-press action * No duplicate or spurious events * **Actual result** * Behavior matches expectations on all tested configurations * **Logs / traces** * No additional logs added * No sensitive data involved ## Licensing confirmation (required) By checking the box below, you confirm ALL of the following: * You are the author of this contribution, or you have the right to contribute it. * You have read `CONTRIBUTING.md`. * You agree that this contribution may be merged, used, modified, and redistributed: * under the AGPLv3 Community Edition, **and** * under any proprietary / commercial / Enterprise editions of this project, now or in the future. * You understand that submitting this PR does not create any support obligation, SLA, or guarantee of merge. **I confirm the above licensing terms:** * [ x] Yes, I agree ## Anything else? This PR intentionally avoids introducing platform-specific logic or callbacks in `picok_board_button_read()` to keep the hardware abstraction layer minimal and consistent. Future work could include configurable press thresholds or multi-press patterns, but those are out of scope for this change.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dearsky/pico-fido#223