Skip to content

feat(presence): add configurable GPIO presence pin#17

Merged
TheMaxMur merged 1 commit into
TheMaxMur:mainfrom
lpiob:feat/custom-presence-pin
Jun 26, 2026
Merged

feat(presence): add configurable GPIO presence pin#17
TheMaxMur merged 1 commit into
TheMaxMur:mainfrom
lpiob:feat/custom-presence-pin

Conversation

@lpiob

@lpiob lpiob commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What

Presence button can be now remapped to a custom GPIO pin at compile time.

Backwards compatibility (default mapping to BOOTSEL) was preserved.

How it was tested

  • nix develop -c ./scripts/check.sh passes locally
  • On hardware: board = TenStar RP2350, what was exercised = webauthn and resident ssh keys
  • Host-only change (CLI / docs / CI) — no device behavior touched

Tested on TenStar RP2350 with a quickly attached capacitive-touch button :
image

Checklist

  • Device behavior / image changed → config.device_release (bcdDevice) bumped by one (hex) in firmware/src/main.rs
  • Fixes something a downgrade would reopen
  • New or changed unsafe → justified in docs/unsafe.md
  • User-visible behavior → the matching guide under docs/ updated
  • N/A New files carry the SPDX header (AGPL-3.0-only)
  • Commit messages use the zone prefix style (fido:, piv:, rsk:, docs:, …)

@lpiob lpiob force-pushed the feat/custom-presence-pin branch from 3763814 to 9216bee Compare June 26, 2026 08:07
@lpiob lpiob force-pushed the feat/custom-presence-pin branch from 9216bee to fca95b9 Compare June 26, 2026 11:23
@TheMaxMur TheMaxMur merged commit 8beab71 into TheMaxMur:main Jun 26, 2026
10 checks passed
@Curious-r

Copy link
Copy Markdown
Contributor

I also implemented a feature similar to this PR a while ago. The underlying principles are similar, but there are some differences in the design. I would like to discuss this with @lpiob and @TheMaxMur to see if there is any room to optimize this feature. My branch is located at https://github.com/Curious-r/RS-Key/tree/touch-btn-config. Below is a summary of the similarities and differences between my implementation and this PR:

Both implementations:

  • Use AnyPin::steal() to obtain a type-erased GPIO pin at runtime
  • Fall back to BOOTSEL when no GPIO is configured
  • Handle the standard UserPresence trait across all applets

Key Differences

Aspect feat/custom-presence-pin (merged) touch-btn-config (my branch)
Configuration mechanism Compile-time env vars (PK_PRESENCE_IS_GPIO, PK_PRESENCE_PIN) read in build.rs Runtime phy record stored in flash, written via the rescue applet over CCID (or CTAPHID vendor APDU)
Polarity Fixed active-low (internal pull-up, is_low()) Configurable: up_driver enum (1 = active-low, 2 = active-high) via phy TLV tag 0x0F
GPIO LED pin selection Still uses the 30-arm gpio_pin! macro match Also uses AnyPin::steal(), eliminating the 30-arm match entirely
Pin conflict detection Compile-time panic! when PRESENCE_PIN == LED_PIN No explicit check — relies on phy record schema (single-pin field) and documentation
CLI tooling None (build.rs env only) rsk hw --up-driver {bootsel,gpio-low,gpio-high} --up-btn <N> via phy record
pins.rs module Not present New module documenting the AnyPin::steal() safety model
Struct design BootselPresence (struct wrapping internal Button enum) PresenceButton (flat enum: Bootsel(Peri<BOOTSEL>) / Gpio(Input, Polarity))
build.rs Reads PK_PRESENCE_* env vars Removed those env vars (no longer needed)
phy.rs TLV schema Unchanged Adds TAG_UP_DRIVER = 0x0F and up_driver: Option<u8> field

Detailed Discussion

1. Compile-time vs Runtime Configuration

The merged PR uses env vars set during the build, which requires a reflash to change the presence pin. This is simple and has zero runtime storage overhead. However, RS-Key already has the phy record mechanism for the LED backend (led_gpio, led_driver, led_order), and our extension makes the presence button consistently runtime-configurable through the same path.

The phy record approach adds about 20 bytes of Rust code (a new TLV tag + parse/serialize) and ~40 lines of Python CLI. In return, users can reassign the button pin without reflashing — useful for testing different board layouts or for field reconfigurations.

Could we support both? A reasonable middle ground: keep the compile-time env vars as a build default, but let an optional phy record override them at runtime. This gives the simplicity of env vars for known builds and the flexibility of runtime config for multi-board deployments.

2. Polarity

The merged PR hard-codes active-low with a pull-up, which covers the common button-to-GND wiring. Our implementation adds an up_driver TLV field supporting three modes:

  • 0 / absent → BOOTSEL (no GPIO)
  • 1 → active-low (standard button to GND, Pull::Up)
  • 2 → active-high (capacitive touch modules, active-high signals)

This is a small extension that adds no meaningful complexity but covers more hardware.

3. GPIO LED Pin Selection

While working on the presence button, we noticed the GPIO LED backend had the same pattern: a 30-arm match block selecting concrete p.PIN_X peripherals. Since Output::new accepts impl Pin, we replaced it with:

let led_pin = unsafe { AnyPin::steal(led_gpio) };
let led = Output::new(led_pin, Level::Low);

This eliminates 30 lines of boilerplate match arms. The safety argument is the same as for the presence button — the GPIO LED and WS2812 backends are mutually exclusive at runtime (different led_driver match arms).

We could upstream this as a separate, trivial cleanup — it's not coupled to the presence button feature.

4. Struct Design

Our commit also renames BootselPresencePresenceButton and flattens the internal structure from a struct-with-enum-field to a top-level enum:

// Upstream
pub struct BootselPresence {
    button: Button,
}
enum Button { Bootsel(Peri<BOOTSEL>), Gpio(Input) }

// Our branch
pub enum PresenceButton {
    Bootsel(Peri<'static, BOOTSEL>),
    Gpio(Input<'static>, Polarity),
}

The enum approach is slightly more idiomatic for a sum type and makes the polarity field naturally part of the Gpio variant. Happy to adopt either — this is cosmetic.

Open Questions

  1. Phy record vs env vars: Should we support both (env as default, phy as override), or pick one? If both, which takes precedence?
  2. up_driver TLV tag: Is there appetite for adding tag 0x0F to the phy record schema? It's a single byte, and the mechanism is already extensible.
  3. Naming: BootselPresence vs PresenceButton? The current name documents which button is default; the proposed name is shorter and reflects that the type can be either.
  4. GPIO LED cleanup: Worth submitting separately?

I'd love to hear thoughts.

TheMaxMur added a commit that referenced this pull request Jul 3, 2026
Brings @lpiob's PRESENCE_PIN feature (a compile-time configurable GPIO presence
button, with the BOOTSEL default preserved) onto the trusted-display branch.

Conflict resolution:
- firmware/src/presence.rs: combine main's Button{Bootsel,Gpio} enum +
  new_bootsel/new_gpio/pressed() with develop's display gating
  (#[cfg(not(feature = "display"))], the TouchPresence alias), the pub(crate)
  statics, the runtime PRESENCE_TIMEOUT_MS, and the Confirm-carrying
  UserPresence::request signature.
- firmware/src/main.rs: the non-display presence path now picks BOOTSEL vs GPIO;
  the display path keeps TouchPresence. bcdDevice resolved to 0x0792 (after
  develop's 0x0791; main's parallel 0x0782 is a separate lineage).
- nix/firmware.nix: keep both the ledKind and presencePin knobs.

Integration (the minor follow-ons):
- A display build takes presence from the touchscreen, so PRESENCE_PIN is now
  rejected at compile time (a const assert) rather than being silently ignored.
- Add the CHANGELOG [Unreleased] entry PR #17 omitted (bcdDevice 0x0791 -> 0x0792).

Both flavors build; `nix develop -c ./scripts/check.sh` is green and the
PRESENCE_PIN GPIO build + the display-build rejection are verified.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@TheMaxMur

Copy link
Copy Markdown
Owner

@Curious-r pls check on v0.3.0

@Curious-r

Copy link
Copy Markdown
Contributor

@TheMaxMur Okay, I’m looking at it right now.

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