feat: add bumble BLE transport with pairing#107
Open
JPHutchins wants to merge 8 commits into
Open
Conversation
Collaborator
Author
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new optional BLE transport implementation backed by Google’s bumble stack (for external HCI controllers) along with supporting keystore, pairing, scanning helpers, and a minimal smpbumble CLI entrypoint.
Changes:
- Add
smpclient[bumble]optional dependency set and register thesmpbumbleCLI script. - Implement
SMPBumbleTransportwith a small connection state machine, proactive bonded-encryption, plus one-shotpair_device()helper. - Add bumble-specific helpers for scanning, pairing delegates/result types, and keystore strategy resolution.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Adds bumble optional extra + smpbumble script; tightens dependency version ranges. |
src/smpclient/transport/bumble/__init__.py |
Implements SMPBumbleTransport, bumble device context manager, and pair_device() helper. |
src/smpclient/transport/bumble/__main__.py |
Adds smpbumble CLI commands: scan, pair, echo. |
src/smpclient/transport/bumble/keystore.py |
Introduces keystore strategy sum type and resolver to bumble KeyStore. |
src/smpclient/transport/bumble/pairing.py |
Adds pairing delegates and a PairingResult sum type for outcomes. |
src/smpclient/transport/bumble/scan.py |
Adds async scan helper with name matching and SMP-service marker detection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JPHutchins
commented
May 22, 2026
Collaborator
Author
JPHutchins
left a comment
There was a problem hiding this comment.
Seems a lot of repetition, unnecessary variable aliasing, not enough use of Final.
Collaborator
Author
|
PR description is out of date. |
Adds an optional `bumble` extra (`smpclient[bumble]`) providing SMPBumbleTransport — a `SMPTransport` backed by Google's bumble Bluetooth stack driving an external HCI USB controller. Useful when the OS BLE stack is unavailable or for reproducible cross-platform behavior via the same HCI dongle. - transport/bumble/__init__.py — SMPBumbleTransport with a Disconnected | Connecting | Connected sum-type state machine, a bumble_device() async context manager, pair_device() one-shot bonding, and a pair() method on the transport itself - transport/bumble/keystore.py — KeystoreStrategy sum type (Tempfile | Local | Custom | InMemory) - transport/bumble/pairing.py — NoInputNoOutput / KeyboardOnly / DisplayOnly delegates and a PairingResult sum type - transport/bumble/scan.py — async BLE scan with name filter, eager mode, and SMP-service marker - transport/bumble/__main__.py — minimal CLI registered as smpbumble (scan, pair, echo) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…T_FOUND, ExistingCustom - assert_never imports moved to typing_extensions (Python 3.10 compat; caught by Copilot) - SMP_SERVICE_UUID + SMP_CHARACTERISTIC_UUID centralized in smpclient.transport; re-exported through ble.py and bumble/__init__.py - pair_on_connect: PairingDelegate | None added to SMPBumbleTransport constructor; connect() pairs after LE-connect and before GATT discovery when no bond exists - module-level pair() function factored out and shared by SMPBumbleTransport.pair(), pair_device(), and the pair_on_connect path - SMPBumbleTransport.scan() static method using bumble_device() ctx mgr - typed NamedTuple args (_ScanArgs / _PairArgs / _EchoArgs) for CLI dispatch instead of bare argparse.Namespace - PairingFailureReason.NOT_FOUND for scan-by-name misses (was USER_REJECTED, which was misleading) - ExistingCustom(path) keystore variant alongside Custom(path); Tempfile and Local filenames validated as bare names (reject path separators) - _prompt_pin enforces exactly 6 digits - mtu raises in non-Connected state instead of returning 0 - _echo uses assert_never on the impossible-after-success/error branch - ATT_WRITE_OVERHEAD named constant replaces the bare `- 3` - _encrypt_using_bond consolidates _encrypt_if_bonded + the inline on-security-request logic - scan.py: _scanning context manager extracts the 3-level nested try/finally - walrus operators, removed self._state aliasing, trimmed multi-line "essay" comments to one-liners - pyproject: norecursedirs += ".claude" and ruff extend-exclude += ".claude" so other agents' worktrees don't break lint/test Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- ConnectedBorrowed sum-type state for caller-owned LE links - ConnectedProtocol structural type shared by Connected/ConnectedBorrowed — send/receive/mtu work on either without procedural sentinel checks - use_connection(connection, *, peer=None) lets SMP share a bumble link with non-SMP traffic; disconnect() only unsubscribes (caller owns the rest of the lifecycle) - transport-extras CI matrix gets a bumble-only row and the "all" row now expects bumble to import as well - tests/test_smp_bumble_transport.py: 39 tests covering state machine, send/receive notify queue, pair delegates + result sum type, keystore strategies (Tempfile/Local/Custom/ExistingCustom/InMemory + filename validation), GATT discovery helpers, module-level pair(), pair_device() end-to-end with mocked bumble stack, bumble_device() context manager, scan() helper, and CLI dispatch (prompt_pin / scan / pair / echo handlers + argparse routing). Coverage now 91% (was 80%). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… transitivity - macOS resolves /tmp via /var → /private/var symlink; tests now compare Path.resolve() so the keystore filename assertions match regardless. - The bumble extra depends on pyserial-asyncio, which transitively installs pyserial — so `from smpclient.transport.serial import SMPSerialTransport` succeeds in a bumble-only install too. Updating expect-serial=pass for the bumble-only matrix row to reflect the actual install surface. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot: - ScanMode sum type (ScanAll | ScanForName) replaces eager:bool / name:str|None on scan() — invalid states (eager without name) are unrepresentable at the type level. ScanForName has an opt-out eager:bool = True flag so callers can enumerate multiple peers sharing a name. - _check_bare_filename uses PurePath; rejects absolute paths, separators, "./..", and Windows drive prefixes like "C:foo". - ExistingCustom keystore strategy validates path.is_file() so a directory or symlink-to-non-file errors clearly at resolve time. - Keystore namespace is now consistently derived from host_address in both connect() and bumble_device(); _standalone_keystore agrees, so bonded_devices() / clear_bond() see the same bonds connect() stored. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
9a7073d to
a7698af
Compare
…irmware `smpclient[hci_firmware]` pulls in `zephyr-4.4.0-hci`, the umbrella that depends on every published `hci_usb` build (3× nrf52840dk + 2× nrf5340dk). `smpclient.transport.bumble.firmware` re-exports the umbrella's typed `firmware: Firmware` NamedTuple so callers can do `firmware.nrf52840dk_default.HEX_PATH` with full IDE autocomplete, or raise a friendly error pointing at the extra when it isn't installed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Move firmware re-export out from under `transport/bumble/` to `smpclient.transport.firmware.hci` — firmware is not bumble-specific and the prior path forced `[hci_firmware]`-only installs to import the bumble package (which fails without the `[bumble]` extra). * `transport.firmware.hci`: narrow `except ModuleNotFoundError` so transitive import errors aren't masked. Drop pip-specific install hint. * `_teardown_borrowed`: actually remove the `EVENT_DISCONNECTION` listener registered by `use_connection()` so the caller's Connection isn't left holding references to the transport after teardown. * Docstring fixes: module docstring + `use_connection()` mention `ConnectedBorrowed`; `scan()` docstring correctly reports `ScanAll()` as the default mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JPHutchins
commented
May 27, 2026
Collaborator
Author
JPHutchins
left a comment
There was a problem hiding this comment.
Review round focused on cleaning up the new transport's __init__.py
* Move generic helpers out of `transport/bumble/__init__.py`:
- `bumble_device()` + host/HCI constants → new `bumble/device.py`
- `pair()`, `pair_device()`, `encrypt_using_bond()` + pairing constants →
`bumble/pairing.py` (was just delegates + result types)
* `SMPBumbleTransport`: split `clear_bond(address)` and `clear_bonds()`
(no `None`-coded meaning), inline `bonds` local in `bonded_devices`,
drop the impl-detail line from its docstring.
* `_notifications` / `_disconnected_event`: `Final` annotations.
* `_next_chunk`: walrus-style `match (chunk := await ...)`.
* `_on_security_request`: pattern-match on state and log at the right level
(info for borrowed, warn for Disconnected/Connecting, debug for Connected).
* Add `borrowed_connection()` async context manager wrapping
`use_connection()` + `disconnect()`.
* Trim narrative comments where the code is the explanation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional
bumbleextra (smpclient[bumble]) providingSMPBumbleTransport— a BLESMPTransportbacked by Google's bumble stack driving an external HCI USB controller (e.g. an nRF52840 DK running the Zephyrhci_usbsample). Alternative to the bleak-backedSMPBLETransportfor when the OS BLE stack is unavailable or reproducible cross-platform behavior is desired via the same HCI dongle everywhere.Validated end-to-end against a real device: scan → pair (PIN flow) → reconnect with proactive
connection.encrypt()from stored LTK → SMP Echo round-trip.What's in the PR
Transport core (
src/smpclient/transport/bumble/__init__.py)SMPBumbleTransportwith aDisconnected | Connecting | Connected | ConnectedBorrowedsum-type state machine; per-steptry/exceptin_teardown()is load-bearing (skipping a bumble cleanup step can hang process exit).ConnectedProtocol(Protocol)— structural type shared byConnected(owned) andConnectedBorrowed(caller-owned LE link).send/receive/mtuoperate on it via a single_require_connected()guard; no procedural None-sentinel checks.pair_on_connect: PairingDelegate | Noneconstructor arg —connect()pairs after LE-connect and before GATT discovery when no bond exists; when a bond is present, proactive_encrypt_using_bondruns instead.use_connection(connection, *, peer=None)— borrow an existing bumbleConnectionfor SMP traffic without owning its lifecycle. Caller manages connect/disconnect/encryption.bumble_device()async context manager owns HCI transport + device lifecycle.pair_device()one-shot bonding (connect → pair → disconnect, no GATT).pair()function shared bypair_device(),SMPBumbleTransport.pair(), and thepair_on_connectpath.SMPBumbleTransport.scan()static method usingbumble_device().Keystore (
src/smpclient/transport/bumble/keystore.py)KeystoreStrategy = Tempfile | Local | Custom | ExistingCustom | InMemorysum type.Custom(path)auto-creates parent dirs;ExistingCustom(path)errors if the file is missing.Tempfile/Localfilenames validated as bare names (reject path separators).Pairing (
src/smpclient/transport/bumble/pairing.py)NoInputNoOutput/KeyboardOnly(pin_cb)/DisplayOnly(display_cb)delegates.PairingResult = PairingSucceeded | PairingAlreadyBonded | PairingTimedOut | PairingFailed.PairingFailureReasonenum incl.NOT_FOUNDfor scan misses.Scan (
src/smpclient/transport/bumble/scan.py)_scanning()async context manager.CLI (
src/smpclient/transport/bumble/__main__.py)smpbumbleentry point:scan,pair [--force],echo. Alsopython -m smpclient.transport.bumble._ScanArgs/_PairArgs/_EchoArgsNamedTuples for dispatch (no bareargparse.Namespacehand-off).Tests (
tests/test_smp_bumble_transport.py)PairingResultsum type, keystore strategies (Tempfile/Local/Custom/ExistingCustom/InMemory + filename validation), GATT discovery helpers, module-levelpair(),pair_device()end-to-end with mocked bumble stack,bumble_device()context manager, scan helper, and CLI dispatch (_prompt_pin/_scan/_pair/_echohandlers + argparse routing). Bumble module at 91%+ coverage.CI (
.github/workflows/test.yaml)bumble-onlyrow intransport-extrasmatrix. Each row asserts the bumble import passes/fails as expected for that extras combination.Cross-cutting
SMP_SERVICE_UUIDandSMP_CHARACTERISTIC_UUIDcentralized insmpclient.transport; re-exported byble.pyandbumble/__init__.py.assert_neverimported fromtyping_extensionseverywhere (Python 3.10 compat).ATT_WRITE_OVERHEADnamed constant replaces magic- 3in MTU math.Test plan
uv run task all— lint, typecheck, full test suite (254 passed, 14 skipped)uv run task matrix— same across Python 3.10–3.14uv run task coverage— total coverage ≥ 91% gateuv run smpbumble scan— lists all advertising devices, marks SMP onesuv run smpbumble pair <addr> [--force]— pairs a device, optionally wiping local bond firstuv run smpbumble echo <addr> <msg>— connects, auto-encrypts from stored LTK, SMP Echo round-tripFollow-up (not blocking this PR)
intercreate/zephyr-hciselectable via per-board smpclient extras +all_firmwareaggregate (separate PR coordinating with the build farm side).FYI @raykamp @raykamp-tht
🤖 Generated with Claude Code