From efa7b9a4f8cab092f8deeb74573f078d196b0ba2 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 15:02:37 -0700 Subject: [PATCH 01/32] test(corebluetooth): Remove unused notification fixture accessor Co-Authored-By: Claude Opus 5.5 (1M context) --- src/corebluetooth/internal/tests.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/corebluetooth/internal/tests.rs b/src/corebluetooth/internal/tests.rs index 62ab38d0..ee13c513 100644 --- a/src/corebluetooth/internal/tests.rs +++ b/src/corebluetooth/internal/tests.rs @@ -207,10 +207,6 @@ impl NotificationFixture { } } - fn characteristic(&self, index: usize) -> &TestCharacteristic { - &self.characteristics[index].characteristic - } - fn set_notify_calls(&self) -> Vec { self.peripheral.set_notify_calls() } From 85987cbd39a7bd844f6601a191939e599d885057 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 15:05:24 -0700 Subject: [PATCH 02/32] doc: Add Zephyr hardware regression test expansion plan Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 220 +++++++++++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 docs/plan-zephyr-regression-tests.md diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md new file mode 100644 index 00000000..3b7fc46e --- /dev/null +++ b/docs/plan-zephyr-regression-tests.md @@ -0,0 +1,220 @@ +# Plan: Zephyr Hardware Regression Test Expansion + +Written: 2026-09-26 +Branch: `bug-finding` (these tests are regression coverage for the fixes already on this branch) + +## Context + +The hardware integration suite (`tests/`, driven against the Zephyr test peripheral on an nRF52840 DK) has not grown since 2026-08-29. The last firmware change was GAP appearance (`39fddc1`); the last real test additions were retrieval, adapter address, and macOS `clear_peripherals`. Since then 0.13.0 through 0.13.3 shipped ~40 fix commits with no new integration coverage. Most of those bugs lived in concurrency, error-status, and teardown paths that the current connect → single-op → disconnect tests never reach. + +The original design (`docs/design-plans/2026-02-22-ble-integration-test-system.md:83,125`) also specified service data advertising and a working `CMD_CHANGE_ADVERTISEMENTS` (0x04) rotation. Neither was built. + +The Bumble virtual peripheral has never been run. It is already non-functional (it packs 47 bytes of AD structures into a 31-byte legacy advertising packet and omits flags) and has drifted from the Zephyr behaviour. It is retired in Step 2 and will be revisited once there is a concrete plan to make it useful (see "Future Work"). + +### Fix → coverage map + +| Issue | Backend | Step | Firmware change | +|---|---|---|---| +| #488 concurrent connect/disconnect/discover waiters | CoreBluetooth | 3 | No | +| #481 deadlock on concurrent ops in one service | WinRT | 3 | No | +| #489 in-flight ops across rediscovery | CoreBluetooth | 3 | No | +| #484 stale services after dropped link; connect-when-connected | WinRT | 4 | No | +| #326 duplicate handlers on re-subscribe | WinRT | 4 | No | +| `DeviceConnected` / `DeviceDisconnected` never asserted | All | 4 | No | +| #490 disconnect mid-operation wedges queue / hides `DeviceDisconnected` | Android | 5 | No | +| `add_peripheral()` and retrieval by identifier untested | WinRT, Android, CB, BlueZ | 6 | No | +| #489 `clear_peripherals()` disconnects connected peripheral | CoreBluetooth | 7 | No | +| #492 failed reads/writes reported as success | Android | 8 | Yes | +| #471 refused subscribe never resolves | CoreBluetooth | 8 | Yes | +| #464 write-without-response order under backpressure | CoreBluetooth | 9 | Yes | +| #487 panic on characteristics for included services | CoreBluetooth | 10 | Yes | +| #483 128-bit service data UUID byte order | WinRT | 11 | Yes | +| MTU-sized notifications (firmware caps payload at 20 bytes) | All | 12 | Yes | +| #476 Windows radio handler tests never run | WinRT | 13 | No | + +Not covered on hardware, with reasons: #485 (needs BlueZ < 5.62), #491 and #486 (GATT discovery failure cannot be forced from Zephyr; empty service set needs separate firmware), #482 (malformed adverts risk other stacks dropping the packet; covered by winrtble unit tests), FnAdapter race (covered by `src/droidplug/jni_utils/ops/test.rs` stress tests), `DeviceServicesModified` (Zephyr does not send Service Changed to unbonded clients; needs bonding support first). + +## Execution Loop + +Every step runs this loop. Do not start the next step until the current step is committed. + +1. **Implement** with a `sonnet` subagent (`ed3d-plan-and-execute:task-implementor-fast`, `model: sonnet`). Use `haiku` (`ed3d-basic-agents:haiku-general-purpose`) for doc-only or mechanical steps (Steps 2 and 14 doc edits). The implementor receives the step text from this file verbatim plus the conventions below. +2. **Verify** locally before review: + - `cargo fmt --check` + - `cargo build --tests` + - `cargo clippy --tests` + - Firmware steps: `uv run west build -b nrf52840dk/nrf52840 --pristine` from `test-peripheral/zephyr/` + - Hardware run when the board is attached: `uv run west flash --runner nrfjprog`, then `./scripts/run-integration-tests.sh ` plus the full suite for firmware steps. If the board is not attached, stop and hand the hardware run to the user before committing. +3. **Review** with an `opus` subagent (`ed3d-plan-and-execute:code-reviewer`, `model: opus`). Reviewer checks the diff against this step's text, the invariants in `tests/AGENTS.md`, and platform gating correctness. +4. **Fix** any review issues with a `sonnet` subagent (`ed3d-plan-and-execute:task-bug-fixer`, `model: sonnet`), then re-review with `opus`. Repeat until the reviewer reports zero issues. +5. **Commit** once per step with the commit message given in the step, ending with the session's Co-Authored-By trailer. + +### Conventions for every test step + +- Test logic goes in `tests/common/test_cases.rs`; each new test gets a thin `tests/test_.rs` wrapper with exactly one `#[tokio::test]` `#[ignore = "requires BLE test peripheral"]` function. +- Unless the test is platform-gated off Android, add a `jni_test!` export in `tests/android/rust/src/lib.rs`, an `external fun` in `tests/android/src/main/kotlin/.../NativeTests.kt`, and a `@Test` in `tests/android/src/androidTest/kotlin/.../BleIntegrationTest.kt`. +- Wrap every concurrent or error-path operation in `tokio::time::timeout` so a regression fails instead of hanging. Use 10-15 second bounds. +- Tests that mutate peripheral state call `peripheral_finder::reset_peripheral()` after connecting. +- New UUIDs and opcodes go in `tests/common/gatt_uuids.rs` and `test-peripheral/zephyr/src/gatt_profile.h` together. +- Platform semantics that differ legitimately (e.g. BlueZ rejecting a concurrent `Connect` with `InProgress`) are gated with `cfg` and a one-line reason. Do not weaken an assertion to make a platform pass without flagging it to the user; an unexpected failure may be a real backend bug. +- New characteristics are appended to the end of existing services so the hardcoded `notify_svc.attrs[]` indices in `control_service.c` stay valid. Update the index table comment when `notify_svc` grows. + +## Steps + +### Step 1: Pin the Zephyr version + +**Goal:** Reproducible firmware builds. + +- Add `test-peripheral/zephyr/west.yml` (T2 star topology) pinning `zephyr` to `v4.4.0`, importing only the modules the two boards need (`hal_nordic`, `cmsis`/`cmsis_6`, `hal_espressif`, plus whatever the ESP32-S3 build requires). Before writing, have a research subagent confirm v4.4.0 is still the latest stable release and verify the allowlist names against the v4.4.0 `west.yml`. +- Update `test-peripheral/README.md` and `docs/zephyr-test-peripheral-debugging.md` with the `west init -l` / `west update` workflow. +- Verify the nRF52840 build succeeds against the pinned tree. + +**Commit:** `build(zephyr): Pin test peripheral firmware to Zephyr v4.4.0` + +### Step 2: Retire Bumble and fix stale test docs + +**Goal:** Stop documenting a second peripheral that does not work. + +- Delete `test-peripheral/bumble/`. +- Remove Bumble instructions from `README.md` (~line 203), `test-peripheral/README.md` (Option B and Bumble troubleshooting), `docs/android-integration-test-runbook.md` (Option A), and `tests/AGENTS.md:44-45`. +- Rename `test-peripheral/CLAUDE.md` → `test-peripheral/AGENTS.md` (matching `1800369`), rewrite it for a single Zephyr implementation, drop the "must behave identically" invariant, and update its freshness date. Note in it that Bumble was removed in this commit and can be restored from history. +- Fix root `AGENTS.md:31-32` to point at `tests/AGENTS.md` and `test-peripheral/AGENTS.md`. +- Fix `tests/AGENTS.md:53`: the scan timeout is 15 seconds (`peripheral_finder.rs:13`). +- Note in the `tests/AGENTS.md` Android invariant that tests backed by APIs Android does not support (e.g. `test_retrieve_connected_peripheral_by_service`, macOS-only tests) intentionally have no JNI export. +- Leave historical design plans under `docs/design-plans/` unchanged. + +**Commit:** `test: Retire unmaintained Bumble peripheral and fix stale test docs` + +### Step 3: Concurrency regression tests + +**Goal:** Cover #488, #481, #489 (rediscovery) with no firmware change. + +- `test_concurrent_connect_and_discover`: after `find_and_connect()` and `disconnect()`, run `join!(connect(), connect())`, then `join!(discover_services(), discover_services())`, then `join!(disconnect(), disconnect())`, each under a timeout. Every call must complete. On macOS and Windows every call must return `Ok`. If BlueZ or Android return an in-progress error for the second call, gate that with `cfg` and require at least one `Ok` plus the expected final `is_connected()` state. +- `test_concurrent_operations_same_service`: `join!(subscribe(NOTIFY_CHAR), subscribe(INDICATE_CHAR))` and `join!(read(STATIC_READ), read(COUNTER_READ))`, all under a timeout, all `Ok`, static value correct. +- `test_discover_services_during_read`: `join!(read(COUNTER_READ), discover_services())`; both resolve `Ok`, and a follow-up `read(STATIC_READ)` succeeds. + +**Commit:** `test: Add concurrent connect, discovery, and same-service operation tests` + +### Step 4: Reconnect, event, and re-subscribe tests + +**Goal:** Cover #484, #326, and connection events. + +- `test_reconnect_after_peripheral_triggered_disconnect`: trigger 0x03, wait for disconnection, call `connect()` without an explicit `disconnect()`, `discover_services()`, read `STATIC_READ` successfully. Then call `connect()` again while connected and assert it returns `Ok` within 2 seconds (the #484 fast path). +- Extend `test_connect_and_disconnect` and `test_peripheral_triggered_disconnect` to subscribe to `adapter.events()` before `find_and_connect()` and assert `DeviceConnected(id)` then `DeviceDisconnected(id)` for the test peripheral's id, each within a timeout. Ignore events for other ids. +- `test_resubscribe_does_not_duplicate_notifications`: subscribe to `NOTIFY_CHAR` twice, start notifications, collect for ~4 seconds, assert at least 3 notifications and no repeated counter byte. + +**Commit:** `test: Add reconnect, connection event, and re-subscribe regression tests` + +### Step 5: Disconnect during operations + +**Goal:** Cover #490. + +- `test_operations_across_peripheral_triggered_disconnect`: subscribe to adapter events, send 0x03, then loop issuing `read_descriptor(READ_ONLY_DESCRIPTOR)` and `read(COUNTER_READ)` (each under a 5 second timeout) until one returns `Err`. No operation may time out. Assert `DeviceDisconnected(id)` arrives. Assert one more read after disconnection returns `Err` promptly (`Error::NotConnected` on Android). Then reconnect, rediscover, and read `STATIC_READ` successfully to prove the command queue is not wedged. + +**Commit:** `test: Add disconnect-during-operation regression test` + +### Step 6: `add_peripheral` and retrieval by identifier + +**Goal:** Cover APIs with no integration coverage. + +- `test_add_peripheral_by_address`: discover the peripheral and record its id, clear local state appropriately, call `add_peripheral(&id)`, connect, discover, read `STATIC_READ`. Windows and Android must succeed; BlueZ and CoreBluetooth must return `Error::NotSupported("add_peripheral")` (assert the contract, gated per target). +- `test_retrieve_connected_peripheral_by_identifier`: connect, then `retrieve_peripherals` with `identifiers: Some(vec![id])`; the peripheral must be returned. Desktop only; no Android export (Android returns `NotSupported`, already covered by `test_retrieve_peripherals_not_supported`). + +**Commit:** `test: Cover add_peripheral and peripheral retrieval by identifier` + +### Step 7: macOS `clear_peripherals` while connected + +**Goal:** Cover the #489 `clear_peripherals` behaviour change. + +- `test_clear_peripherals_disconnects_connected_peripheral` (macOS only): `find_and_connect()`, subscribe to events, `clear_peripherals()`, assert `DeviceDisconnected(id)` within a timeout, then scan and assert the peripheral is rediscovered (it only re-advertises once the link is actually down). + +**Commit:** `test(corebluetooth): Cover clear_peripherals on a connected peripheral` + +### Step 8: GATT error and refused-CCC characteristics + +**Goal:** Cover #492 and #471. + +Firmware: +- `ERROR_CHAR` `0x00000207` appended to the Read/Write service, `READ | WRITE`, both handlers return `BT_GATT_ERR(0x80)` (application error; avoid authentication/encryption errors, which trigger pairing on macOS and Windows). +- `REFUSED_NOTIFY_CHAR` `0x00000304` appended to the Notification service, `NOTIFY`, with a managed CCC whose write callback rejects every enable with an ATT error. Verify the exact v4.4.0 API (`BT_GATT_CCC_MANAGED` / `struct bt_gatt_ccc_managed_user_data` `cfg_write`, or a newer write-callback CCC macro) against the pinned tree before writing; earlier research on this signature was unreliable. + +Tests: +- `test_gatt_error_status_is_reported`: `read(ERROR_CHAR)` and `write(ERROR_CHAR, WithResponse)` both return `Err` within a timeout; a follow-up `read(STATIC_READ)` succeeds. +- `test_refused_subscribe_returns_error`: `subscribe(REFUSED_NOTIFY_CHAR)` returns `Err` within a timeout; a follow-up `subscribe(NOTIFY_CHAR)` succeeds and receives a notification. + +**Commit:** `test(zephyr): Add GATT error and refused subscription characteristics` + +### Step 9: Write-without-response ordering + +**Goal:** Cover #464; the existing burst test cannot observe reordering. + +Firmware: +- In `write_without_resp`, keep the existing `rw_value` storage and additionally track `wwr_count`, `wwr_last_seq` (first byte of each write), and `wwr_out_of_order` (incremented when a sequence byte is not `last + 1`). Reset clears them. +- `WRITE_LOG_CHAR` `0x00000208` appended to the Read/Write service, `READ`, returning `[count_lo, count_hi, last_seq, out_of_order]`. + +Tests: +- Extend `test_write_without_response_burst`: after the 50 writes, poll `WRITE_LOG_CHAR` for up to 2 seconds until the count reaches 50, then assert count == 50 and out_of_order == 0. A dropped write on any platform is a finding to report, not something to loosen. + +**Commit:** `test(zephyr): Verify write-without-response delivery order` + +### Step 10: Included secondary service + +**Goal:** Cover #487. + +Firmware: +- Secondary service `0x00000005` (`BT_GATT_SECONDARY_SERVICE`) with `INCLUDED_CHAR` `0x00000501` (`READ`, fixed value `[0x05]`). +- Include it from the Descriptor service. Include declarations must immediately follow the service declaration, so this inserts at `descriptor_svc.attrs[1]`; nothing references `descriptor_svc` indices. Verify the v4.4.0 `BT_GATT_INCLUDE_SERVICE` argument type (it takes the included service's declaration attribute, not the service struct) against the pinned tree. + +Tests: +- `test_discovery_with_included_service`: `find_and_connect()` succeeds (discovery completes), all four primary services are present, and `read(STATIC_READ)` succeeds afterwards. Do not assert whether the secondary service appears in `services()`; that is backend-dependent. + +Note in the step's hardware run: Windows caches GATT databases even for unpaired devices. After flashing, remove the device in Windows Bluetooth settings (or clear the cache) if discovery returns the old layout. + +**Commit:** `test(zephyr): Add included secondary service to the test profile` + +### Step 11: Advertisement rotation with 128-bit service data + +**Goal:** Cover #483 and build the part of the original design that was deferred. + +Firmware: +- Implement `CMD_CHANGE_ADVERTISEMENTS` (0x04): set an `alt_adv_pending` flag. When advertising restarts after a disconnect with the flag set, use the alternate set: advertising data = flags + complete name; scan response = 128-bit service data (`BT_DATA_SVC_DATA128`: Control Service UUID little-endian + `[0x01]`) + manufacturer data. On the next `connected()` callback, clear the flag so the following disconnect restores the default set. `CMD_RESET_STATE` also clears it. +- Replace the "deferred" comment in `control_service.c` and update the opcode description in `test-peripheral/AGENTS.md`. + +Tests: +- `test_advertisement_service_data_128bit`: `find_and_connect()`, send 0x04, `disconnect()`, listen for `CentralEvent::ServiceDataAdvertisement` whose map contains `CONTROL_SERVICE` → `[0x01]` (byte-for-byte UUID match proves the byte order) within 15 seconds; also assert `properties().service_data` contains it. Add a `SERVICE_DATA_VALUE` constant to `gatt_uuids.rs`. + +**Commit:** `test(zephyr): Implement advertisement rotation with 128-bit service data` + +### Step 12: MTU-sized notification payloads + +**Goal:** Exercise notifications larger than the default ATT payload. + +Firmware: +- Grow `notify_payload` from 20 to 244 bytes (`CONFIG_BT_L2CAP_TX_MTU=247` minus the 3-byte ATT header), keeping the truncation guard. + +Tests: +- `test_mtu_sized_notification_payload`: after connecting, compute `len = min(peripheral.mtu() - 4, 243)` (the Control Point write carries the opcode byte and must fit in one ATT write), set a patterned payload of that length with 0x06, subscribe to `CONFIGURABLE_NOTIFY`, start notifications, and assert a notification arrives with the exact payload. If the negotiated MTU is the default 23, the test still runs with a 19-byte payload. + +**Commit:** `test(zephyr): Support and test MTU-sized notification payloads` + +### Step 13: Integration script updates + +**Goal:** Run the orphaned Windows hardware tests and fit the longer new tests. + +- In `scripts/run-integration-tests.sh`, on Windows (`$OS == Windows_NT` or `uname` matching MINGW/MSYS), also run `cargo test --lib -- --ignored` so the `src/winrtble/adapter.rs` radio tests (#476) execute. +- Raise the default per-test `TIMEOUT` from 20 to 40 seconds (Steps 4, 5, 7, and 11 include a reconnect or rescan cycle that can exceed 20 seconds). + +**Commit:** `scripts: Run Windows radio tests and raise integration test timeout` + +### Step 14: Final documentation and branch review + +- Update `tests/AGENTS.md` test categories and freshness date for the new tests (concurrency, errors, advertisement rotation). +- Update `test-peripheral/AGENTS.md` with the new characteristics, secondary service, opcode 0x04 behaviour, and payload size. +- Run a final `opus` review over the whole range of commits from Step 1 to here, and a full hardware run of `./scripts/run-integration-tests.sh` on macOS with the nRF52840. Record any platform that could not be run. + +**Commit:** `doc: Update test suite and test peripheral docs for regression expansion` + +## Future Work + +- **Bumble as hardware-free Linux CI.** The only strong reason to bring Bumble back: a Bumble virtual controller exposed to BlueZ through `hci_vhci`, with the virtual peripheral on the same virtual link, could run the suite against the BlueZ backend without a radio, possibly in GitHub Actions. Needs its own design plan; restore the old implementation from the Step 2 commit's parent as a starting point, fix its advertising layout, and bring it to parity with the Zephyr profile from this plan. +- **Service Changed / `DeviceServicesModified`.** Requires bonding support in the firmware so Zephyr sends Service Changed indications. From 7ff431f697022d445550f50dbf117856f309c5b4 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 16:17:11 -0700 Subject: [PATCH 03/32] build(zephyr): Pin test peripheral firmware to Zephyr v4.4.2 Add a west T2 manifest at test-peripheral/zephyr/west.yml pinning zephyr to v4.4.2 with dependencies under test-peripheral/deps/. The allowlist covers the nRF52840 DK and ESP32-S3 DevKitC builds: cmsis_6, hal_nordic, hal_espressif, segger (RTT logging), and mbedtls/tf-psa-crypto (ESP32-S3 host crypto, since its vendor controller lacks BT_CTLR_CRYPTO). v4.4 requires Zephyr SDK 1.0.1 and Python 3.12+. Document the uv venv, west init -l / west update, SDK install, and zephyr.base-prefer configfile setup, and replace the stale in-app workspace ignores. Co-Authored-By: Claude Opus 5.5 (1M context) --- .gitignore | 8 ++- README.md | 6 ++- docs/plan-zephyr-regression-tests.md | 8 +-- docs/zephyr-test-peripheral-debugging.md | 8 +-- test-peripheral/README.md | 62 ++++++++++++++++++++---- test-peripheral/zephyr/west.yml | 19 ++++++++ 6 files changed, 87 insertions(+), 24 deletions(-) create mode 100644 test-peripheral/zephyr/west.yml diff --git a/.gitignore b/.gitignore index 0f7a3d84..88377731 100644 --- a/.gitignore +++ b/.gitignore @@ -6,13 +6,11 @@ /workspace.xml **/*.code-workspace test-peripheral/.python-version +test-peripheral/.venv/ +test-peripheral/.west/ +test-peripheral/deps/ test-peripheral/zephyr/.cache -test-peripheral/zephyr/.west/ -test-peripheral/zephyr/bootloader/ test-peripheral/zephyr/build* -test-peripheral/zephyr/modules/ -test-peripheral/zephyr/tools/ -test-peripheral/zephyr/zephyr/ tests/android/gradle/ tests/android/gradlew .worktrees/ \ No newline at end of file diff --git a/README.md b/README.md index d43b0c82..c54fa086 100644 --- a/README.md +++ b/README.md @@ -188,10 +188,12 @@ btleplug includes integration tests that exercise real BLE operations against a **Quick start with hardware:** +See `test-peripheral/README.md` for the one-time pinned-workspace setup (`uv`, `west`, the Zephyr SDK). Once that's done: + ```bash -# Flash the nRF52840 DK (one-time setup) +# Flash the nRF52840 DK (.venv activated) cd test-peripheral/zephyr -west build -b nrf52840dk/nrf52840 && west flash +west build -b nrf52840dk/nrf52840 --pristine && west flash --runner nrfjprog # Run integration tests cargo test --test '*' -- --ignored diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index 3b7fc46e..76db80cc 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -65,11 +65,11 @@ Every step runs this loop. Do not start the next step until the current step is **Goal:** Reproducible firmware builds. -- Add `test-peripheral/zephyr/west.yml` (T2 star topology) pinning `zephyr` to `v4.4.0`, importing only the modules the two boards need (`hal_nordic`, `cmsis`/`cmsis_6`, `hal_espressif`, plus whatever the ESP32-S3 build requires). Before writing, have a research subagent confirm v4.4.0 is still the latest stable release and verify the allowlist names against the v4.4.0 `west.yml`. +- Add `test-peripheral/zephyr/west.yml` (T2 star topology) pinning `zephyr` to `v4.4.2` (Zephyr SDK 1.0.1), importing only the modules the two boards need (`hal_nordic`, `cmsis`/`cmsis_6`, `hal_espressif`, plus whatever the ESP32-S3 build requires). Before writing, have a research subagent confirm v4.4.2 is still the latest stable release and verify the allowlist names against the v4.4.2 `west.yml`. - Update `test-peripheral/README.md` and `docs/zephyr-test-peripheral-debugging.md` with the `west init -l` / `west update` workflow. - Verify the nRF52840 build succeeds against the pinned tree. -**Commit:** `build(zephyr): Pin test peripheral firmware to Zephyr v4.4.0` +**Commit:** `build(zephyr): Pin test peripheral firmware to Zephyr v4.4.2` ### Step 2: Retire Bumble and fix stale test docs @@ -136,7 +136,7 @@ Every step runs this loop. Do not start the next step until the current step is Firmware: - `ERROR_CHAR` `0x00000207` appended to the Read/Write service, `READ | WRITE`, both handlers return `BT_GATT_ERR(0x80)` (application error; avoid authentication/encryption errors, which trigger pairing on macOS and Windows). -- `REFUSED_NOTIFY_CHAR` `0x00000304` appended to the Notification service, `NOTIFY`, with a managed CCC whose write callback rejects every enable with an ATT error. Verify the exact v4.4.0 API (`BT_GATT_CCC_MANAGED` / `struct bt_gatt_ccc_managed_user_data` `cfg_write`, or a newer write-callback CCC macro) against the pinned tree before writing; earlier research on this signature was unreliable. +- `REFUSED_NOTIFY_CHAR` `0x00000304` appended to the Notification service, `NOTIFY`, with a managed CCC whose write callback rejects every enable with an ATT error. Verify the exact v4.4.2 API (`BT_GATT_CCC_MANAGED` / `struct bt_gatt_ccc_managed_user_data` `cfg_write`, or a newer write-callback CCC macro) against the pinned tree before writing; earlier research on this signature was unreliable. Tests: - `test_gatt_error_status_is_reported`: `read(ERROR_CHAR)` and `write(ERROR_CHAR, WithResponse)` both return `Err` within a timeout; a follow-up `read(STATIC_READ)` succeeds. @@ -163,7 +163,7 @@ Tests: Firmware: - Secondary service `0x00000005` (`BT_GATT_SECONDARY_SERVICE`) with `INCLUDED_CHAR` `0x00000501` (`READ`, fixed value `[0x05]`). -- Include it from the Descriptor service. Include declarations must immediately follow the service declaration, so this inserts at `descriptor_svc.attrs[1]`; nothing references `descriptor_svc` indices. Verify the v4.4.0 `BT_GATT_INCLUDE_SERVICE` argument type (it takes the included service's declaration attribute, not the service struct) against the pinned tree. +- Include it from the Descriptor service. Include declarations must immediately follow the service declaration, so this inserts at `descriptor_svc.attrs[1]`; nothing references `descriptor_svc` indices. Verify the v4.4.2 `BT_GATT_INCLUDE_SERVICE` argument type (it takes the included service's declaration attribute, not the service struct) against the pinned tree. Tests: - `test_discovery_with_included_service`: `find_and_connect()` succeeds (discovery completes), all four primary services are present, and `read(STATIC_READ)` succeeds afterwards. Do not assert whether the secondary service appears in `services()`; that is backend-dependent. diff --git a/docs/zephyr-test-peripheral-debugging.md b/docs/zephyr-test-peripheral-debugging.md index 77dae419..47d7169c 100644 --- a/docs/zephyr-test-peripheral-debugging.md +++ b/docs/zephyr-test-peripheral-debugging.md @@ -6,7 +6,7 @@ Date: 2026-03-01 1. **`BT_LE_ADV_CONN` removed in Zephyr v4.3** — replaced with `BT_LE_ADV_CONN_FAST_1` (30-60ms intervals). Fix in `src/main.c`. -2. **Missing `intelhex` Python package** — needed by `nrfjprog` flash runner. Fix: `uv pip install intelhex`. +2. **Missing `intelhex` Python package** — needed by `nrfjprog` flash runner. Fix: `uv pip install intelhex` (now installed via requirements-base.txt). 3. **Scan response too large** — adding `BT_DATA_UUID128_ALL` (18 bytes) + `BT_DATA_SVC_DATA128` (19 bytes) exceeds the 31-byte BLE legacy advertising limit. Fix: removed `svc_data`, kept only UUID list. @@ -14,8 +14,10 @@ Date: 2026-03-01 - Board: nRF52840 DK (S/N 683223459) - J-Link mass storage disabled via `JLinkExe` → `MSDDisable` (persistent, fixes macOS "Disk Not Ejected Properly" spam) -- Flash command: `uv run west flash --runner nrfjprog` (from `test-peripheral/zephyr/`) -- Build command: `uv run west build -b nrf52840dk/nrf52840` (from `test-peripheral/zephyr/`) +- Flash command: `west flash --runner nrfjprog` (from `test-peripheral/zephyr/`, with `test-peripheral/.venv` activated) +- Build command: `west build -b nrf52840dk/nrf52840 --pristine` (from `test-peripheral/zephyr/`) + +**Pinned workspace note:** the firmware now builds against a Zephyr `v4.4.2` tree pinned by `test-peripheral/zephyr/west.yml`, fetched into `test-peripheral/deps/` via `west init -l zephyr && west update` (from `test-peripheral/`), with a `uv`-managed `.venv` providing `west` and Zephyr's Python build requirements. See `test-peripheral/README.md` for the full one-time setup (including the Zephyr SDK 1.0.1 install). The commands above assume that workspace already exists and `.venv` is activated. ## UART Serial Issues (UNRESOLVED) diff --git a/test-peripheral/README.md b/test-peripheral/README.md index 9e905086..2ddbb5e8 100644 --- a/test-peripheral/README.md +++ b/test-peripheral/README.md @@ -6,26 +6,59 @@ This directory contains BLE test peripheral implementations for btleplug's integ ### Option A: Hardware (Zephyr) -The Zephyr firmware supports multiple boards. Pick whichever you have. +The Zephyr firmware supports multiple boards, built from a pinned Zephyr `v4.4.2` tree so builds are reproducible. Pick whichever board you have. **Prerequisites:** -- [Zephyr SDK](https://docs.zephyrproject.org/latest/develop/getting_started/index.html) and `west` tool +- `uv` (used below to create an isolated Python environment for `west` and Zephyr's build tooling — never system `pip`) +- Python 3.12+ (Zephyr v4.4.2 requires it; `uv venv --python 3.12` below provisions it for you even if your system Python is older) +- Host build tools — CMake ≥ 3.20.5, Ninja, and `dtc` (the devicetree compiler): + - macOS: `brew install cmake ninja dtc` + - Debian/Ubuntu: `apt install cmake ninja-build device-tree-compiler` + - See also Zephyr's [Install dependencies](https://docs.zephyrproject.org/latest/develop/getting_started/index.html) guide. - One of the supported boards: - [nRF52840 DK](https://www.nordicsemi.com/Products/Development-hardware/nRF52840-DK) — also needs [nRF Command Line Tools](https://www.nordicsemi.com/Products/Development-tools/nRF-Command-Line-Tools) - - [ESP32-S3 DevKitC](https://docs.espressif.com/projects/esp-dev-kits/en/latest/esp32s3/esp32-s3-devkitc-1/) — also needs `esptool` (`pip install esptool`) + - [ESP32-S3 DevKitC](https://docs.espressif.com/projects/esp-dev-kits/en/latest/esp32s3/esp32-s3-devkitc-1/) -**Build and flash:** +**Set up the pinned workspace (once, from `test-peripheral/`):** ```bash -cd zephyr +uv venv --python 3.12 .venv +uv pip install --python .venv/bin/python west +source .venv/bin/activate +# fish: source .venv/bin/activate.fish + +west init -l zephyr +west update +# Prevent an exported ZEPHYR_BASE from overriding the pinned tree above: +west config zephyr.base-prefer configfile + +# Now that deps/zephyr exists, install its build and flash requirements: +uv pip install --python .venv/bin/python \ + -r deps/zephyr/scripts/requirements-base.txt \ + -r deps/modules/hal/espressif/zephyr/requirements.txt + +# One-time Zephyr toolchain install (v4.4.2 requires SDK 1.0.1; installs to +# ~/zephyr-sdk-1.0.1 by default): +west sdk install --version 1.0.1 -t arm-zephyr-eabi xtensa-espressif_esp32s3_zephyr-elf + +# ESP32-S3 also needs its HAL blob libraries fetched once: +west blobs fetch hal_espressif +``` + +This creates `test-peripheral/.west/` (workspace metadata) and `test-peripheral/deps/` (the pinned Zephyr `v4.4.2` tree and its modules — `hal_nordic`, `hal_espressif`, `cmsis_6`, `mbedtls`, `tf-psa-crypto`, `segger`), both gitignored along with `.venv/`. `test-peripheral/zephyr/west.yml` is the manifest; after editing it, re-run `west update` from `test-peripheral/` with `.venv` activated. + +**Migrating from an older checkout:** if you previously ran `west init` inside `test-peripheral/zephyr/` (the pre-pinned layout), delete `test-peripheral/zephyr/.west`, `test-peripheral/zephyr/zephyr`, `test-peripheral/zephyr/modules`, `test-peripheral/zephyr/bootloader`, and `test-peripheral/zephyr/tools` before following the setup above. + +**Build and flash (from `test-peripheral/zephyr/`, with `.venv` activated):** +```bash # nRF52840 DK -west build -b nrf52840dk/nrf52840 -west flash +west build -b nrf52840dk/nrf52840 --pristine +west flash --runner nrfjprog # ESP32-S3 DevKitC -west build -b esp32s3_devkitc/esp32s3/procpu -west flash +west build -b esp32s3_devkitc/esp32s3/procpu --pristine -d build-esp32s3 +west flash -d build-esp32s3 ``` The board boots and immediately starts advertising as `"btleplug-test"`. @@ -107,11 +140,20 @@ Write these opcodes to the Control Point characteristic (`00000101-...`): ### Zephyr build fails -1. **Verify Zephyr SDK:** Run `west --version` and `cmake --version`. +1. **Verify the pinned workspace is active and intact:** + - `.venv` is activated (`source test-peripheral/.venv/bin/activate`, or `.venv/bin/activate.fish` under fish). + - `west topdir` prints `.../test-peripheral`. + - `west list zephyr` shows `deps/zephyr v4.4.2`. + - `echo $ZEPHYR_BASE` is empty, or `west config zephyr.base-prefer` prints `configfile` (see "Stray `ZEPHYR_BASE`" below). + - `west sdk list` shows `1.0.1` installed. 2. **Verify board target:** Board targets use slashes, not underscores (e.g. `nrf52840dk/nrf52840`, `esp32s3_devkitc/esp32s3/procpu`). 3. **Clean build:** `west build -b --pristine` 4. **ESP32 first build:** The Espressif HAL is large — first build takes significantly longer than subsequent builds. This is normal. +### Stray `ZEPHYR_BASE` + +If your shell (or another Zephyr workspace) exports `ZEPHYR_BASE`, `west` can resolve against that tree instead of the pinned `deps/zephyr` here. `west config zephyr.base-prefer configfile` (run once as part of setup above) makes `west` always prefer the workspace's own manifest over an exported `ZEPHYR_BASE`. + ### Bumble can't find USB dongle 1. **Check connection:** `lsusb` (Linux) or `system_profiler SPUSBDataType` (macOS). diff --git a/test-peripheral/zephyr/west.yml b/test-peripheral/zephyr/west.yml new file mode 100644 index 00000000..6343addb --- /dev/null +++ b/test-peripheral/zephyr/west.yml @@ -0,0 +1,19 @@ +manifest: + remotes: + - name: zephyrproject-rtos + url-base: https://github.com/zephyrproject-rtos + + projects: + - name: zephyr + remote: zephyrproject-rtos + revision: v4.4.2 + path: zephyr + import: + path-prefix: deps + name-allowlist: + - cmsis_6 + - hal_nordic + - hal_espressif + - mbedtls + - tf-psa-crypto + - segger From 9bc6f9e094531c431f63d77aeed4184846e51f3a Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 16:23:47 -0700 Subject: [PATCH 04/32] test: Retire unmaintained Bumble peripheral and fix stale test docs The Bumble virtual peripheral was never run, packed 47 bytes of AD structures into a 31-byte legacy advertising packet, and had drifted from the Zephyr firmware. Remove it and its documentation; it can be restored from history if a hardware-free CI path is pursued. Rename test-peripheral/CLAUDE.md to AGENTS.md and rewrite it for the single Zephyr implementation. Fix doc pointers in AGENTS.md, the scan timeout in tests/AGENTS.md, the opcode 0x04 description, and note which tests intentionally have no Android JNI export. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 8 +- README.md | 12 - docs/android-integration-test-runbook.md | 19 +- test-peripheral/AGENTS.md | 33 ++ test-peripheral/CLAUDE.md | 33 -- test-peripheral/README.md | 53 +-- test-peripheral/bumble/requirements.txt | 1 - test-peripheral/bumble/run.sh | 18 - test-peripheral/bumble/test_peripheral.py | 439 ---------------------- tests/AGENTS.md | 12 +- 10 files changed, 52 insertions(+), 576 deletions(-) create mode 100644 test-peripheral/AGENTS.md delete mode 100644 test-peripheral/CLAUDE.md delete mode 100644 test-peripheral/bumble/requirements.txt delete mode 100755 test-peripheral/bumble/run.sh delete mode 100644 test-peripheral/bumble/test_peripheral.py diff --git a/AGENTS.md b/AGENTS.md index 481aa601..cf0e22f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,13 +1,13 @@ # btleplug -Last verified: 2026-03-08 +Last verified: 2026-09-26 ## Tech Stack - Language: Rust (edition 2024) - Platforms: Windows (WinRT), macOS/iOS (CoreBluetooth), Linux (BlueZ), Android (JNI) - Async runtime: Tokio -- Testing: `cargo test`, integration tests require BLE hardware/virtual peripheral +- Testing: `cargo test`, integration tests require BLE hardware running the Zephyr test peripheral ## Commands @@ -28,8 +28,8 @@ Last verified: 2026-03-08 - `src/winrtble/` -- Windows WinRT backend - `src/common/` -- Shared utilities (non-Linux platforms) - `src/platform/` -- Platform-specific type exports -- `tests/` -- Integration test suite (see `tests/CLAUDE.md`) -- `test-peripheral/` -- BLE test peripheral implementations (see `test-peripheral/CLAUDE.md`) +- `tests/` -- Integration test suite (see `tests/AGENTS.md`) +- `test-peripheral/` -- BLE test peripheral firmware (Zephyr) (see `test-peripheral/AGENTS.md`) - `scripts/` -- Build and test automation scripts ## Feature Flags diff --git a/README.md b/README.md index c54fa086..15255307 100644 --- a/README.md +++ b/README.md @@ -199,18 +199,6 @@ west build -b nrf52840dk/nrf52840 --pristine && west flash --runner nrfjprog cargo test --test '*' -- --ignored ``` -**Quick start with a virtual peripheral (requires USB BLE dongle):** - -```bash -# Start the Bumble virtual peripheral -cd test-peripheral/bumble -pip install -r requirements.txt -./run.sh usb:0 # transport argument may vary; see Bumble docs - -# In another terminal, run integration tests -cargo test --test '*' -- --ignored -``` - See `test-peripheral/README.md` for full setup instructions. ## License diff --git a/docs/android-integration-test-runbook.md b/docs/android-integration-test-runbook.md index 3f261f0f..1a1b0e08 100644 --- a/docs/android-integration-test-runbook.md +++ b/docs/android-integration-test-runbook.md @@ -14,22 +14,9 @@ The SIGABRT in logcat is a Rust panic from `peripheral_finder::find_and_connect( ## What's Needed to Run Tests -### 1. Start a BLE test peripheral +### 1. Start the Zephyr test peripheral -Two options exist in `test-peripheral/`: - -**Option A: Bumble virtual peripheral (easiest)** -```bash -cd test-peripheral/bumble -pip install -r requirements.txt -python test_peripheral.py -``` -- Requires a Bluetooth USB adapter or HCI transport -- The Bumble peripheral advertises as "btleplug-test" with the test GATT profile - -**Option B: Zephyr hardware peripheral** -- Requires a Zephyr-supported BLE board (e.g., nRF52840) -- See `test-peripheral/zephyr/` and `docs/zephyr-test-peripheral-debugging.md` +See `test-peripheral/README.md` for setup and build instructions. The firmware runs on a Zephyr-supported BLE board (e.g., nRF52840) and advertises as "btleplug-test" with the test GATT profile. ### 2. Run the Android tests @@ -159,7 +146,7 @@ d21c37a fix: resolve build issues for Android integration tests ## Next Steps -1. **Start a test peripheral** (Bumble or Zephyr) so the scan finds "btleplug-test" +1. **Start the Zephyr test peripheral** so the scan finds "btleplug-test" 2. **Implement `catch_unwind`** in the JNI test wrapper so panics become JUnit failures instead of process crashes 3. **Run tests** and iterate on any BLE-specific Android failures 4. **Squash/clean commits** if desired before merging to master diff --git a/test-peripheral/AGENTS.md b/test-peripheral/AGENTS.md new file mode 100644 index 00000000..7ee0203e --- /dev/null +++ b/test-peripheral/AGENTS.md @@ -0,0 +1,33 @@ +# test-peripheral/ -- BLE Test Peripheral Firmware (Zephyr) + +Freshness: 2026-09-26 + +## Purpose + +Zephyr firmware GATT test peripheral used by the integration tests in `tests/`. The Bumble virtual peripheral was removed in the commit that introduced this file and can be restored from git history. + +## Structure + +- `zephyr/` -- C firmware for Zephyr RTOS (nRF52840 DK, ESP32-S3 DevKitC) + - `src/gatt_profile.h` -- UUIDs, Control Point opcodes, test constants, shared `peripheral_state` + - `src/gatt_profile.c` -- GATT service attribute tables (`BT_GATT_SERVICE_DEFINE`) + - `src/control_service.c` -- Control Point dispatch, periodic notify/indicate, reset, delayed disconnect + - `src/test_handlers.c` -- read/write/descriptor/CCC callbacks + - `src/main.c` -- advertising and scan response data, connection callbacks, advertising restart + - `boards/*.conf` -- per-board config (nRF52840 uses Segger RTT logging) + - `west.yml` -- west T2 manifest pinning Zephyr v4.4.2 (SDK 1.0.1) +- `.venv/` -- gitignored uv venv +- `.west/` -- gitignored west workspace (setup in `README.md`) +- `deps/` -- gitignored Zephyr v4.4.2 tree and modules (hal_nordic, hal_espressif, cmsis_6, mbedtls, tf-psa-crypto, segger) + +## Contracts + +- The canonical UUID source of truth is `tests/common/gatt_uuids.rs`, mirrored in `zephyr/src/gatt_profile.h`. +- Four GATT services: Control (`0x0001`), Read/Write (`0x0002`), Notification (`0x0003`), Descriptor (`0x0004`). +- Control Point opcodes: `0x01` start notifications, `0x02` stop, `0x03` disconnect, `0x04` change adverts (currently a no-op, logged only; not yet implemented), `0x05` reset, `0x06` set notification payload. +- Peripheral advertises as `"btleplug-test"` with the Control Service UUID in the scan response. + +## Invariants + +- Adding a new GATT characteristic requires updating both `tests/common/gatt_uuids.rs` and `zephyr/src/gatt_profile.h`. +- New characteristics are appended to the end of existing services so the hardcoded `notify_svc.attrs[]` indices in `control_service.c` stay valid, and update the index table comment in `periodic_notify_handler` (`control_service.c`) when `notify_svc` grows. diff --git a/test-peripheral/CLAUDE.md b/test-peripheral/CLAUDE.md deleted file mode 100644 index eca85d51..00000000 --- a/test-peripheral/CLAUDE.md +++ /dev/null @@ -1,33 +0,0 @@ -# test-peripheral/ -- BLE Test Peripheral Implementations - -Freshness: 2026-02-22 - -## Purpose - -Two implementations of the same GATT profile used by the integration tests in `tests/`. One targets real hardware (Zephyr on nRF52840), the other is a virtual peripheral (Bumble on a USB BLE dongle). - -## Structure - -- `bumble/` -- Python virtual peripheral using the Bumble BLE stack - - `test_peripheral.py` -- full GATT profile implementation - - `run.sh` -- launcher script (usage: `./run.sh usb:0`) - - `requirements.txt` -- Python dependencies (bumble) -- `zephyr/` -- C firmware for Zephyr RTOS (nRF52840 DK, ESP32-S3 DevKitC) - - `src/gatt_profile.h` -- UUID definitions and GATT service declarations - - `src/gatt_profile.c` -- read/write/notify handlers - - `src/control_service.c` -- Control Point command dispatch - - `src/test_handlers.c` -- test-specific behavior (counters, buffers) - - `src/main.c` -- BLE init, advertising, connection management - -## Contracts - -- Both implementations MUST expose identical GATT services with identical UUIDs and behavior. -- The canonical UUID source of truth is `tests/common/gatt_uuids.rs`. Both peripherals mirror these. -- Four GATT services: Control (`0x0001`), Read/Write (`0x0002`), Notification (`0x0003`), Descriptor (`0x0004`). -- Control Point opcodes: `0x01` start notifications, `0x02` stop, `0x03` disconnect, `0x04` change adverts, `0x05` reset, `0x06` set notification payload. -- Peripheral advertises as `"btleplug-test"` with the Control Service UUID in the scan response. - -## Invariants - -- Adding a new GATT characteristic requires updating all three locations: `gatt_uuids.rs`, `gatt_profile.h`, and `test_peripheral.py`. -- The Bumble implementation must behave identically to the Zephyr firmware for all tests to pass on both. diff --git a/test-peripheral/README.md b/test-peripheral/README.md index 2ddbb5e8..f7f4025c 100644 --- a/test-peripheral/README.md +++ b/test-peripheral/README.md @@ -1,10 +1,10 @@ -# btleplug Integration Test Peripherals +# btleplug Integration Test Peripheral -This directory contains BLE test peripheral implementations for btleplug's integration test suite. Both implementations expose an identical GATT profile that the Rust tests in `tests/` exercise. +This directory contains the Zephyr firmware for a BLE test peripheral used by btleplug's integration test suite. It exposes a GATT profile that the Rust tests in `tests/` exercise. ## Quick Start -### Option A: Hardware (Zephyr) +### Zephyr hardware setup The Zephyr firmware supports multiple boards, built from a pinned Zephyr `v4.4.2` tree so builds are reproducible. Pick whichever board you have. @@ -70,36 +70,9 @@ The board boots and immediately starts advertising as `"btleplug-test"`. cargo test --test '*' -- --ignored ``` -### Option B: Virtual Peripheral (Bumble) - -**Prerequisites:** -- Python 3.10+ -- A USB BLE dongle (separate from the host's built-in BLE adapter) -- `libusb` (`brew install libusb` on macOS, `apt install libusb-1.0-0` on Linux) - -**Setup:** - -```bash -cd bumble -pip install -r requirements.txt -``` - -**Run:** - -```bash -./run.sh usb:0 # USB dongle (most common) -./run.sh hci-socket:0 # Linux HCI socket (requires sudo) -``` - -**Run integration tests** (in another terminal): - -```bash -cargo test --test '*' -- --ignored -``` - ## GATT Test Profile -Both peripherals implement an identical GATT profile. The canonical UUID definitions are in `tests/common/gatt_uuids.rs` (Rust) and mirrored in `zephyr/src/gatt_profile.h` (C) and `bumble/test_peripheral.py` (Python). +The Zephyr firmware implements the test GATT profile. The canonical UUID definitions are in `tests/common/gatt_uuids.rs` (Rust) and mirrored in `zephyr/src/gatt_profile.h` (C). ### Services @@ -119,7 +92,7 @@ Write these opcodes to the Control Point characteristic (`00000101-...`): | `0x01` | Start Notifications | Begin periodic notifications (1 Hz) | | `0x02` | Stop Notifications | Stop all periodic notifications | | `0x03` | Trigger Disconnect | Peripheral disconnects after 500ms | -| `0x04` | Change Advertisements | Rotate advertisement data | +| `0x04` | Change Advertisements | Reserved; currently a no-op (logged only) | | `0x05` | Reset State | Stop notifications, clear all buffers | | `0x06` | Set Notification Payload | Remaining bytes become the notification payload | @@ -136,7 +109,7 @@ Write these opcodes to the Control Point characteristic (`00000101-...`): 1. **Verify the peripheral is advertising:** Use a BLE scanner app (nRF Connect, LightBlue) to confirm `"btleplug-test"` appears. 2. **Check Bluetooth adapter:** Ensure your host has a working BLE adapter. On Linux, run `hciconfig` or `bluetoothctl show`. 3. **Check permissions:** On Linux, you may need to run tests with `sudo` or add your user to the `bluetooth` group. -4. **Increase scan timeout:** If the peripheral takes a while to appear, the default 10-second scan timeout in `peripheral_finder.rs` may need extending. +4. **Increase scan timeout:** If the peripheral takes a while to appear, `DEFAULT_SCAN_TIMEOUT` (15 s) in `tests/common/peripheral_finder.rs` may need extending. ### Zephyr build fails @@ -154,17 +127,3 @@ Write these opcodes to the Control Point characteristic (`00000101-...`): If your shell (or another Zephyr workspace) exports `ZEPHYR_BASE`, `west` can resolve against that tree instead of the pinned `deps/zephyr` here. `west config zephyr.base-prefer configfile` (run once as part of setup above) makes `west` always prefer the workspace's own manifest over an exported `ZEPHYR_BASE`. -### Bumble can't find USB dongle - -1. **Check connection:** `lsusb` (Linux) or `system_profiler SPUSBDataType` (macOS). -2. **Check libusb:** `python -c "import usb.core; print(list(usb.core.find(find_all=True)))"` -3. **Permissions:** On Linux, you may need udev rules for your dongle. On macOS, the dongle should work out of the box. - -### Tests pass with hardware but fail with Bumble - -Some tests are sensitive to hardware timing: -- **RSSI tests:** Bumble may not provide realistic RSSI values. -- **MTU tests:** MTU negotiation behavior may differ. -- **Connection parameter tests:** May not be supported over virtual transport. - -These tests should pass with the nRF52840 DK. diff --git a/test-peripheral/bumble/requirements.txt b/test-peripheral/bumble/requirements.txt deleted file mode 100644 index 634634ed..00000000 --- a/test-peripheral/bumble/requirements.txt +++ /dev/null @@ -1 +0,0 @@ -bumble>=0.0.200 diff --git a/test-peripheral/bumble/run.sh b/test-peripheral/bumble/run.sh deleted file mode 100755 index 98dc08b3..00000000 --- a/test-peripheral/bumble/run.sh +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env bash -# Start the btleplug test peripheral using Bumble. -# -# Usage: -# ./run.sh # Use default USB transport (usb:0) -# ./run.sh usb:0 # Explicit USB transport -# ./run.sh hci-socket:0 # Linux HCI socket transport -# -# Prerequisites: -# pip install -r requirements.txt - -set -euo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -TRANSPORT="${1:-usb:0}" - -echo "Starting btleplug test peripheral on transport: ${TRANSPORT}" -python3 "${SCRIPT_DIR}/test_peripheral.py" "${TRANSPORT}" diff --git a/test-peripheral/bumble/test_peripheral.py b/test-peripheral/bumble/test_peripheral.py deleted file mode 100644 index 2f58d99a..00000000 --- a/test-peripheral/bumble/test_peripheral.py +++ /dev/null @@ -1,439 +0,0 @@ -""" -btleplug Test Peripheral — Bumble Implementation - -Implements the same GATT profile as the Zephyr firmware for testing -btleplug without dedicated hardware. Requires a USB BLE dongle or -platform-specific BLE transport. - -Usage: - python test_peripheral.py - python test_peripheral.py usb:0 - python test_peripheral.py hci-socket:0 -""" - -import asyncio -import logging -import struct -import sys - -from bumble.core import UUID as BumbleUUID, AdvertisingData -from bumble.device import Device, Connection -from bumble.gatt import ( - Service, - Characteristic, - CharacteristicValue, - Descriptor, -) -from bumble.host import Host -from bumble.transport import open_transport_or_link - -logging.basicConfig(level=logging.INFO) -logger = logging.getLogger(__name__) - -# ────────────────────────────────────────────────────────────── -# UUID Constants (must match tests/common/gatt_uuids.rs and -# test-peripheral/zephyr/src/gatt_profile.h) -# ────────────────────────────────────────────────────────────── - -CONTROL_SERVICE_UUID = BumbleUUID("00000001-b5a3-f393-e0a9-e50e24dcca9e") -CONTROL_POINT_UUID = BumbleUUID("00000101-b5a3-f393-e0a9-e50e24dcca9e") -CONTROL_RESPONSE_UUID = BumbleUUID("00000102-b5a3-f393-e0a9-e50e24dcca9e") - -RW_SERVICE_UUID = BumbleUUID("00000002-b5a3-f393-e0a9-e50e24dcca9e") -STATIC_READ_UUID = BumbleUUID("00000201-b5a3-f393-e0a9-e50e24dcca9e") -COUNTER_READ_UUID = BumbleUUID("00000202-b5a3-f393-e0a9-e50e24dcca9e") -WRITE_WITH_RESP_UUID = BumbleUUID("00000203-b5a3-f393-e0a9-e50e24dcca9e") -WRITE_WITHOUT_RESP_UUID = BumbleUUID("00000204-b5a3-f393-e0a9-e50e24dcca9e") -READ_WRITE_UUID = BumbleUUID("00000205-b5a3-f393-e0a9-e50e24dcca9e") -LONG_VALUE_UUID = BumbleUUID("00000206-b5a3-f393-e0a9-e50e24dcca9e") - -NOTIFY_SERVICE_UUID = BumbleUUID("00000003-b5a3-f393-e0a9-e50e24dcca9e") -NOTIFY_CHAR_UUID = BumbleUUID("00000301-b5a3-f393-e0a9-e50e24dcca9e") -INDICATE_CHAR_UUID = BumbleUUID("00000302-b5a3-f393-e0a9-e50e24dcca9e") -CONFIGURABLE_NOTIFY_UUID = BumbleUUID("00000303-b5a3-f393-e0a9-e50e24dcca9e") - -DESCRIPTOR_SERVICE_UUID = BumbleUUID("00000004-b5a3-f393-e0a9-e50e24dcca9e") -DESCRIPTOR_TEST_CHAR_UUID = BumbleUUID("00000401-b5a3-f393-e0a9-e50e24dcca9e") -RO_DESCRIPTOR_UUID = BumbleUUID("000004a1-b5a3-f393-e0a9-e50e24dcca9e") -RW_DESCRIPTOR_UUID = BumbleUUID("000004a2-b5a3-f393-e0a9-e50e24dcca9e") - -# Control Point opcodes -CMD_START_NOTIFICATIONS = 0x01 -CMD_STOP_NOTIFICATIONS = 0x02 -CMD_TRIGGER_DISCONNECT = 0x03 -CMD_CHANGE_ADVERTISEMENTS = 0x04 -CMD_RESET_STATE = 0x05 -CMD_SET_NOTIFICATION_PAYLOAD = 0x06 - -DEVICE_NAME = "btleplug-test" -TEST_APPEARANCE = 0x0340 -MANUFACTURER_COMPANY_ID = 0xFFFF -STATIC_READ_VALUE = bytes([0x01, 0x02, 0x03, 0x04]) -NOTIFICATION_INTERVAL = 1.0 # seconds - - -class TestPeripheralState: - """Mutable state for the test peripheral.""" - - def __init__(self): - self.reset() - - def reset(self): - self.read_counter = 0 - self.rw_value = bytearray() - self.long_value = bytearray(512) - self.write_with_resp_value = bytearray() - self.write_without_resp_value = bytearray() - self.notify_payload = bytearray() - self.rw_descriptor_value = bytearray() - self.notify_task: asyncio.Task | None = None - - -state = TestPeripheralState() - - -# ────────────────────────────────────────────────────────────── -# GATT Handlers -# ────────────────────────────────────────────────────────────── - -def read_static(_connection): - return STATIC_READ_VALUE - - -def read_counter(_connection): - val = struct.pack(" 0: - try: - await device.notify_subscribers( - configurable_notify_ref, bytes(state.notify_payload) - ) - except Exception as e: - logger.debug("Configurable notify failed: %s", e) - - -def start_notifications(device: Device): - if state.notify_task is None or state.notify_task.done(): - state.notify_task = asyncio.get_running_loop().create_task( - notification_loop(device) - ) - logger.info("Periodic notifications started") - - -def stop_notifications(): - if state.notify_task is not None and not state.notify_task.done(): - state.notify_task.cancel() - state.notify_task = None - logger.info("Periodic notifications stopped") - - -# ────────────────────────────────────────────────────────────── -# Control Point Handler -# ────────────────────────────────────────────────────────────── - -def make_control_point_handler(device: Device): - def handler(_connection, value): - if len(value) < 1: - logger.warning("Empty control command") - return - - opcode = value[0] - logger.info("Control command: 0x%02x", opcode) - - if opcode == CMD_START_NOTIFICATIONS: - start_notifications(device) - elif opcode == CMD_STOP_NOTIFICATIONS: - stop_notifications() - elif opcode == CMD_TRIGGER_DISCONNECT: - async def do_disconnect(): - await asyncio.sleep(0.5) - if _connection and not _connection.is_disconnected: - await _connection.disconnect() - asyncio.get_running_loop().create_task(do_disconnect()) - elif opcode == CMD_CHANGE_ADVERTISEMENTS: - # Deferred: advertisement rotation is not tested in the initial suite - logger.info("Change advertisements (deferred — not tested in initial suite)") - elif opcode == CMD_RESET_STATE: - stop_notifications() - state.reset() - logger.info("Peripheral state reset") - elif opcode == CMD_SET_NOTIFICATION_PAYLOAD: - if len(value) > 1: - state.notify_payload = bytearray(value[1:]) - logger.info("Notification payload set: %d bytes", len(state.notify_payload)) - else: - logger.warning("Unknown control opcode: 0x%02x", opcode) - - return handler - - -# ────────────────────────────────────────────────────────────── -# Build GATT Services -# ────────────────────────────────────────────────────────────── - -def build_services(device: Device): - global notify_char_ref, indicate_char_ref, configurable_notify_ref - - # --- Control Service --- - control_point_char = Characteristic( - CONTROL_POINT_UUID, - Characteristic.Properties.WRITE, - Characteristic.WRITEABLE, - CharacteristicValue(write=make_control_point_handler(device)), - ) - control_response_char = Characteristic( - CONTROL_RESPONSE_UUID, - Characteristic.Properties.NOTIFY, - 0, - ) - control_service = Service(CONTROL_SERVICE_UUID, [ - control_point_char, - control_response_char, - ]) - - # --- Read/Write Test Service --- - static_read_char = Characteristic( - STATIC_READ_UUID, - Characteristic.Properties.READ, - Characteristic.READABLE, - CharacteristicValue(read=read_static), - ) - counter_read_char = Characteristic( - COUNTER_READ_UUID, - Characteristic.Properties.READ, - Characteristic.READABLE, - CharacteristicValue(read=read_counter), - ) - write_with_resp_char = Characteristic( - WRITE_WITH_RESP_UUID, - Characteristic.Properties.WRITE, - Characteristic.WRITEABLE, - CharacteristicValue(write=write_with_resp), - ) - write_without_resp_char = Characteristic( - WRITE_WITHOUT_RESP_UUID, - Characteristic.Properties.WRITE_WITHOUT_RESPONSE, - Characteristic.WRITEABLE, - CharacteristicValue(write=write_without_resp), - ) - read_write_char = Characteristic( - READ_WRITE_UUID, - Characteristic.Properties.READ | Characteristic.Properties.WRITE, - Characteristic.READABLE | Characteristic.WRITEABLE, - CharacteristicValue(read=read_rw, write=write_rw), - ) - long_value_char = Characteristic( - LONG_VALUE_UUID, - Characteristic.Properties.READ | Characteristic.Properties.WRITE, - Characteristic.READABLE | Characteristic.WRITEABLE, - CharacteristicValue(read=read_long, write=write_long), - ) - rw_service = Service(RW_SERVICE_UUID, [ - static_read_char, - counter_read_char, - write_with_resp_char, - write_without_resp_char, - read_write_char, - long_value_char, - ]) - - # --- Notification Test Service --- - notify_char = Characteristic( - NOTIFY_CHAR_UUID, - Characteristic.Properties.NOTIFY, - 0, - ) - indicate_char = Characteristic( - INDICATE_CHAR_UUID, - Characteristic.Properties.INDICATE, - 0, - ) - configurable_notify_char = Characteristic( - CONFIGURABLE_NOTIFY_UUID, - Characteristic.Properties.NOTIFY, - 0, - ) - notify_service = Service(NOTIFY_SERVICE_UUID, [ - notify_char, - indicate_char, - configurable_notify_char, - ]) - - # Store references for notification sending - notify_char_ref = notify_char - indicate_char_ref = indicate_char - configurable_notify_ref = configurable_notify_char - - # --- Descriptor Test Service --- - ro_descriptor = Descriptor( - RO_DESCRIPTOR_UUID, - Descriptor.READABLE, - CharacteristicValue(read=read_ro_descriptor), - ) - rw_descriptor = Descriptor( - RW_DESCRIPTOR_UUID, - Descriptor.READABLE | Descriptor.WRITEABLE, - CharacteristicValue(read=read_rw_descriptor, write=write_rw_descriptor), - ) - descriptor_test_char = Characteristic( - DESCRIPTOR_TEST_CHAR_UUID, - Characteristic.Properties.READ, - Characteristic.READABLE, - CharacteristicValue(read=read_descriptor_test_char), - descriptors=[ro_descriptor, rw_descriptor], - ) - descriptor_service = Service(DESCRIPTOR_SERVICE_UUID, [ - descriptor_test_char, - ]) - - return [control_service, rw_service, notify_service, descriptor_service] - - -# ────────────────────────────────────────────────────────────── -# Main -# ────────────────────────────────────────────────────────────── - -async def main(): - if len(sys.argv) < 2: - print(f"Usage: {sys.argv[0]} ") - print(" e.g.: python test_peripheral.py usb:0") - print(" e.g.: python test_peripheral.py hci-socket:0") - sys.exit(1) - - transport_name = sys.argv[1] - logger.info("Opening transport: %s", transport_name) - - async with await open_transport_or_link(transport_name) as ( - hci_source, - hci_sink, - ): - device = Device(name=DEVICE_NAME, host=Host(hci_source, hci_sink)) - - # Register GATT services - services = build_services(device) - for service in services: - device.add_service(service) - - # Power on - await device.power_on() - - # Set up advertising data - device.advertising_data = bytes( - AdvertisingData( - [ - ( - AdvertisingData.COMPLETE_LOCAL_NAME, - DEVICE_NAME.encode("utf-8"), - ), - ( - AdvertisingData.INCOMPLETE_LIST_OF_128_BIT_SERVICE_CLASS_UUIDS, - bytes(CONTROL_SERVICE_UUID), - ), - ( - AdvertisingData.MANUFACTURER_SPECIFIC_DATA, - struct.pack(" Date: Sat, 26 Sep 2026 16:46:58 -0700 Subject: [PATCH 05/32] test: Add concurrent connect, discovery, and same-service operation tests Cover #488 (every concurrent connect/discover/disconnect waiter resolves), #481 (concurrent operations within one service), and #489 (reads in flight across service rediscovery). BlueZ is the only backend allowed to reject a second concurrent connect(), since bluez-async issues Device1.Connect() without dedup. On macOS the test waits for the peripheral to be rediscovered after the initial disconnect, because CoreBluetooth drops it from its peripheral map on disconnect and cannot reconnect until it advertises again. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/android/rust/src/lib.rs | 12 + .../btleplug/test/BleIntegrationTest.kt | 5 + .../btleplug/test/NativeTests.kt | 5 + tests/common/test_cases.rs | 212 ++++++++++++++++++ tests/test_concurrent_connect_and_discover.rs | 7 + ...test_concurrent_operations_same_service.rs | 7 + tests/test_discover_services_during_read.rs | 7 + 7 files changed, 255 insertions(+) create mode 100644 tests/test_concurrent_connect_and_discover.rs create mode 100644 tests/test_concurrent_operations_same_service.rs create mode 100644 tests/test_discover_services_during_read.rs diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index d34baa37..ad4328a6 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -243,3 +243,15 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testRequestConnectionParameters, test_cases::test_request_connection_parameters ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testConcurrentConnectAndDiscover, + test_cases::test_concurrent_connect_and_discover +); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testConcurrentOperationsSameService, + test_cases::test_concurrent_operations_same_service +); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testDiscoverServicesDuringRead, + test_cases::test_discover_services_during_read +); diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index 87f1eb65..22d6ab13 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -108,4 +108,9 @@ class BleIntegrationTest { @Test fun testPropertiesContainPeripheralInfo() = NativeTests.testPropertiesContainPeripheralInfo() @Test fun testConnectionParameters() = NativeTests.testConnectionParameters() @Test fun testRequestConnectionParameters() = NativeTests.testRequestConnectionParameters() + + // ── Concurrency ───────────────────────────────────────────────── + @Test fun testConcurrentConnectAndDiscover() = NativeTests.testConcurrentConnectAndDiscover() + @Test fun testConcurrentOperationsSameService() = NativeTests.testConcurrentOperationsSameService() + @Test fun testDiscoverServicesDuringRead() = NativeTests.testDiscoverServicesDuringRead() } diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index 8c0b8815..5279b726 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -50,4 +50,9 @@ object NativeTests { external fun testPropertiesContainPeripheralInfo() external fun testConnectionParameters() external fun testRequestConnectionParameters() + + // Concurrency + external fun testConcurrentConnectAndDiscover() + external fun testConcurrentOperationsSameService() + external fun testDiscoverServicesDuringRead() } diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 1a836aca..a4a065f9 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -849,6 +849,218 @@ pub async fn test_connection_parameters() { peripheral.disconnect().await.unwrap(); } +// ── Concurrency ───────────────────────────────────────────────────── + +pub async fn test_concurrent_connect_and_discover() { + use futures::join; + use std::time::Duration; + use tokio::time::timeout; + + let peripheral = peripheral_finder::find_and_connect().await; + assert!(peripheral.is_connected().await.unwrap()); + + #[cfg(target_vendor = "apple")] + let mut events = { + use btleplug::api::Central; + peripheral_finder::get_adapter() + .await + .events() + .await + .unwrap() + }; + + peripheral.disconnect().await.unwrap(); + assert!(!peripheral.is_connected().await.unwrap()); + + // CoreBluetooth drops the peripheral from its internal map on disconnect + // and only re-inserts it once the background scan rediscovers it; wait + // for that rediscovery before attempting to reconnect (see + // src/corebluetooth/internal.rs on_peripheral_disconnect/on_discovered_peripheral). + // Other backends keep the peripheral connectable across disconnects, so + // they go straight to the concurrent connect below. + #[cfg(target_vendor = "apple")] + { + use btleplug::api::CentralEvent; + use futures::StreamExt; + + // CoreBluetooth only emits DeviceDiscovered for an id that is + // currently absent from AdapterManager, and AdapterManager removes + // the peripheral on DeviceDisconnected (src/common/adapter_manager.rs + // emit()), so a DeviceDiscovered(target_id) is always fresh. A + // DeviceUpdated(target_id) is only trustworthy once we've also seen + // the id actually leave and re-enter the map (DeviceDisconnected or + // DeviceDiscovered), since one queued before our disconnect would + // otherwise be indistinguishable from a post-rediscovery update. + let target_id = peripheral.id(); + let mut seen_removed_or_discovered = false; + timeout(Duration::from_secs(15), async { + loop { + match events.next().await { + Some(CentralEvent::DeviceDiscovered(id)) if id == target_id => break, + Some(CentralEvent::DeviceDisconnected(id)) if id == target_id => { + seen_removed_or_discovered = true; + } + Some(CentralEvent::DeviceUpdated(id)) + if id == target_id && seen_removed_or_discovered => + { + break; + } + Some(_) => continue, + None => panic!("adapter event stream ended while waiting for rediscovery"), + } + } + }) + .await + .expect("timed out waiting for peripheral rediscovery after disconnect"); + } + + // Concurrent connect (#488). + let results = timeout(Duration::from_secs(15), async { + join!(peripheral.connect(), peripheral.connect()) + }) + .await + .expect("concurrent connect() calls did not complete within timeout"); + #[cfg(target_os = "linux")] + { + // bluez-async's connect_with_timeout issues Device1.Connect() directly + // with no dedup, so a second concurrent connect() may be rejected + // with org.bluez.Error.InProgress; require at least one to succeed. + let (first, second) = results; + assert!( + first.is_ok() || second.is_ok(), + "both concurrent connect() calls failed" + ); + } + #[cfg(not(target_os = "linux"))] + { + let (first, second) = results; + first.expect("first concurrent connect() failed"); + second.expect("second concurrent connect() failed"); + } + assert!( + peripheral.is_connected().await.unwrap(), + "peripheral should be connected after concurrent connect() calls" + ); + + // Concurrent service discovery (#489 rediscovery). + let results = timeout(Duration::from_secs(15), async { + join!( + peripheral.discover_services(), + peripheral.discover_services() + ) + }) + .await + .expect("concurrent discover_services() calls did not complete within timeout"); + let (first, second) = results; + first.expect("first concurrent discover_services() failed"); + second.expect("second concurrent discover_services() failed"); + assert!( + !peripheral.services().is_empty(), + "services should be populated after concurrent discover_services() calls" + ); + + // Concurrent disconnect. + let results = timeout(Duration::from_secs(15), async { + join!(peripheral.disconnect(), peripheral.disconnect()) + }) + .await + .expect("concurrent disconnect() calls did not complete within timeout"); + let (first, second) = results; + first.expect("first concurrent disconnect() failed"); + second.expect("second concurrent disconnect() failed"); + assert!( + !peripheral.is_connected().await.unwrap(), + "peripheral should be disconnected after concurrent disconnect() calls" + ); +} + +pub async fn test_concurrent_operations_same_service() { + use futures::join; + use std::time::Duration; + use tokio::time::timeout; + + let peripheral = peripheral_finder::find_and_connect().await; + // Subscribing mutates CCC state. + peripheral_finder::reset_peripheral(&peripheral).await; + + let notify_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::NOTIFY_CHAR); + let indicate_char = + peripheral_finder::find_characteristic(&peripheral, gatt_uuids::INDICATE_CHAR); + + // Concurrent subscribe to two characteristics in the same service (#481 + // WinRT deadlock). + let (subscribe_notify, subscribe_indicate) = timeout(Duration::from_secs(15), async { + join!( + peripheral.subscribe(¬ify_char), + peripheral.subscribe(&indicate_char) + ) + }) + .await + .expect("concurrent subscribe() calls did not complete within timeout"); + subscribe_notify.expect("subscribe(NOTIFY_CHAR) should succeed"); + subscribe_indicate.expect("subscribe(INDICATE_CHAR) should succeed"); + + let static_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::STATIC_READ); + let counter_char = + peripheral_finder::find_characteristic(&peripheral, gatt_uuids::COUNTER_READ); + + // Concurrent read of two characteristics in the same service. + let (read_static, read_counter) = timeout(Duration::from_secs(15), async { + join!( + peripheral.read(&static_char), + peripheral.read(&counter_char) + ) + }) + .await + .expect("concurrent read() calls did not complete within timeout"); + let static_value = read_static.expect("read(STATIC_READ) should succeed"); + read_counter.expect("read(COUNTER_READ) should succeed"); + assert_eq!( + static_value, + gatt_uuids::STATIC_READ_VALUE, + "Static read should return [0x01, 0x02, 0x03, 0x04]" + ); + + peripheral.unsubscribe(¬ify_char).await.unwrap(); + peripheral.unsubscribe(&indicate_char).await.unwrap(); + peripheral.disconnect().await.unwrap(); +} + +pub async fn test_discover_services_during_read() { + use futures::join; + use std::time::Duration; + use tokio::time::timeout; + + let peripheral = peripheral_finder::find_and_connect().await; + peripheral_finder::reset_peripheral(&peripheral).await; + let counter_char = + peripheral_finder::find_characteristic(&peripheral, gatt_uuids::COUNTER_READ); + + let (read_result, discover_result) = timeout(Duration::from_secs(15), async { + join!( + peripheral.read(&counter_char), + peripheral.discover_services() + ) + }) + .await + .expect("concurrent read()/discover_services() calls did not complete within timeout"); + read_result.expect("read(COUNTER_READ) should succeed during concurrent discovery"); + discover_result.expect("discover_services() should succeed during concurrent read"); + + let static_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::STATIC_READ); + let static_value = peripheral + .read(&static_char) + .await + .expect("follow-up read(STATIC_READ) should succeed"); + assert_eq!( + static_value, + gatt_uuids::STATIC_READ_VALUE, + "Static read should return [0x01, 0x02, 0x03, 0x04]" + ); + + peripheral.disconnect().await.unwrap(); +} + pub async fn test_request_connection_parameters() { use btleplug::api::ConnectionParameterPreset; diff --git a/tests/test_concurrent_connect_and_discover.rs b/tests/test_concurrent_connect_and_discover.rs new file mode 100644 index 00000000..28ed122f --- /dev/null +++ b/tests/test_concurrent_connect_and_discover.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_concurrent_connect_and_discover() { + common::test_cases::test_concurrent_connect_and_discover().await; +} diff --git a/tests/test_concurrent_operations_same_service.rs b/tests/test_concurrent_operations_same_service.rs new file mode 100644 index 00000000..1693bf27 --- /dev/null +++ b/tests/test_concurrent_operations_same_service.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_concurrent_operations_same_service() { + common::test_cases::test_concurrent_operations_same_service().await; +} diff --git a/tests/test_discover_services_during_read.rs b/tests/test_discover_services_during_read.rs new file mode 100644 index 00000000..34537337 --- /dev/null +++ b/tests/test_discover_services_during_read.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_discover_services_during_read() { + common::test_cases::test_discover_services_during_read().await; +} From 4429e11e12ec94db59050c8c9e4d8c360fac28b9 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 17:05:48 -0700 Subject: [PATCH 06/32] test: Add reconnect, connection event, and re-subscribe regression tests Cover #484 (reconnect after a peripheral-triggered disconnect without an explicit disconnect, and the connect-while-connected fast path) and #326 (re-subscribing must not duplicate notifications). Assert DeviceConnected and DeviceDisconnected in the existing connect/disconnect tests. Add event helpers to peripheral_finder: a collector that drains adapter.events() into an unbounded channel (the central event broadcast silently drops lagged events), wait_for_event, wait_for_connected (skips stale connection events BlueZ synthesises for already-connected devices), and wait_for_rediscovery for CoreBluetooth reconnects. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/AGENTS.md | 2 +- tests/android/rust/src/lib.rs | 8 + .../btleplug/test/BleIntegrationTest.kt | 2 + .../btleplug/test/NativeTests.kt | 2 + tests/common/peripheral_finder.rs | 200 +++++++++++++- tests/common/test_cases.rs | 251 ++++++++++++++---- ...t_after_peripheral_triggered_disconnect.rs | 7 + ...scribe_does_not_duplicate_notifications.rs | 7 + 8 files changed, 422 insertions(+), 57 deletions(-) create mode 100644 tests/test_reconnect_after_peripheral_triggered_disconnect.rs create mode 100644 tests/test_resubscribe_does_not_duplicate_notifications.rs diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 6098890a..d7b0bc39 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -27,7 +27,7 @@ Each test is its own file (and therefore its own binary), ensuring process isola - **Discovery**: `test_discover_*.rs`, `test_scan_*.rs`, `test_advertisement_*.rs` - **Connection**: `test_connect_*.rs`, `test_reconnect_*.rs`, `test_peripheral_triggered_*.rs` - **Read/Write**: `test_read_*.rs`, `test_write_*.rs`, `test_long_value_*.rs`, `test_characteristic_properties.rs` -- **Notifications**: `test_subscribe_*.rs`, `test_unsubscribe_*.rs`, `test_configurable_notification_*.rs` +- **Notifications**: `test_subscribe_*.rs`, `test_unsubscribe_*.rs`, `test_configurable_notification_*.rs`, `test_resubscribe_*.rs` - **Descriptors**: `test_*_descriptor*.rs` - **Device Info**: `test_mtu_*.rs`, `test_read_rssi.rs`, `test_properties_*.rs`, `test_connection_parameters.rs`, `test_request_connection_parameters.rs` diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index ad4328a6..9b24255c 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -163,6 +163,10 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testPeripheralTriggeredDisconnect, test_cases::test_peripheral_triggered_disconnect ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testReconnectAfterPeripheralTriggeredDisconnect, + test_cases::test_reconnect_after_peripheral_triggered_disconnect +); jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testReadStaticValue, test_cases::test_read_static_value @@ -211,6 +215,10 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testConfigurableNotificationPayload, test_cases::test_configurable_notification_payload ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testResubscribeDoesNotDuplicateNotifications, + test_cases::test_resubscribe_does_not_duplicate_notifications +); jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testReadOnlyDescriptor, test_cases::test_read_only_descriptor diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index 22d6ab13..1f69292d 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -80,6 +80,7 @@ class BleIntegrationTest { @Test fun testConnectAndDisconnect() = NativeTests.testConnectAndDisconnect() @Test fun testReconnectAfterDisconnect() = NativeTests.testReconnectAfterDisconnect() @Test fun testPeripheralTriggeredDisconnect() = NativeTests.testPeripheralTriggeredDisconnect() + @Test fun testReconnectAfterPeripheralTriggeredDisconnect() = NativeTests.testReconnectAfterPeripheralTriggeredDisconnect() // ── Read/Write ────────────────────────────────────────────────── @Test fun testReadStaticValue() = NativeTests.testReadStaticValue() @@ -96,6 +97,7 @@ class BleIntegrationTest { @Test fun testSubscribeAndReceiveIndications() = NativeTests.testSubscribeAndReceiveIndications() @Test fun testUnsubscribeStopsNotifications() = NativeTests.testUnsubscribeStopsNotifications() @Test fun testConfigurableNotificationPayload() = NativeTests.testConfigurableNotificationPayload() + @Test fun testResubscribeDoesNotDuplicateNotifications() = NativeTests.testResubscribeDoesNotDuplicateNotifications() // ── Descriptors ───────────────────────────────────────────────── @Test fun testReadOnlyDescriptor() = NativeTests.testReadOnlyDescriptor() diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index 5279b726..1a0731f5 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -22,6 +22,7 @@ object NativeTests { external fun testConnectAndDisconnect() external fun testReconnectAfterDisconnect() external fun testPeripheralTriggeredDisconnect() + external fun testReconnectAfterPeripheralTriggeredDisconnect() // Read/Write external fun testReadStaticValue() @@ -38,6 +39,7 @@ object NativeTests { external fun testSubscribeAndReceiveIndications() external fun testUnsubscribeStopsNotifications() external fun testConfigurableNotificationPayload() + external fun testResubscribeDoesNotDuplicateNotifications() // Descriptors external fun testReadOnlyDescriptor() diff --git a/tests/common/peripheral_finder.rs b/tests/common/peripheral_finder.rs index 9cf1dd79..3ece22d0 100644 --- a/tests/common/peripheral_finder.rs +++ b/tests/common/peripheral_finder.rs @@ -1,10 +1,11 @@ //! Helper to discover and connect to the btleplug test peripheral. -use btleplug::api::{Central, Manager as _, Peripheral as _, ScanFilter, WriteType}; -use btleplug::platform::{Adapter, Manager, Peripheral}; +use btleplug::api::{Central, CentralEvent, Manager as _, Peripheral as _, ScanFilter, WriteType}; +use btleplug::platform::{Adapter, Manager, Peripheral, PeripheralId}; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; use tokio::sync::OnceCell; +use tokio::sync::mpsc::{self, UnboundedReceiver}; use tokio::time; use super::gatt_uuids; @@ -199,3 +200,198 @@ pub fn find_characteristic( .find(|c| c.uuid == uuid) .unwrap_or_else(|| panic!("characteristic {} not found", uuid)) } + +/// Subscribe to `adapter.events()` and forward every event into an unbounded +/// channel. +/// +/// The central event broadcast channel (`src/common/adapter_manager.rs`) has +/// capacity 16 and `event_stream()` silently drops events for a receiver that +/// falls behind (`filter_map(|x| x.ok())`). During an active scan CoreBluetooth +/// emits `DeviceUpdated` for every nearby advertiser, so a receiver that isn't +/// polled continuously can lose events (in particular `DeviceConnected`) +/// between the moment it subscribes and the moment a test gets around to +/// reading it. Forwarding into an unbounded channel from a task that starts +/// draining immediately avoids that loss. The subscription (`events().await`) +/// happens before the forwarding task is spawned, so no event between the +/// caller's request and the task starting is missed. +pub async fn spawn_event_collector() -> UnboundedReceiver { + use futures::StreamExt; + + let adapter = get_adapter().await; + let mut events = adapter + .events() + .await + .expect("failed to subscribe to adapter events"); + let (tx, rx) = mpsc::unbounded_channel(); + tokio::spawn(async move { + loop { + tokio::select! { + _ = tx.closed() => break, + ev = events.next() => match ev { + Some(e) => { + if tx.send(e).is_err() { + break; + } + } + None => break, + }, + } + } + }); + rx +} + +/// Wait for an event matching `matches` to arrive on `rx`, ignoring events +/// that don't match (e.g. events for other peripherals' ids). Panics with a +/// clear message identifying `what` was being waited for if `timeout` elapses +/// first, or if the collector task has exited (the underlying event stream +/// ended). +pub async fn wait_for_event( + rx: &mut UnboundedReceiver, + timeout: Duration, + what: &str, + mut matches: F, +) -> CentralEvent +where + F: FnMut(&CentralEvent) -> bool, +{ + tokio::time::timeout(timeout, async { + loop { + match rx.recv().await { + Some(event) if matches(&event) => return event, + Some(_) => continue, + None => panic!("event collector channel closed while waiting for {}", what), + } + } + }) + .await + .unwrap_or_else(|_| panic!("timed out after {:?} waiting for {}", timeout, what)) +} + +/// Wait for the peripheral identified by `target_id` to be genuinely +/// connected, discarding a stale `DeviceConnected(target_id)` that does not +/// reflect the current connection. +/// +/// On BlueZ, `events()` synthesises a `DeviceConnected` for every +/// already-connected device the moment the stream is opened +/// (`src/bluez/adapter.rs`), and BlueZ keeps LE links alive after a process +/// exits. So after a prior test panicked mid-connection, a fresh collector +/// can see a stale `DeviceConnected(id)`, then a `DeviceDisconnected(id)` +/// from this test's own `ensure_clean_state()`, then the real +/// `DeviceConnected(id)` -- three events for what looks like one connection. +/// +/// This waits for `DeviceConnected(target_id)`, then drains everything +/// already buffered for `target_id` (after yielding once so a concurrently +/// running collector task has a chance to enqueue anything it already +/// received), keeping only the latest connection-state event seen. If that +/// latest event is a `DeviceDisconnected(target_id)`, the `DeviceConnected` +/// was stale, so it goes back to waiting for another one. It returns only +/// once the buffer is empty and the latest event for `target_id` is a +/// `DeviceConnected`. +pub async fn wait_for_connected( + rx: &mut UnboundedReceiver, + target_id: &PeripheralId, + timeout: Duration, +) { + let what = "DeviceConnected(test peripheral)"; + tokio::time::timeout(timeout, async { + loop { + // (1) Wait for a DeviceConnected(target_id). + loop { + match rx.recv().await { + Some(CentralEvent::DeviceConnected(id)) if id == *target_id => break, + Some(_) => continue, + None => panic!("event collector channel closed while waiting for {}", what), + } + } + + // Give the collector task a chance to enqueue anything it + // already received before we drain what's buffered. + tokio::task::yield_now().await; + + // (2) Drain everything currently buffered, tracking the latest + // connection-state event seen for target_id. + let mut last_was_connected = true; + loop { + match rx.try_recv() { + Ok(CentralEvent::DeviceConnected(id)) if id == *target_id => { + last_was_connected = true; + } + Ok(CentralEvent::DeviceDisconnected(id)) if id == *target_id => { + last_was_connected = false; + } + Ok(_) => continue, + Err(mpsc::error::TryRecvError::Empty) => break, + Err(mpsc::error::TryRecvError::Disconnected) => { + panic!("event collector channel closed while waiting for {}", what) + } + } + } + + // (3) A DeviceDisconnected(target_id) after the DeviceConnected + // we found means it was stale; go back to waiting for another. + if last_was_connected { + return; + } + } + }) + .await + .unwrap_or_else(|_| panic!("timed out after {:?} waiting for {}", timeout, what)) +} + +/// Wait, if needed, for a disconnected peripheral to become connectable again. +/// +/// CoreBluetooth removes a peripheral from its internal map on disconnect and +/// only re-inserts it once the background scan rediscovers it +/// (`src/corebluetooth/internal.rs` `on_peripheral_disconnect`; intentional, +/// issue #57), and `AdapterManager::emit` removes it from the public map on +/// `DeviceDisconnected`. `connect()` fails fast with "Peripheral no longer +/// available" until then. Other backends keep the peripheral connectable +/// across a disconnect, so this is a no-op there. +/// +/// `rx` must be a collector started before the disconnect. The caller may +/// already have consumed the `DeviceDisconnected` event itself before calling +/// this: once disconnected, `AdapterManager` has dropped the id from its map, +/// so rediscovery always emits `DeviceDiscovered(id)` first regardless of +/// whether `DeviceDisconnected` was observed by this call. +pub async fn wait_for_rediscovery( + rx: &mut UnboundedReceiver, + target_id: PeripheralId, +) { + #[cfg(target_vendor = "apple")] + { + // A `DeviceDiscovered(target_id)` is always fresh, since CoreBluetooth + // only emits it for an id currently absent from AdapterManager, and + // AdapterManager removes the id on `DeviceDisconnected`. A + // `DeviceUpdated(target_id)` is only trustworthy once we've also seen + // the id actually leave and re-enter the map (`DeviceDisconnected` or + // `DeviceDiscovered`), since one queued before our disconnect would + // otherwise be indistinguishable from a post-rediscovery update. + let mut seen_removed_or_discovered = false; + tokio::time::timeout(Duration::from_secs(15), async { + loop { + match rx.recv().await { + Some(CentralEvent::DeviceDiscovered(id)) if id == target_id => break, + Some(CentralEvent::DeviceDisconnected(id)) if id == target_id => { + seen_removed_or_discovered = true; + } + Some(CentralEvent::DeviceUpdated(id)) + if id == target_id && seen_removed_or_discovered => + { + break; + } + Some(_) => continue, + None => panic!("event collector channel closed while waiting for rediscovery"), + } + } + }) + .await + .expect("timed out waiting for peripheral rediscovery after disconnect"); + } + #[cfg(not(target_vendor = "apple"))] + { + // Other backends keep the peripheral connectable across a disconnect; + // nothing to wait for. + let _ = (rx, target_id); + } +} diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index a4a065f9..d0b8f0e8 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -300,15 +300,40 @@ pub async fn test_retrieve_connected_peripheral_by_service() { // ── Connection ────────────────────────────────────────────────────── pub async fn test_connect_and_disconnect() { + use btleplug::api::CentralEvent; use std::time::Duration; - use tokio::time; + + // Start the collector before find_and_connect() so the DeviceConnected + // event for this peripheral can't be lost to broadcast-channel lag + // (see peripheral_finder::spawn_event_collector). + let mut events = peripheral_finder::spawn_event_collector().await; let peripheral = peripheral_finder::find_and_connect().await; assert!(peripheral.is_connected().await.unwrap()); + let target_id = peripheral.id(); + + // On BlueZ, events() synthesises a DeviceConnected for every + // already-connected device the moment the stream opens, and BlueZ keeps + // LE links alive after a process exits -- so a prior test that panicked + // mid-connection can leave a stale DeviceConnected(id) at the head of + // this collector, followed by a DeviceDisconnected(id) from this test's + // own ensure_clean_state() and then the real DeviceConnected(id). + // wait_for_connected() discards that stale connect/disconnect pair and + // only returns once it has seen a DeviceConnected(id) with nothing after + // it but silence (see peripheral_finder::wait_for_connected). + peripheral_finder::wait_for_connected(&mut events, &target_id, Duration::from_secs(15)).await; + println!("Disconnecting"); peripheral.disconnect().await.unwrap(); println!("Disconnected"); - time::sleep(Duration::from_millis(500)).await; + + peripheral_finder::wait_for_event( + &mut events, + Duration::from_secs(15), + "DeviceDisconnected(test peripheral)", + |event| matches!(event, CentralEvent::DeviceDisconnected(id) if *id == target_id), + ) + .await; assert!(!peripheral.is_connected().await.unwrap()); println!("Waiting on is connected update?"); } @@ -330,19 +355,107 @@ pub async fn test_reconnect_after_disconnect() { } pub async fn test_peripheral_triggered_disconnect() { + use btleplug::api::CentralEvent; use std::time::Duration; - use tokio::time; + + // Start the collector before find_and_connect() so the DeviceConnected + // event for this peripheral can't be lost to broadcast-channel lag + // (see peripheral_finder::spawn_event_collector). + let mut events = peripheral_finder::spawn_event_collector().await; let peripheral = peripheral_finder::find_and_connect().await; assert!(peripheral.is_connected().await.unwrap()); + let target_id = peripheral.id(); + + // See test_connect_and_disconnect: a stale DeviceConnected/Disconnected + // pair left over from ensure_clean_state() cleaning up a prior test's + // lingering connection would make a bare DeviceConnected wait match too + // early, so the later DeviceDisconnected wait below would then match the + // buffered disconnect from that stale pair instead of the firmware's + // 500ms delayed disconnect. wait_for_connected() discards the stale pair + // (see peripheral_finder::wait_for_connected). + peripheral_finder::wait_for_connected(&mut events, &target_id, Duration::from_secs(15)).await; + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_TRIGGER_DISCONNECT).await; - time::sleep(Duration::from_secs(2)).await; + + peripheral_finder::wait_for_event( + &mut events, + Duration::from_secs(10), + "DeviceDisconnected(test peripheral)", + |event| matches!(event, CentralEvent::DeviceDisconnected(id) if *id == target_id), + ) + .await; assert!( !peripheral.is_connected().await.unwrap(), "Peripheral should have disconnected us" ); } +/// Covers #484: connect() must succeed after a peripheral-triggered +/// disconnect with no explicit disconnect() call first, and a subsequent +/// connect() while already connected must return Ok promptly (the fast path). +pub async fn test_reconnect_after_peripheral_triggered_disconnect() { + use btleplug::api::CentralEvent; + use std::time::Duration; + + let peripheral = peripheral_finder::find_and_connect().await; + peripheral_finder::reset_peripheral(&peripheral).await; + assert!(peripheral.is_connected().await.unwrap()); + let target_id = peripheral.id(); + + // Start the collector after reset_peripheral() -- only the disconnect + // and rediscovery events matter here, and reset_peripheral() doesn't + // disconnect. + let mut events = peripheral_finder::spawn_event_collector().await; + + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_TRIGGER_DISCONNECT).await; + + peripheral_finder::wait_for_event( + &mut events, + Duration::from_secs(10), + "DeviceDisconnected(test peripheral)", + |event| matches!(event, CentralEvent::DeviceDisconnected(id) if *id == target_id), + ) + .await; + assert!(!peripheral.is_connected().await.unwrap()); + + peripheral_finder::wait_for_rediscovery(&mut events, target_id).await; + + // #484: connect() without an explicit disconnect() first. + tokio::time::timeout(Duration::from_secs(10), peripheral.connect()) + .await + .expect("connect() after peripheral-triggered disconnect timed out") + .expect("connect() after peripheral-triggered disconnect failed"); + assert!(peripheral.is_connected().await.unwrap()); + + tokio::time::timeout(Duration::from_secs(10), peripheral.discover_services()) + .await + .expect("discover_services() timed out") + .expect("discover_services() failed"); + + let char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::STATIC_READ); + let value = tokio::time::timeout(Duration::from_secs(10), peripheral.read(&char)) + .await + .expect("read(STATIC_READ) timed out") + .expect("read(STATIC_READ) failed"); + assert_eq!( + value, + gatt_uuids::STATIC_READ_VALUE, + "Static read should return [0x01, 0x02, 0x03, 0x04]" + ); + + // #484 fast path: connect() while already connected must return Ok + // within 2 seconds instead of re-running the full connection sequence. + let result = tokio::time::timeout(Duration::from_secs(2), peripheral.connect()).await; + assert!( + matches!(result, Ok(Ok(()))), + "connect() while already connected did not return Ok within 2s: {:?}", + result + ); + + peripheral.disconnect().await.unwrap(); +} + // ── Read / Write ──────────────────────────────────────────────────── pub async fn test_read_static_value() { @@ -692,6 +805,79 @@ pub async fn test_configurable_notification_payload() { peripheral.disconnect().await.unwrap(); } +/// Covers #326: subscribing to the same characteristic twice must not +/// register a duplicate notification handler, which would deliver every +/// notification more than once. +pub async fn test_resubscribe_does_not_duplicate_notifications() { + use futures::StreamExt; + use std::collections::HashSet; + use std::time::Duration; + use tokio::time; + + let peripheral = peripheral_finder::find_and_connect().await; + peripheral_finder::reset_peripheral(&peripheral).await; + let char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::NOTIFY_CHAR); + + tokio::time::timeout(Duration::from_secs(10), peripheral.subscribe(&char)) + .await + .expect("first subscribe(NOTIFY_CHAR) timed out") + .expect("first subscribe(NOTIFY_CHAR) should succeed"); + tokio::time::timeout(Duration::from_secs(10), peripheral.subscribe(&char)) + .await + .expect("second subscribe(NOTIFY_CHAR) timed out") + .expect("second subscribe(NOTIFY_CHAR) should succeed"); + + // Take the notification stream before starting notifications so nothing + // is missed. + let mut stream = peripheral.notifications().await.unwrap(); + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_START_NOTIFICATIONS).await; + + let mut received = Vec::new(); + let timeout = time::sleep(Duration::from_secs(4)); + tokio::pin!(timeout); + loop { + tokio::select! { + Some(notification) = stream.next() => { + if notification.uuid == gatt_uuids::NOTIFY_CHAR { + received.push(notification.value); + } + } + _ = &mut timeout => break, + } + } + + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_STOP_NOTIFICATIONS).await; + peripheral.unsubscribe(&char).await.unwrap(); + + assert!( + received.len() >= 3, + "Expected at least 3 notifications, got {}", + received.len() + ); + + // NOTIFY_CHAR's payload is a single incrementing counter byte + // (test-peripheral/zephyr/src/control_service.c periodic_notify_handler: + // `static uint8_t counter; uint8_t data[] = {counter++};`). If resubscribe + // duplicated the notification handler, the same counter value would + // appear more than once in this collection window. + let mut seen = HashSet::new(); + for value in &received { + assert_eq!( + value.len(), + 1, + "NOTIFY_CHAR payload should be a single counter byte, got {:?}", + value + ); + assert!( + seen.insert(value[0]), + "counter byte {} repeated -- resubscribe duplicated the notification handler", + value[0] + ); + } + + peripheral.disconnect().await.unwrap(); +} + // ── Descriptors ───────────────────────────────────────────────────── pub async fn test_read_only_descriptor() { @@ -859,60 +1045,17 @@ pub async fn test_concurrent_connect_and_discover() { let peripheral = peripheral_finder::find_and_connect().await; assert!(peripheral.is_connected().await.unwrap()); - #[cfg(target_vendor = "apple")] - let mut events = { - use btleplug::api::Central; - peripheral_finder::get_adapter() - .await - .events() - .await - .unwrap() - }; + // Start the collector before disconnect() so nothing is lost while it is + // awaited (see peripheral_finder::spawn_event_collector). + let mut events = peripheral_finder::spawn_event_collector().await; peripheral.disconnect().await.unwrap(); assert!(!peripheral.is_connected().await.unwrap()); - // CoreBluetooth drops the peripheral from its internal map on disconnect - // and only re-inserts it once the background scan rediscovers it; wait - // for that rediscovery before attempting to reconnect (see - // src/corebluetooth/internal.rs on_peripheral_disconnect/on_discovered_peripheral). - // Other backends keep the peripheral connectable across disconnects, so - // they go straight to the concurrent connect below. - #[cfg(target_vendor = "apple")] - { - use btleplug::api::CentralEvent; - use futures::StreamExt; - - // CoreBluetooth only emits DeviceDiscovered for an id that is - // currently absent from AdapterManager, and AdapterManager removes - // the peripheral on DeviceDisconnected (src/common/adapter_manager.rs - // emit()), so a DeviceDiscovered(target_id) is always fresh. A - // DeviceUpdated(target_id) is only trustworthy once we've also seen - // the id actually leave and re-enter the map (DeviceDisconnected or - // DeviceDiscovered), since one queued before our disconnect would - // otherwise be indistinguishable from a post-rediscovery update. - let target_id = peripheral.id(); - let mut seen_removed_or_discovered = false; - timeout(Duration::from_secs(15), async { - loop { - match events.next().await { - Some(CentralEvent::DeviceDiscovered(id)) if id == target_id => break, - Some(CentralEvent::DeviceDisconnected(id)) if id == target_id => { - seen_removed_or_discovered = true; - } - Some(CentralEvent::DeviceUpdated(id)) - if id == target_id && seen_removed_or_discovered => - { - break; - } - Some(_) => continue, - None => panic!("adapter event stream ended while waiting for rediscovery"), - } - } - }) - .await - .expect("timed out waiting for peripheral rediscovery after disconnect"); - } + // On macOS, wait for the peripheral to be rediscovered before attempting + // to reconnect; other backends keep it connectable across disconnects and + // this is a no-op (see peripheral_finder::wait_for_rediscovery). + peripheral_finder::wait_for_rediscovery(&mut events, peripheral.id()).await; // Concurrent connect (#488). let results = timeout(Duration::from_secs(15), async { diff --git a/tests/test_reconnect_after_peripheral_triggered_disconnect.rs b/tests/test_reconnect_after_peripheral_triggered_disconnect.rs new file mode 100644 index 00000000..c2751f0c --- /dev/null +++ b/tests/test_reconnect_after_peripheral_triggered_disconnect.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_reconnect_after_peripheral_triggered_disconnect() { + common::test_cases::test_reconnect_after_peripheral_triggered_disconnect().await; +} diff --git a/tests/test_resubscribe_does_not_duplicate_notifications.rs b/tests/test_resubscribe_does_not_duplicate_notifications.rs new file mode 100644 index 00000000..1f238f70 --- /dev/null +++ b/tests/test_resubscribe_does_not_duplicate_notifications.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_resubscribe_does_not_duplicate_notifications() { + common::test_cases::test_resubscribe_does_not_duplicate_notifications().await; +} From dd75aea34388df24269b8d1bd3981b2528f0a8d9 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 17:35:09 -0700 Subject: [PATCH 07/32] test: Add disconnect-during-operation regression test Cover #490: a peripheral-triggered disconnect while operations are in flight must fail those operations promptly, emit DeviceDisconnected, and leave the command queue usable after reconnecting. The test runs two disconnect cycles so the in-flight operation is deterministic: descriptor reads only (the #490 path), then characteristic reads only. Every operation is bounded by a timeout, and Android must report Error::NotConnected for reads after disconnect. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/android/rust/src/lib.rs | 4 + .../btleplug/test/BleIntegrationTest.kt | 1 + .../btleplug/test/NativeTests.kt | 1 + tests/common/test_cases.rs | 194 ++++++++++++++++++ ..._across_peripheral_triggered_disconnect.rs | 7 + 5 files changed, 207 insertions(+) create mode 100644 tests/test_operations_across_peripheral_triggered_disconnect.rs diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index 9b24255c..296ea495 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -263,3 +263,7 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testDiscoverServicesDuringRead, test_cases::test_discover_services_during_read ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testOperationsAcrossPeripheralTriggeredDisconnect, + test_cases::test_operations_across_peripheral_triggered_disconnect +); diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index 1f69292d..1af06df6 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -115,4 +115,5 @@ class BleIntegrationTest { @Test fun testConcurrentConnectAndDiscover() = NativeTests.testConcurrentConnectAndDiscover() @Test fun testConcurrentOperationsSameService() = NativeTests.testConcurrentOperationsSameService() @Test fun testDiscoverServicesDuringRead() = NativeTests.testDiscoverServicesDuringRead() + @Test fun testOperationsAcrossPeripheralTriggeredDisconnect() = NativeTests.testOperationsAcrossPeripheralTriggeredDisconnect() } diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index 1a0731f5..1df4b8af 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -57,4 +57,5 @@ object NativeTests { external fun testConcurrentConnectAndDiscover() external fun testConcurrentOperationsSameService() external fun testDiscoverServicesDuringRead() + external fun testOperationsAcrossPeripheralTriggeredDisconnect() } diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index d0b8f0e8..8c0a9fe9 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -1204,6 +1204,200 @@ pub async fn test_discover_services_during_read() { peripheral.disconnect().await.unwrap(); } +/// Covers #490: a peripheral-triggered disconnect that lands mid-operation +/// must not wedge the command queue or hide the `DeviceDisconnected` event. +/// +/// This branch also fixed CoreBluetooth replying to pending operations when a +/// peripheral goes away (`f3de711`, `src/corebluetooth/internal.rs`), so a +/// timeout anywhere in this test is a real regression signal on every +/// platform, not just Android -- it must never be relaxed away. +pub async fn test_operations_across_peripheral_triggered_disconnect() { + use btleplug::api::CentralEvent; + use std::time::{Duration, Instant}; + + let peripheral = peripheral_finder::find_and_connect().await; + peripheral_finder::reset_peripheral(&peripheral).await; + assert!(peripheral.is_connected().await.unwrap()); + let target_id = peripheral.id(); + + // Start the collector before sending the first 0x03 so the + // DeviceDisconnected event can't be lost (see + // peripheral_finder::spawn_event_collector). Reused for both disconnect + // cycles; the buffer is drained before cycle 2's 0x03 so no leftover + // cycle-1 event can satisfy a cycle-2 wait. + let mut events = peripheral_finder::spawn_event_collector().await; + + let descriptor = super::find_descriptor( + &peripheral, + gatt_uuids::DESCRIPTOR_TEST_CHAR, + gatt_uuids::READ_ONLY_DESCRIPTOR, + ); + let counter_char = + peripheral_finder::find_characteristic(&peripheral, gatt_uuids::COUNTER_READ); + let static_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::STATIC_READ); + + // Repeats a single kind of read operation, each bounded by a 5 second + // timeout, until one returns Err (the firmware disconnects ~500ms after + // 0x03). No operation may time out -- that would mean the disconnect + // never surfaced to a pending command. The whole loop is also bounded to + // ~10s so a regression that swallows the error forever fails instead of + // hanging. Restricting each cycle to one op kind (instead of alternating) + // makes it unambiguous which op the disconnect actually caught. + async fn run_until_disconnect(cycle: &str, op_name: &str, mut op: F) -> E + where + F: FnMut() -> Fut, + Fut: std::future::Future>, + E: std::fmt::Debug, + { + let loop_deadline = Instant::now() + Duration::from_secs(10); + let mut success_count = 0u32; + let mut iteration = 0u32; + loop { + iteration += 1; + if Instant::now() >= loop_deadline { + panic!( + "{cycle}: no {op_name} call returned Err after {success_count} successful \ + calls within the 10s bound -- the peripheral-triggered disconnect (0x03) \ + never surfaced to a pending command" + ); + } + + let result = tokio::time::timeout(Duration::from_secs(5), op()) + .await + .unwrap_or_else(|_| { + panic!( + "{cycle}: {op_name} timed out on iteration {iteration} (after \ + {success_count} prior successful calls) -- disconnect mid-operation \ + wedged the command queue" + ) + }); + + match result { + Ok(_) => success_count += 1, + Err(e) => { + println!( + "{cycle}: {op_name} failed on iteration {iteration} (after \ + {success_count} prior successful calls) with error: {e:?}" + ); + return e; + } + } + } + } + + // Cycle 1: only descriptor reads are ever in flight, so the disconnect + // that ends the loop is unambiguously caught by a descriptor read. + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_TRIGGER_DISCONNECT).await; + let _ = run_until_disconnect("cycle 1", "read_descriptor(READ_ONLY_DESCRIPTOR)", || { + peripheral.read_descriptor(&descriptor) + }) + .await; + + peripheral_finder::wait_for_event( + &mut events, + Duration::from_secs(10), + "DeviceDisconnected(test peripheral) [cycle 1]", + |event| matches!(event, CentralEvent::DeviceDisconnected(id) if *id == target_id), + ) + .await; + assert!( + !peripheral.is_connected().await.unwrap(), + "peripheral should be disconnected after cycle 1" + ); + + // One more read after disconnection must return Err promptly, not hang -- + // proof the queue isn't wedged and the disconnect is visible to new ops. + // Checked once here; cycle 2 below re-proves the queue survives a second + // disconnect via its own reconnect + STATIC_READ at the end. + let post_disconnect_result = tokio::time::timeout( + Duration::from_secs(2), + peripheral.read(&counter_char), + ) + .await + .expect("read(COUNTER_READ) after disconnection timed out -- command queue is wedged (#490)"); + // Peripheral.java clears `connected` under its lock (~L551) before + // `adapter.onConnectionStateChanged` (~L564) emits `DeviceDisconnected`, so + // `read()`'s `!this.connected` guard (~L198) throws `NotConnectedException`, + // mapped by `get_poll_result` (`src/droidplug/peripheral.rs` ~L92) to `Error::NotConnected`. + #[cfg(target_os = "android")] + assert!( + matches!(post_disconnect_result, Err(btleplug::Error::NotConnected)), + "expected Error::NotConnected reading after disconnection on Android, got {:?}", + post_disconnect_result + ); + #[cfg(not(target_os = "android"))] + assert!( + post_disconnect_result.is_err(), + "expected an error reading after disconnection, got {:?}", + post_disconnect_result + ); + + // Reconnect, rediscover, and reset before cycle 2. + peripheral_finder::wait_for_rediscovery(&mut events, target_id.clone()).await; + + tokio::time::timeout(Duration::from_secs(10), peripheral.connect()) + .await + .expect("connect() after cycle 1 disconnect timed out") + .expect("connect() after cycle 1 disconnect failed"); + assert!(peripheral.is_connected().await.unwrap()); + + tokio::time::timeout(Duration::from_secs(10), peripheral.discover_services()) + .await + .expect("discover_services() after cycle 1 timed out") + .expect("discover_services() after cycle 1 failed"); + + peripheral_finder::reset_peripheral(&peripheral).await; + + // Cycle 2: only characteristic reads are ever in flight, so the + // disconnect that ends the loop is unambiguously caught by a + // characteristic read. + while events.try_recv().is_ok() {} + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_TRIGGER_DISCONNECT).await; + let _ = run_until_disconnect("cycle 2", "read(COUNTER_READ)", || { + peripheral.read(&counter_char) + }) + .await; + + peripheral_finder::wait_for_event( + &mut events, + Duration::from_secs(10), + "DeviceDisconnected(test peripheral) [cycle 2]", + |event| matches!(event, CentralEvent::DeviceDisconnected(id) if *id == target_id), + ) + .await; + assert!( + !peripheral.is_connected().await.unwrap(), + "peripheral should be disconnected after cycle 2" + ); + + // Reconnect, rediscover, and read STATIC_READ to prove the command queue + // is not wedged. + peripheral_finder::wait_for_rediscovery(&mut events, target_id).await; + + tokio::time::timeout(Duration::from_secs(10), peripheral.connect()) + .await + .expect("connect() after cycle 2 disconnect timed out") + .expect("connect() after cycle 2 disconnect failed"); + assert!(peripheral.is_connected().await.unwrap()); + + tokio::time::timeout(Duration::from_secs(10), peripheral.discover_services()) + .await + .expect("discover_services() after cycle 2 timed out") + .expect("discover_services() after cycle 2 failed"); + + let value = tokio::time::timeout(Duration::from_secs(10), peripheral.read(&static_char)) + .await + .expect("read(STATIC_READ) timed out") + .expect("read(STATIC_READ) failed"); + assert_eq!( + value, + gatt_uuids::STATIC_READ_VALUE, + "Static read should return [0x01, 0x02, 0x03, 0x04]" + ); + + peripheral.disconnect().await.unwrap(); +} + pub async fn test_request_connection_parameters() { use btleplug::api::ConnectionParameterPreset; diff --git a/tests/test_operations_across_peripheral_triggered_disconnect.rs b/tests/test_operations_across_peripheral_triggered_disconnect.rs new file mode 100644 index 00000000..c0c458a0 --- /dev/null +++ b/tests/test_operations_across_peripheral_triggered_disconnect.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_operations_across_peripheral_triggered_disconnect() { + common::test_cases::test_operations_across_peripheral_triggered_disconnect().await; +} From f4034b2745bc5f968104a9b9617259356f385a4e Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 17:45:55 -0700 Subject: [PATCH 08/32] test: Cover add_peripheral and peripheral retrieval by identifier add_peripheral() must produce a usable handle on Windows and Android and return Error::NotSupported on BlueZ and CoreBluetooth. Retrieval by identifier must return the connected test peripheral on desktop backends. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/AGENTS.md | 2 +- tests/android/rust/src/lib.rs | 4 + .../btleplug/test/BleIntegrationTest.kt | 1 + .../btleplug/test/NativeTests.kt | 1 + tests/common/test_cases.rs | 97 +++++++++++++++++++ tests/test_add_peripheral_by_address.rs | 7 ++ ...ieve_connected_peripheral_by_identifier.rs | 8 ++ 7 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 tests/test_add_peripheral_by_address.rs create mode 100644 tests/test_retrieve_connected_peripheral_by_identifier.rs diff --git a/tests/AGENTS.md b/tests/AGENTS.md index d7b0bc39..fa4db3a3 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -51,5 +51,5 @@ Each test is its own file (and therefore its own binary), ensuring process isola - One test per file ensures process isolation — never put multiple tests in the same file. - Tests must not depend on execution order; each test connects independently. - The scan timeout is 15 seconds (hardcoded in `peripheral_finder.rs`). -- When adding a new test, also add the corresponding JNI export in `android/rust/src/lib.rs`, native declaration in `NativeTests.kt`, and `@Test` in `BleIntegrationTest.kt`. Tests backed by APIs Android does not support (e.g. `test_retrieve_connected_peripheral_by_service`) and macOS-only tests intentionally have no JNI export. +- When adding a new test, also add the corresponding JNI export in `android/rust/src/lib.rs`, native declaration in `NativeTests.kt`, and `@Test` in `BleIntegrationTest.kt`. Tests backed by APIs Android does not support (e.g. `test_retrieve_connected_peripheral_by_service`, `test_retrieve_connected_peripheral_by_identifier`) and macOS-only tests intentionally have no JNI export. - Adapter-only tests require local adapter hardware but do not require the btleplug test peripheral; their desktop assertions are target-specific because CoreBluetooth and ordinary Android intentionally return `Ok(None)`. diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index 296ea495..254a99ed 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -267,3 +267,7 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testOperationsAcrossPeripheralTriggeredDisconnect, test_cases::test_operations_across_peripheral_triggered_disconnect ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testAddPeripheralByAddress, + test_cases::test_add_peripheral_by_address +); diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index 1af06df6..c1de6d54 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -116,4 +116,5 @@ class BleIntegrationTest { @Test fun testConcurrentOperationsSameService() = NativeTests.testConcurrentOperationsSameService() @Test fun testDiscoverServicesDuringRead() = NativeTests.testDiscoverServicesDuringRead() @Test fun testOperationsAcrossPeripheralTriggeredDisconnect() = NativeTests.testOperationsAcrossPeripheralTriggeredDisconnect() + @Test fun testAddPeripheralByAddress() = NativeTests.testAddPeripheralByAddress() } diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index 1df4b8af..e33a16f7 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -58,4 +58,5 @@ object NativeTests { external fun testConcurrentOperationsSameService() external fun testDiscoverServicesDuringRead() external fun testOperationsAcrossPeripheralTriggeredDisconnect() + external fun testAddPeripheralByAddress() } diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 8c0a9fe9..13445a29 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -275,6 +275,103 @@ pub async fn test_retrieve_peripherals_not_supported() { )); } +/// Windows and Android construct a `Peripheral` from a bare id, so the returned handle must be +/// usable. BlueZ and CoreBluetooth return `Error::NotSupported`; only the variant is asserted +/// because its message does not follow the operation-name convention `retrieve_peripherals` uses. +pub async fn test_add_peripheral_by_address() { + use btleplug::api::{Central, Peripheral as _}; + use std::time::Duration; + + let peripheral = peripheral_finder::find_and_connect().await; + let id = peripheral.id(); + let adapter = peripheral_finder::get_adapter().await; + + #[cfg(any(target_os = "linux", target_vendor = "apple"))] + { + let result = tokio::time::timeout(Duration::from_secs(10), adapter.add_peripheral(&id)) + .await + .expect("add_peripheral() timed out"); + assert!( + matches!(result, Err(btleplug::Error::NotSupported(_))), + "expected Error::NotSupported from add_peripheral(), got {:?}", + result + ); + peripheral.disconnect().await.unwrap(); + } + + #[cfg(any(target_os = "windows", target_os = "android"))] + { + peripheral.disconnect().await.unwrap(); + + // The background scan may re-add the peripheral before add_peripheral() runs; both + // backends return an existing map entry before constructing one, so either way the + // returned handle is valid. + adapter.clear_peripherals().await.unwrap(); + + let added = tokio::time::timeout(Duration::from_secs(10), adapter.add_peripheral(&id)) + .await + .expect("add_peripheral() timed out") + .expect("add_peripheral() should succeed"); + assert_eq!( + added.id(), + id, + "add_peripheral() returned a peripheral with a different id" + ); + + tokio::time::timeout(Duration::from_secs(10), added.connect()) + .await + .expect("connect() on add_peripheral()'s handle timed out") + .expect("connect() on add_peripheral()'s handle failed"); + assert!(added.is_connected().await.unwrap()); + + tokio::time::timeout(Duration::from_secs(10), added.discover_services()) + .await + .expect("discover_services() on add_peripheral()'s handle timed out") + .expect("discover_services() on add_peripheral()'s handle failed"); + + let char = peripheral_finder::find_characteristic(&added, gatt_uuids::STATIC_READ); + let value = tokio::time::timeout(Duration::from_secs(10), added.read(&char)) + .await + .expect("read(STATIC_READ) on add_peripheral()'s handle timed out") + .expect("read(STATIC_READ) on add_peripheral()'s handle failed"); + assert_eq!( + value, + gatt_uuids::STATIC_READ_VALUE, + "Static read should return [0x01, 0x02, 0x03, 0x04]" + ); + + added.disconnect().await.unwrap(); + } +} + +/// Android does not support retrieval (covered by `test_retrieve_peripherals_not_supported`). +#[cfg(not(target_os = "android"))] +pub async fn test_retrieve_connected_peripheral_by_identifier() { + use btleplug::api::{Central, Peripheral as _, RetrievePeripheralsOptions}; + + let adapter = peripheral_finder::get_adapter().await; + let expected = peripheral_finder::find_and_connect().await; + let expected_id = expected.id(); + + let retrieved = adapter + .retrieve_peripherals(RetrievePeripheralsOptions { + identifiers: Some(vec![expected_id.clone()]), + services: None, + }) + .await + .expect("retrieval by identifier should be supported on desktop backends"); + let matched = retrieved + .iter() + .find(|peripheral| peripheral.id() == expected_id) + .expect("connected test peripheral was not returned by identifier retrieval"); + assert!( + matched.is_connected().await.unwrap(), + "retrieved peripheral should report connected" + ); + + expected.disconnect().await.unwrap(); +} + pub async fn test_retrieve_connected_peripheral_by_service() { use btleplug::api::{Central, RetrievePeripheralsOptions}; diff --git a/tests/test_add_peripheral_by_address.rs b/tests/test_add_peripheral_by_address.rs new file mode 100644 index 00000000..3235f034 --- /dev/null +++ b/tests/test_add_peripheral_by_address.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_add_peripheral_by_address() { + common::test_cases::test_add_peripheral_by_address().await; +} diff --git a/tests/test_retrieve_connected_peripheral_by_identifier.rs b/tests/test_retrieve_connected_peripheral_by_identifier.rs new file mode 100644 index 00000000..c0874e41 --- /dev/null +++ b/tests/test_retrieve_connected_peripheral_by_identifier.rs @@ -0,0 +1,8 @@ +mod common; + +#[cfg(not(target_os = "android"))] +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_retrieve_connected_peripheral_by_identifier() { + common::test_cases::test_retrieve_connected_peripheral_by_identifier().await; +} From 05b1922ae140bc47a7ee41c3005b32fcdeb518ad Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 18:37:49 -0700 Subject: [PATCH 09/32] fix(corebluetooth): Ignore advertisement callbacks after scanning stops Delegate callbacks and adapter messages reach the internal actor on separate channels multiplexed with select!, so a didDiscoverPeripheral callback delivered before stop_scan() could be processed after a later clear_peripherals() and re-add the peripheral. Track whether a scan is active and drop discovery and advertisement-data delegate events while it is not; StopScanning and ClearPeripherals share the message channel, so nothing processed after a clear can resurrect an entry. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 5 + src/corebluetooth/internal.rs | 20 +++ src/corebluetooth/internal/tests.rs | 241 +++++++++++++++++++++++++++- 3 files changed, 264 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afde4d02..5f202a39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,11 @@ - Fix failed Android GATT reads and writes being reported as success. Failures, such as insufficient authentication, are now returned as errors that include the GATT status code. (#492) +- Fix CoreBluetooth discovery callbacks queued before `stop_scan()` re-adding + peripherals after `clear_peripherals()`, so `Central::peripherals()` could + still return a device (and `DeviceDiscovered` could fire) immediately after + stopping the scan and clearing. Advertisement callbacks that arrive after + `stop_scan()` are now ignored. # 0.13.2 (2026-09-20) diff --git a/src/corebluetooth/internal.rs b/src/corebluetooth/internal.rs index 3a25c749..f32650c9 100644 --- a/src/corebluetooth/internal.rs +++ b/src/corebluetooth/internal.rs @@ -719,6 +719,12 @@ struct CoreBluetoothInternal { // task::block this when sending even though it'll never actually block. event_sender: Sender, message_receiver: Fuse>, + // Gates DiscoveredPeripheral and advertisement-derived (ManufacturerData, + // ServiceData, Services, TxPowerLevel) delegate events: CBqueue callbacks + // queued before stopScan can still arrive after StopScanning/ + // ClearPeripherals are processed, so we drop them instead of re-adding a + // peripheral or reporting stale advertisement data. + scanning: bool, } impl Debug for CoreBluetoothInternal { @@ -730,6 +736,7 @@ impl Debug for CoreBluetoothInternal { .field("delegate_receiver", &self.delegate_receiver) .field("event_sender", &self.event_sender) .field("message_receiver", &self.message_receiver) + .field("scanning", &self.scanning) .finish() } } @@ -874,6 +881,7 @@ impl CoreBluetoothInternal { event_sender, message_receiver: message_receiver.fuse(), delegate, + scanning: false, } } @@ -1121,6 +1129,7 @@ impl CoreBluetoothInternal { async fn on_adapter_powered_off(&mut self) { warn!("Adapter powered off, canceling all pending operations"); + self.scanning = false; let peripheral_uuids: Vec = self.peripherals.keys().cloned().collect(); for uuid in peripheral_uuids { if let Err(e) = self @@ -1763,6 +1772,15 @@ impl CoreBluetoothInternal { } self.dispatch_event(CoreBluetoothEvent::DidUpdateState{state}).await } + CentralDelegateEvent::DiscoveredPeripheral{..} + | CentralDelegateEvent::ManufacturerData{..} + | CentralDelegateEvent::ServiceData{..} + | CentralDelegateEvent::Services{..} + | CentralDelegateEvent::TxPowerLevel{..} + if !self.scanning => + { + trace!("Ignoring discovery/advertisement delegate event while not scanning"); + } CentralDelegateEvent::DiscoveredPeripheral{cbperipheral, advertisement_name} => { self.on_discovered_peripheral(cbperipheral, advertisement_name).await } @@ -1920,6 +1938,7 @@ impl CoreBluetoothInternal { fn start_discovery(&mut self, filter: ScanFilter) { trace!("BluetoothAdapter::start_discovery"); + self.scanning = true; let service_uuids = scan_filter_to_service_uuids(filter); let options: Retained> = NSMutableDictionary::new(); @@ -1939,6 +1958,7 @@ impl CoreBluetoothInternal { fn stop_discovery(&mut self) { trace!("BluetoothAdapter::stop_discovery"); + self.scanning = false; unsafe { self.manager.stopScan() }; } } diff --git a/src/corebluetooth/internal/tests.rs b/src/corebluetooth/internal/tests.rs index ee13c513..fe16e344 100644 --- a/src/corebluetooth/internal/tests.rs +++ b/src/corebluetooth/internal/tests.rs @@ -2,8 +2,8 @@ use super::*; use futures::StreamExt; use objc2::{DefinedClass, define_class}; use objc2_core_bluetooth::{ - CBAttributePermissions, CBMutableCharacteristic, CBMutableDescriptor, CBMutableService, - CBPeripheralDelegate, + CBAdvertisementDataManufacturerDataKey, CBAttributePermissions, CBCentralManagerDelegate, + CBMutableCharacteristic, CBMutableDescriptor, CBMutableService, CBPeripheralDelegate, }; use objc2_foundation::{NSError, NSObjectProtocol, NSString, ns_string}; use std::sync::{ @@ -1995,3 +1995,240 @@ async fn remove_services_drains_matching_write_without_response_queue() { std::mem::forget(internal); std::mem::forget(peripheral); } + +/// Deliver a real `centralManager:didDiscoverPeripheral:advertisementData:RSSI:` +/// callback on `internal`'s own delegate/manager, exactly as CBqueue would. +/// +/// When `manufacturer_data` is given, the advertisement dictionary also +/// carries a `CBAdvertisementDataManufacturerDataKey` entry, so the same +/// callback can be used to exercise gating of the advertisement-derived +/// events (`ManufacturerData`, `ServiceData`, `Services`, `TxPowerLevel`) +/// that `centralManager:didDiscoverPeripheral:...` also produces. +fn deliver_discovered_peripheral( + internal: &CoreBluetoothInternal, + peripheral: &TestPeripheral, + manufacturer_data: Option<(u16, &[u8])>, +) { + let adv_data: Retained> = NSMutableDictionary::new(); + if let Some((manufacturer_id, data)) = manufacturer_data { + let mut bytes = manufacturer_id.to_le_bytes().to_vec(); + bytes.extend_from_slice(data); + adv_data.insert( + unsafe { CBAdvertisementDataManufacturerDataKey }, + &*Retained::into_super(NSData::from_vec(bytes)), + ); + } + let rssi = NSNumber::new_i16(-50); + unsafe { + internal + .delegate + .centralManager_didDiscoverPeripheral_advertisementData_RSSI( + &internal.manager, + peripheral, + &adv_data, + &rssi, + ); + } +} + +fn test_peripheral_with_uuid(uuid: Uuid) -> Retained { + let uuid_string = NSString::from_str(&uuid.to_string()); + let identifier = NSUUID::initWithUUIDString(NSUUID::alloc(), &uuid_string).expect("valid UUID"); + TestPeripheral::new(identifier) +} + +/// Drain `internal`'s delegate/message channels by repeatedly calling +/// `wait_for_message()` under a short timeout until one times out (i.e. +/// nothing is pending any more). `CBCentralManager`'s +/// `initWithDelegate:queue:` posts an initial `centralManagerDidUpdateState:` +/// on CBqueue that can otherwise land in the delegate channel at an +/// unpredictable time relative to an event a test injects directly, so tests +/// settle the channel before and after injecting to stay timing-independent. +async fn settle(internal: &mut CoreBluetoothInternal) { + loop { + if tokio::time::timeout(Duration::from_millis(200), internal.wait_for_message()) + .await + .is_err() + { + break; + } + } +} + +/// Drain every event currently buffered on `event_receiver` without blocking. +fn drain_events( + event_receiver: &mut mpsc::Receiver, +) -> Vec { + let mut events = Vec::new(); + while let Ok(event) = event_receiver.try_recv() { + events.push(event); + } + events +} + +#[tokio::test] +#[ignore = "requires CoreBluetooth (creates a real CBCentralManager)"] +async fn discovered_peripheral_is_ignored_while_not_scanning() { + let (_msg_sender, msg_receiver) = mpsc::channel::(4); + let (event_sender, mut event_receiver) = mpsc::channel::(4); + let mut internal = CoreBluetoothInternal::new(msg_receiver, event_sender); + settle(&mut internal).await; + assert!(!internal.scanning, "internal must start out not scanning"); + + // Simulate a DiscoveredPeripheral callback that was queued on CBqueue + // before stop_discovery() took effect (or that arrives with no scan + // ever started). + let uuid = Uuid::from_u128(0x22222222_2222_2222_2222_222222222222); + let peripheral = test_peripheral_with_uuid(uuid); + deliver_discovered_peripheral(&internal, &peripheral, None); + settle(&mut internal).await; + + let peripherals_empty = internal.peripherals.is_empty(); + let device_discovered = drain_events(&mut event_receiver) + .into_iter() + .find(|event| matches!(event, CoreBluetoothEvent::DeviceDiscovered { .. })); + + std::mem::forget(internal); + std::mem::forget(peripheral); + + assert!( + peripherals_empty, + "a DiscoveredPeripheral event received while not scanning must not add a peripheral" + ); + assert!( + device_discovered.is_none(), + "no DeviceDiscovered event should have been dispatched, got {device_discovered:?}" + ); +} + +#[tokio::test] +#[ignore = "requires CoreBluetooth (creates a real CBCentralManager)"] +async fn discovered_peripheral_is_processed_while_scanning() { + let (_msg_sender, msg_receiver) = mpsc::channel::(4); + let (event_sender, mut event_receiver) = mpsc::channel::(4); + let mut internal = CoreBluetoothInternal::new(msg_receiver, event_sender); + settle(&mut internal).await; + // Set after settling: a PoweredOff initial state would otherwise clear + // `scanning` right back to false via on_adapter_powered_off(). + internal.scanning = true; + + let uuid = Uuid::from_u128(0x33333333_3333_3333_3333_333333333333); + let peripheral = test_peripheral_with_uuid(uuid); + deliver_discovered_peripheral(&internal, &peripheral, None); + settle(&mut internal).await; + + let peripheral_known = internal.peripherals.contains_key(&uuid); + let device_discovered = drain_events(&mut event_receiver) + .into_iter() + .find(|event| matches!(event, CoreBluetoothEvent::DeviceDiscovered { .. })); + + std::mem::forget(internal); + std::mem::forget(peripheral); + + assert!( + peripheral_known, + "a DiscoveredPeripheral event received while scanning must add the peripheral" + ); + match device_discovered { + Some(CoreBluetoothEvent::DeviceDiscovered { + uuid: event_uuid, .. + }) => { + assert_eq!( + event_uuid, uuid, + "DeviceDiscovered event was for the wrong peripheral" + ); + } + other => panic!("expected a DeviceDiscovered event, got {other:?}"), + } +} + +#[tokio::test] +#[ignore = "requires CoreBluetooth (creates a real CBCentralManager)"] +async fn stop_discovery_causes_late_discovered_peripheral_to_be_ignored() { + let (_msg_sender, msg_receiver) = mpsc::channel::(4); + let (event_sender, mut event_receiver) = mpsc::channel::(4); + let mut internal = CoreBluetoothInternal::new(msg_receiver, event_sender); + settle(&mut internal).await; + + // Mirrors the regression scenario: a scan was running (so this + // callback was legitimately queued on CBqueue), stop_discovery() has + // since run, and only then does the queued callback get processed. + internal.scanning = true; + let uuid = Uuid::from_u128(0x44444444_4444_4444_4444_444444444444); + let peripheral = test_peripheral_with_uuid(uuid); + deliver_discovered_peripheral(&internal, &peripheral, None); + internal.stop_discovery(); + settle(&mut internal).await; + + let peripherals_empty = internal.peripherals.is_empty(); + let device_discovered = drain_events(&mut event_receiver) + .into_iter() + .find(|event| matches!(event, CoreBluetoothEvent::DeviceDiscovered { .. })); + + std::mem::forget(internal); + std::mem::forget(peripheral); + + assert!( + peripherals_empty, + "a callback queued before stop_discovery() must not re-add the peripheral once processed after it" + ); + assert!( + device_discovered.is_none(), + "no DeviceDiscovered event should have been dispatched, got {device_discovered:?}" + ); +} + +#[tokio::test] +#[ignore = "requires CoreBluetooth (creates a real CBCentralManager)"] +async fn late_advertisement_data_for_known_peripheral_is_ignored_after_stop_scan() { + let (_msg_sender, msg_receiver) = mpsc::channel::(4); + let (event_sender, mut event_receiver) = mpsc::channel::(4); + let mut internal = CoreBluetoothInternal::new(msg_receiver, event_sender); + settle(&mut internal).await; + internal.scanning = true; + + // Discover the peripheral while scanning, so it becomes known and we get + // its per-peripheral event channel out of the DeviceDiscovered event. + let uuid = Uuid::from_u128(0x55555555_5555_5555_5555_555555555555); + let peripheral = test_peripheral_with_uuid(uuid); + deliver_discovered_peripheral(&internal, &peripheral, None); + settle(&mut internal).await; + + let device_discovered = drain_events(&mut event_receiver) + .into_iter() + .find(|event| matches!(event, CoreBluetoothEvent::DeviceDiscovered { .. })); + let mut peripheral_event_receiver = match device_discovered { + Some(CoreBluetoothEvent::DeviceDiscovered { + uuid: event_uuid, + event_receiver, + .. + }) if event_uuid == uuid => event_receiver, + other => { + std::mem::forget(internal); + std::mem::forget(peripheral); + panic!("expected a DeviceDiscovered event for the injected peripheral, got {other:?}"); + } + }; + + // Now simulate stop_scan(): the peripheral remains known, but a + // callback carrying fresh advertisement data for it that was queued + // before stop_scan() must not be forwarded once processed after it. + internal.stop_discovery(); + deliver_discovered_peripheral(&internal, &peripheral, Some((0x1234, &[0xAA, 0xBB]))); + settle(&mut internal).await; + + let peripheral_still_known = internal.peripherals.contains_key(&uuid); + let manufacturer_event = peripheral_event_receiver.try_recv().ok(); + + std::mem::forget(internal); + std::mem::forget(peripheral); + + assert!( + peripheral_still_known, + "the peripheral should remain known after stop_discovery()" + ); + assert!( + manufacturer_event.is_none(), + "a late ManufacturerData advertisement callback must not reach an already-known peripheral after stop_scan(), got {manufacturer_event:?}" + ); +} From 487a633cc0db8294bc8e641b0e08103c6b58609a Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 18:37:49 -0700 Subject: [PATCH 10/32] test: Drain stale events before the clear_peripherals rediscovery wait The initial DeviceDiscovered could remain buffered in the event stream, so the rediscovery wait matched a pre-clear event and checked the map before the peripheral was actually rediscovered. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/common/test_cases.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 13445a29..4fa86a08 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -71,7 +71,7 @@ pub async fn test_discover_peripheral_by_name() { #[cfg(target_os = "macos")] pub async fn test_clear_peripherals_rediscovers_device() { use btleplug::api::{Central, CentralEvent, ScanFilter}; - use futures::StreamExt; + use futures::{FutureExt, StreamExt}; use std::time::Duration; use tokio::time; @@ -108,6 +108,8 @@ pub async fn test_clear_peripherals_rediscovers_device() { adapter.peripherals().await.unwrap().is_empty(), "clear_peripherals returned before the public map was cleared" ); + // Drop events emitted before the clear, including the initial DeviceDiscovered. + while let Some(Some(_)) = events.next().now_or_never() {} adapter.start_scan(ScanFilter::default()).await.unwrap(); time::timeout(Duration::from_secs(15), async { From 20b5520a1fd4183a5d48daebba552892e72b9bea Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 18:37:54 -0700 Subject: [PATCH 11/32] test(corebluetooth): Cover clear_peripherals on a connected peripheral Cover the #489 clear_peripherals change: clearing a connected peripheral on CoreBluetooth must remove it from the public map and drop the link without emitting DeviceDisconnected, as the Central::clear_peripherals doc states. Rediscovery proves the link went down (the firmware only re-advertises once disconnected), and the rediscovered handle must be usable. Update the Step 7 plan text to match the documented contract. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 2 +- tests/common/test_cases.rs | 77 +++++++++++++++++++ ...herals_disconnects_connected_peripheral.rs | 8 ++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 tests/test_clear_peripherals_disconnects_connected_peripheral.rs diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index 76db80cc..eac415bc 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -126,7 +126,7 @@ Every step runs this loop. Do not start the next step until the current step is **Goal:** Cover the #489 `clear_peripherals` behaviour change. -- `test_clear_peripherals_disconnects_connected_peripheral` (macOS only): `find_and_connect()`, subscribe to events, `clear_peripherals()`, assert `DeviceDisconnected(id)` within a timeout, then scan and assert the peripheral is rediscovered (it only re-advertises once the link is actually down). +- `test_clear_peripherals_disconnects_connected_peripheral` (macOS only): `find_and_connect()`, subscribe to events, `clear_peripherals()`, assert the peripheral left the public map, then assert it is rediscovered (it only re-advertises once the link is actually down) with no `DeviceDisconnected(id)` before that, per the `Central::clear_peripherals` doc. Finally connect the rediscovered handle and read `STATIC_READ`. **Commit:** `test(corebluetooth): Cover clear_peripherals on a connected peripheral` diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 4fa86a08..10279f4f 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -135,6 +135,83 @@ pub async fn test_clear_peripherals_rediscovers_device() { ); } +/// Covers #489: CoreBluetooth disconnects a connected peripheral as part of +/// `clear_peripherals()` but does not emit `DeviceDisconnected` for it (see +/// the `Central::clear_peripherals` doc). Proves the link actually dropped +/// via rediscovery -- the firmware only re-advertises once the connection is +/// really gone -- and that the rediscovered handle still works. +#[cfg(target_os = "macos")] +pub async fn test_clear_peripherals_disconnects_connected_peripheral() { + use btleplug::api::{Central, CentralEvent, Peripheral as _}; + use std::time::Duration; + + let peripheral = peripheral_finder::find_and_connect().await; + let target_id = peripheral.id(); + let mut events = peripheral_finder::spawn_event_collector().await; + + let adapter = peripheral_finder::get_adapter().await; + tokio::time::timeout(Duration::from_secs(10), adapter.clear_peripherals()) + .await + .expect("clear_peripherals() timed out") + .expect("clear_peripherals() failed"); + + assert!( + !adapter + .peripherals() + .await + .unwrap() + .iter() + .any(|p| p.id() == target_id), + "cleared peripheral is still in the public map" + ); + + // Bound the negative assertion (no DeviceDisconnected) by the positive + // rediscovery event rather than a sleep. + tokio::time::timeout(Duration::from_secs(15), async { + loop { + match events.recv().await { + Some(CentralEvent::DeviceDisconnected(id)) if id == target_id => panic!( + "DeviceDisconnected(test peripheral) observed -- clear_peripherals() must \ + not emit it on CoreBluetooth" + ), + Some(CentralEvent::DeviceDiscovered(id)) if id == target_id => break, + Some(_) => continue, + None => panic!("event collector channel closed while waiting for rediscovery"), + } + } + }) + .await + .expect("timed out waiting for rediscovery after clear_peripherals"); + + let rediscovered = adapter + .peripheral(&target_id) + .await + .expect("rediscovered peripheral not returned by peripheral()"); + + tokio::time::timeout(Duration::from_secs(10), rediscovered.connect()) + .await + .expect("connect() on rediscovered peripheral timed out") + .expect("connect() on rediscovered peripheral failed"); + + tokio::time::timeout(Duration::from_secs(10), rediscovered.discover_services()) + .await + .expect("discover_services() on rediscovered peripheral timed out") + .expect("discover_services() on rediscovered peripheral failed"); + + let char = peripheral_finder::find_characteristic(&rediscovered, gatt_uuids::STATIC_READ); + let value = tokio::time::timeout(Duration::from_secs(10), rediscovered.read(&char)) + .await + .expect("read(STATIC_READ) on rediscovered peripheral timed out") + .expect("read(STATIC_READ) on rediscovered peripheral failed"); + assert_eq!( + value, + gatt_uuids::STATIC_READ_VALUE, + "Static read should return [0x01, 0x02, 0x03, 0x04]" + ); + + rediscovered.disconnect().await.unwrap(); +} + pub async fn test_discover_services() { let peripheral = peripheral_finder::find_and_connect().await; let services = peripheral.services(); diff --git a/tests/test_clear_peripherals_disconnects_connected_peripheral.rs b/tests/test_clear_peripherals_disconnects_connected_peripheral.rs new file mode 100644 index 00000000..8f3ef2e5 --- /dev/null +++ b/tests/test_clear_peripherals_disconnects_connected_peripheral.rs @@ -0,0 +1,8 @@ +mod common; + +#[cfg(target_os = "macos")] +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_clear_peripherals_disconnects_connected_peripheral() { + common::test_cases::test_clear_peripherals_disconnects_connected_peripheral().await; +} From 316d8630a52dcef93002e43ea2f46c0f2c0081ea Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 18:38:22 -0700 Subject: [PATCH 12/32] doc: Record regression plan progress, conventions, and findings Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 41 ++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index eac415bc..c8782012 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -59,6 +59,47 @@ Every step runs this loop. Do not start the next step until the current step is - Platform semantics that differ legitimately (e.g. BlueZ rejecting a concurrent `Connect` with `InProgress`) are gated with `cfg` and a one-line reason. Do not weaken an assertion to make a platform pass without flagging it to the user; an unexpected failure may be a real backend bug. - New characteristics are appended to the end of existing services so the hardcoded `notify_svc.attrs[]` indices in `control_service.c` stay valid. Update the index table comment when `notify_svc` grows. +## Progress and Handoff + +Updated: 2026-09-26, after Step 7. + +### Status + +| Step | Commit(s) | +|---|---| +| Plan | `85987cb` | +| 1 | `7ff431f` (pinned to v4.4.2, not v4.4.0; SDK 1.0.1) | +| 2 | `9bc6f9e` | +| 3 | `a2507c0` | +| 4 | `4429e11` | +| 5 | `dd75aea` | +| 6 | `f4034b2` | +| 7 | `05b1922` (library fix found by this step), `487a633` (stale-event fix to `test_clear_peripherals_rediscovers_device`), `20b5520` | +| 8-14 | Not started | + +Baseline before Step 1: 32/32 hardware tests passing on macOS. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. + +### Working conventions learned so far + +- **Firmware tooling.** Workspace topdir is `test-peripheral/` (deps in `test-peripheral/deps/`, uv venv in `test-peripheral/.venv/`). Use `test-peripheral/.venv/bin/west` (or activate the venv; fish: `activate.fish`). Build from `test-peripheral/zephyr/`: `west build -b nrf52840dk/nrf52840 --pristine`; flash: `west flash --runner nrfjprog`. +- **Hardware runs.** Use `TIMEOUT=60 ./scripts/run-integration-tests.sh ` until Step 13 raises the default. Run new tests at least 3 times (5 for timing-dependent ones). Write repeat loops as bash scripts in the scratchpad; the tool shell is zsh and mangles things like `echo ====`. +- **Event helpers** in `tests/common/peripheral_finder.rs`: `spawn_event_collector` (always use it; the central event broadcast has capacity 16 and silently drops lagged events), `wait_for_event(rx, timeout, what, pred)`, `wait_for_connected` (skips stale BlueZ synthetic connection events), `wait_for_rediscovery` (no-op off Apple). Drain the receiver with `try_recv()` before starting a new phase that must not see earlier events. +- **CoreBluetooth reconnect.** CoreBluetooth drops a peripheral on disconnect (issue #57); call `wait_for_rediscovery` before any reconnect after a disconnect. +- **CoreBluetooth `clear_peripherals`** disconnects a connected peripheral without emitting `DeviceDisconnected` (documented contract since f3de711). +- **No pre-emptive relaxation.** Implementors repeatedly added `cfg` relaxations for BlueZ/Android without evidence; reviewers rejected them. Gate only with backend source evidence cited in a one-line comment. +- **Housekeeping.** Any cargo command against `tests/android/rust` rewrites `tests/android/rust/Cargo.lock`; `git checkout -- tests/android/rust/Cargo.lock` afterwards. Never use `git stash` (shared stack). Keep comments in `test_cases.rs` sparse; implementors tend to over-comment. +- **Plan text vs. source.** Check plan claims against the source before implementing: Step 6's `NotSupported("add_peripheral")` string and Step 7's `DeviceDisconnected` expectation were both wrong. + +### Findings outside this plan (report to the user; not fixed) + +1. `find_and_connect()` returns as soon as the local name matches, before the scan response (manufacturer data, service UUIDs) may have arrived, so `test_properties_contain_peripheral_info` is flaky (observed 1 failure in ~40 runs). +2. CoreBluetooth `connect()` immediately after `disconnect()` fails with "Peripheral no longer available" until the device re-advertises; other backends allow it. +3. The integration tests never initialise a logger, so `RUST_LOG` has no effect. +4. The ESP32-S3 build warns `CONFIG_HEAP_MEM_POOL_SIZE` 4096 is below the required 29696 (Zephyr bumps it automatically). +5. `AdapterManager` (`src/common/adapter_manager.rs`) uses a capacity-16 broadcast and `event_stream()` silently drops `Lagged`. Measured bursts of ~130 `DeviceUpdated` within ±500 ms of a disconnect during scanning. Any `events()` consumer can lose `DeviceConnected`/`DeviceDisconnected`. One unexplained cycle-2 `DeviceDisconnected` miss in Step 5 (1/5 runs, not reproduced in 25 instrumented runs) is attributed to this. +6. WinRT does not clear `ble_services` after a peripheral-triggered disconnect, so a read afterwards may trigger an implicit reconnect; check on Windows hardware. +7. `add_peripheral`'s `NotSupported` message ("Can't add a Peripheral from a PeripheralId") does not follow the operation-name convention `retrieve_peripherals` uses. + ## Steps ### Step 1: Pin the Zephyr version From 84f9dc23ccca7c2e324b21647929e291e1a7e1b2 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 23:04:38 -0700 Subject: [PATCH 13/32] test(zephyr): Add GATT error and refused subscription characteristics Cover #492 (failed reads and writes reported as success) and #471 (a refused subscribe never resolving). ERROR_CHAR fails every read and write with an application ATT error, and REFUSED_NOTIFY_CHAR's CCC rejects every write with Write Request Rejected. The refusing CCC is an unmanaged descriptor because a managed CCC claims Zephyr's only cfg slot before its write callback runs and never releases it on rejection. Co-Authored-By: Claude Opus 5.5 (1M context) --- test-peripheral/zephyr/src/control_service.c | 3 + test-peripheral/zephyr/src/gatt_profile.c | 21 +++++ test-peripheral/zephyr/src/gatt_profile.h | 7 ++ test-peripheral/zephyr/src/test_handlers.c | 37 ++++++++ tests/android/rust/src/lib.rs | 8 ++ .../btleplug/test/BleIntegrationTest.kt | 4 + .../btleplug/test/NativeTests.kt | 4 + tests/common/gatt_uuids.rs | 2 + tests/common/test_cases.rs | 94 +++++++++++++++++++ tests/test_gatt_error_status_is_reported.rs | 7 ++ tests/test_refused_subscribe_returns_error.rs | 7 ++ 11 files changed, 194 insertions(+) create mode 100644 tests/test_gatt_error_status_is_reported.rs create mode 100644 tests/test_refused_subscribe_returns_error.rs diff --git a/test-peripheral/zephyr/src/control_service.c b/test-peripheral/zephyr/src/control_service.c index 3cc1240f..5e0f649f 100644 --- a/test-peripheral/zephyr/src/control_service.c +++ b/test-peripheral/zephyr/src/control_service.c @@ -37,6 +37,9 @@ static void periodic_notify_handler(struct k_work *work) * [7] = Configurable Notify declaration * [8] = Configurable Notify value <-- use for bt_gatt_notify * [9] = Configurable Notify CCC + * [10] = Refused Notify declaration + * [11] = Refused Notify value (never notified -- CCC write always rejected) + * [12] = Refused Notify CCC */ /* Send notification on Notify Char (attr index 2 = value attribute) */ diff --git a/test-peripheral/zephyr/src/gatt_profile.c b/test-peripheral/zephyr/src/gatt_profile.c index 94cfd9d4..cfeb141a 100644 --- a/test-peripheral/zephyr/src/gatt_profile.c +++ b/test-peripheral/zephyr/src/gatt_profile.c @@ -20,6 +20,10 @@ extern ssize_t read_long_value(struct bt_conn *, const struct bt_gatt_attr *, void *, uint16_t, uint16_t); extern ssize_t write_long_value(struct bt_conn *, const struct bt_gatt_attr *, const void *, uint16_t, uint16_t, uint8_t); +extern ssize_t read_error(struct bt_conn *, const struct bt_gatt_attr *, + void *, uint16_t, uint16_t); +extern ssize_t write_error(struct bt_conn *, const struct bt_gatt_attr *, + const void *, uint16_t, uint16_t, uint8_t); extern ssize_t read_descriptor_test_char(struct bt_conn *, const struct bt_gatt_attr *, void *, uint16_t, uint16_t); @@ -32,6 +36,10 @@ extern ssize_t write_rw_descriptor(struct bt_conn *, const struct bt_gatt_attr * extern void notify_ccc_changed(const struct bt_gatt_attr *, uint16_t); extern void indicate_ccc_changed(const struct bt_gatt_attr *, uint16_t); extern void configurable_notify_ccc_changed(const struct bt_gatt_attr *, uint16_t); +extern ssize_t read_refused_ccc(struct bt_conn *, const struct bt_gatt_attr *, + void *, uint16_t, uint16_t); +extern ssize_t write_refused_ccc(struct bt_conn *, const struct bt_gatt_attr *, + const void *, uint16_t, uint16_t, uint8_t); /* Forward declaration for control point write handler */ extern ssize_t write_control_point(struct bt_conn *, const struct bt_gatt_attr *, @@ -92,6 +100,11 @@ BT_GATT_SERVICE_DEFINE(rw_svc, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, read_long_value, write_long_value, NULL), + /* Error Char -- read and write both always fail (#492) */ + BT_GATT_CHARACTERISTIC(BT_UUID_ERROR_CHAR, + BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, + read_error, write_error, NULL), ); /* ============================================================ @@ -120,6 +133,14 @@ BT_GATT_SERVICE_DEFINE(notify_svc, NULL, NULL, NULL), BT_GATT_CCC(configurable_notify_ccc_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), + /* Refused Notify -- unmanaged CCC whose write callback rejects every subscribe (#471) */ + BT_GATT_CHARACTERISTIC(BT_UUID_REFUSED_NOTIFY_CHAR, + BT_GATT_CHRC_NOTIFY, + BT_GATT_PERM_NONE, + NULL, NULL, NULL), + BT_GATT_DESCRIPTOR(BT_UUID_GATT_CCC, + BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, + read_refused_ccc, write_refused_ccc, NULL), ); /* ============================================================ diff --git a/test-peripheral/zephyr/src/gatt_profile.h b/test-peripheral/zephyr/src/gatt_profile.h index 2eb7a24f..6df0be1d 100644 --- a/test-peripheral/zephyr/src/gatt_profile.h +++ b/test-peripheral/zephyr/src/gatt_profile.h @@ -31,6 +31,8 @@ BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000205, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) #define BT_UUID_LONG_VALUE \ BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000206, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) +#define BT_UUID_ERROR_CHAR \ + BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000207, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) /* --- Notification Test Service (0x00000003-...) --- */ #define BT_UUID_NOTIFY_SERVICE \ @@ -41,6 +43,8 @@ BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000302, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) #define BT_UUID_CONFIGURABLE_NOTIFY \ BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000303, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) +#define BT_UUID_REFUSED_NOTIFY_CHAR \ + BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000304, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) /* --- Descriptor Test Service (0x00000004-...) --- */ #define BT_UUID_DESCRIPTOR_SERVICE \ @@ -66,6 +70,9 @@ #define MANUFACTURER_COMPANY_ID 0xFFFF #define NOTIFICATION_INTERVAL_MS 1000 +/* Application error (0x80-0x9F); auth/encryption errors would trigger pairing. */ +#define ERROR_CHAR_ATT_ERROR 0x80 + /* --- Shared State --- */ /** Peripheral state accessible from all modules. */ diff --git a/test-peripheral/zephyr/src/test_handlers.c b/test-peripheral/zephyr/src/test_handlers.c index 42ac418b..950705ee 100644 --- a/test-peripheral/zephyr/src/test_handlers.c +++ b/test-peripheral/zephyr/src/test_handlers.c @@ -105,6 +105,22 @@ ssize_t write_long_value(struct bt_conn *conn, const struct bt_gatt_attr *attr, return len; } +/* --- Error Test Characteristic Callbacks --- */ + +/* ERROR_CHAR: every read and write fails (#492). */ +ssize_t read_error(struct bt_conn *conn, const struct bt_gatt_attr *attr, + void *buf, uint16_t len, uint16_t offset) +{ + return BT_GATT_ERR(ERROR_CHAR_ATT_ERROR); +} + +ssize_t write_error(struct bt_conn *conn, const struct bt_gatt_attr *attr, + const void *buf, uint16_t len, uint16_t offset, + uint8_t flags) +{ + return BT_GATT_ERR(ERROR_CHAR_ATT_ERROR); +} + /* --- Descriptor Test Service Callbacks --- */ static const uint8_t read_only_descriptor_val[] = {0xDE, 0xAD, 0xBE, 0xEF}; @@ -168,3 +184,24 @@ void configurable_notify_ccc_changed(const struct bt_gatt_attr *attr, uint16_t v LOG_INF("Configurable Notify CCC: %s", g_state.configurable_notify_enabled ? "enabled" : "disabled"); } + +/* + * Unmanaged CCC that rejects every write so subscribe fails with an ATT error (#471). + * Unmanaged (not BT_GATT_CCC/BT_GATT_CCC_WITH_WRITE_CB) because a managed CCC claims + * its one cfg slot before this write callback runs, and a rejected write never frees it. + */ +ssize_t write_refused_ccc(struct bt_conn *conn, const struct bt_gatt_attr *attr, + const void *buf, uint16_t len, uint16_t offset, + uint8_t flags) +{ + LOG_INF("Refused Notify CCC write rejected"); + return BT_GATT_ERR(BT_ATT_ERR_WRITE_REQ_REJECTED); +} + +ssize_t read_refused_ccc(struct bt_conn *conn, const struct bt_gatt_attr *attr, + void *buf, uint16_t len, uint16_t offset) +{ + static const uint8_t ccc_value[2] = {0x00, 0x00}; + return bt_gatt_attr_read(conn, attr, buf, len, offset, + ccc_value, sizeof(ccc_value)); +} diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index 254a99ed..e900db34 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -271,3 +271,11 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testAddPeripheralByAddress, test_cases::test_add_peripheral_by_address ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testGattErrorStatusIsReported, + test_cases::test_gatt_error_status_is_reported +); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testRefusedSubscribeReturnsError, + test_cases::test_refused_subscribe_returns_error +); diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index c1de6d54..a5fb5b23 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -117,4 +117,8 @@ class BleIntegrationTest { @Test fun testDiscoverServicesDuringRead() = NativeTests.testDiscoverServicesDuringRead() @Test fun testOperationsAcrossPeripheralTriggeredDisconnect() = NativeTests.testOperationsAcrossPeripheralTriggeredDisconnect() @Test fun testAddPeripheralByAddress() = NativeTests.testAddPeripheralByAddress() + + // ── GATT errors ────────────────────────────────────────────────── + @Test fun testGattErrorStatusIsReported() = NativeTests.testGattErrorStatusIsReported() + @Test fun testRefusedSubscribeReturnsError() = NativeTests.testRefusedSubscribeReturnsError() } diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index e33a16f7..b783608c 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -59,4 +59,8 @@ object NativeTests { external fun testDiscoverServicesDuringRead() external fun testOperationsAcrossPeripheralTriggeredDisconnect() external fun testAddPeripheralByAddress() + + // GATT errors + external fun testGattErrorStatusIsReported() + external fun testRefusedSubscribeReturnsError() } diff --git a/tests/common/gatt_uuids.rs b/tests/common/gatt_uuids.rs index 944edec5..f9ba48d0 100644 --- a/tests/common/gatt_uuids.rs +++ b/tests/common/gatt_uuids.rs @@ -17,12 +17,14 @@ pub const WRITE_WITH_RESPONSE: Uuid = Uuid::from_u128(0x00000203_b5a3_f393_e0a9_ pub const WRITE_WITHOUT_RESPONSE: Uuid = Uuid::from_u128(0x00000204_b5a3_f393_e0a9_e50e24dcca9e); pub const READ_WRITE: Uuid = Uuid::from_u128(0x00000205_b5a3_f393_e0a9_e50e24dcca9e); pub const LONG_VALUE: Uuid = Uuid::from_u128(0x00000206_b5a3_f393_e0a9_e50e24dcca9e); +pub const ERROR_CHAR: Uuid = Uuid::from_u128(0x00000207_b5a3_f393_e0a9_e50e24dcca9e); // --- Notification Test Service --- pub const NOTIFICATION_SERVICE: Uuid = Uuid::from_u128(0x00000003_b5a3_f393_e0a9_e50e24dcca9e); pub const NOTIFY_CHAR: Uuid = Uuid::from_u128(0x00000301_b5a3_f393_e0a9_e50e24dcca9e); pub const INDICATE_CHAR: Uuid = Uuid::from_u128(0x00000302_b5a3_f393_e0a9_e50e24dcca9e); pub const CONFIGURABLE_NOTIFY: Uuid = Uuid::from_u128(0x00000303_b5a3_f393_e0a9_e50e24dcca9e); +pub const REFUSED_NOTIFY_CHAR: Uuid = Uuid::from_u128(0x00000304_b5a3_f393_e0a9_e50e24dcca9e); // --- Descriptor Test Service --- pub const DESCRIPTOR_SERVICE: Uuid = Uuid::from_u128(0x00000004_b5a3_f393_e0a9_e50e24dcca9e); diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 10279f4f..c7d64217 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -776,6 +776,50 @@ pub async fn test_characteristic_properties() { peripheral.disconnect().await.unwrap(); } +/// Covers #492: GATT errors on read/write must surface as `Err`, and the link must stay usable. +pub async fn test_gatt_error_status_is_reported() { + use btleplug::api::WriteType; + use std::time::Duration; + + let peripheral = peripheral_finder::find_and_connect().await; + peripheral_finder::reset_peripheral(&peripheral).await; + let char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::ERROR_CHAR); + + let read_result = tokio::time::timeout(Duration::from_secs(10), peripheral.read(&char)) + .await + .expect("read(ERROR_CHAR) timed out"); + assert!( + read_result.is_err(), + "expected an error reading ERROR_CHAR, got {:?}", + read_result + ); + + let write_result = tokio::time::timeout( + Duration::from_secs(10), + peripheral.write(&char, &[0x00], WriteType::WithResponse), + ) + .await + .expect("write(ERROR_CHAR) timed out"); + assert!( + write_result.is_err(), + "expected an error writing ERROR_CHAR, got {:?}", + write_result + ); + + let static_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::STATIC_READ); + let value = tokio::time::timeout(Duration::from_secs(10), peripheral.read(&static_char)) + .await + .expect("follow-up read(STATIC_READ) timed out") + .expect("follow-up read(STATIC_READ) failed"); + assert_eq!( + value, + gatt_uuids::STATIC_READ_VALUE, + "Static read should return [0x01, 0x02, 0x03, 0x04]" + ); + + peripheral.disconnect().await.unwrap(); +} + // ── Notifications ─────────────────────────────────────────────────── pub async fn test_subscribe_and_receive_notifications() { @@ -1054,6 +1098,56 @@ pub async fn test_resubscribe_does_not_duplicate_notifications() { peripheral.disconnect().await.unwrap(); } +/// Covers #471: a subscribe the peripheral refuses must resolve to `Err`, and later subscribes must still work. +pub async fn test_refused_subscribe_returns_error() { + use futures::StreamExt; + use std::time::Duration; + use tokio::time; + + let peripheral = peripheral_finder::find_and_connect().await; + peripheral_finder::reset_peripheral(&peripheral).await; + let refused_char = + peripheral_finder::find_characteristic(&peripheral, gatt_uuids::REFUSED_NOTIFY_CHAR); + + let subscribe_result = + tokio::time::timeout(Duration::from_secs(10), peripheral.subscribe(&refused_char)) + .await + .expect("subscribe(REFUSED_NOTIFY_CHAR) timed out"); + assert!( + subscribe_result.is_err(), + "expected an error subscribing to REFUSED_NOTIFY_CHAR, got {:?}", + subscribe_result + ); + + let notify_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::NOTIFY_CHAR); + let mut stream = peripheral.notifications().await.unwrap(); + tokio::time::timeout(Duration::from_secs(10), peripheral.subscribe(¬ify_char)) + .await + .expect("subscribe(NOTIFY_CHAR) timed out") + .expect("subscribe(NOTIFY_CHAR) should succeed after the refused subscribe"); + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_START_NOTIFICATIONS).await; + + let received = time::timeout(Duration::from_secs(5), async { + loop { + match stream.next().await { + Some(notification) if notification.uuid == gatt_uuids::NOTIFY_CHAR => return true, + Some(_) => continue, + None => return false, + } + } + }) + .await + .unwrap_or(false); + assert!( + received, + "expected a notification on NOTIFY_CHAR after successfully subscribing" + ); + + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_STOP_NOTIFICATIONS).await; + peripheral.unsubscribe(¬ify_char).await.unwrap(); + peripheral.disconnect().await.unwrap(); +} + // ── Descriptors ───────────────────────────────────────────────────── pub async fn test_read_only_descriptor() { diff --git a/tests/test_gatt_error_status_is_reported.rs b/tests/test_gatt_error_status_is_reported.rs new file mode 100644 index 00000000..04da69a1 --- /dev/null +++ b/tests/test_gatt_error_status_is_reported.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_gatt_error_status_is_reported() { + common::test_cases::test_gatt_error_status_is_reported().await; +} diff --git a/tests/test_refused_subscribe_returns_error.rs b/tests/test_refused_subscribe_returns_error.rs new file mode 100644 index 00000000..1de28a0f --- /dev/null +++ b/tests/test_refused_subscribe_returns_error.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_refused_subscribe_returns_error() { + common::test_cases::test_refused_subscribe_returns_error().await; +} From 20f3e5f8e4a9af90311a868e15dadc1efef5e490 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 23:04:47 -0700 Subject: [PATCH 14/32] doc: Record Step 8 progress and firmware tooling notes Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index c8782012..50df56bb 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -61,7 +61,7 @@ Every step runs this loop. Do not start the next step until the current step is ## Progress and Handoff -Updated: 2026-09-26, after Step 7. +Updated: 2026-09-26, after Step 8. ### Status @@ -75,7 +75,8 @@ Updated: 2026-09-26, after Step 7. | 5 | `dd75aea` | | 6 | `f4034b2` | | 7 | `05b1922` (library fix found by this step), `487a633` (stale-event fix to `test_clear_peripherals_rediscovers_device`), `20b5520` | -| 8-14 | Not started | +| 8 | `84f9dc2` (refusing CCC is an unmanaged descriptor, not the plan's managed CCC: a managed CCC leaks Zephyr's only cfg slot on rejection) | +| 9-14 | Not started | Baseline before Step 1: 32/32 hardware tests passing on macOS. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. @@ -88,6 +89,8 @@ Baseline before Step 1: 32/32 hardware tests passing on macOS. Only macOS has be - **CoreBluetooth `clear_peripherals`** disconnects a connected peripheral without emitting `DeviceDisconnected` (documented contract since f3de711). - **No pre-emptive relaxation.** Implementors repeatedly added `cfg` relaxations for BlueZ/Android without evidence; reviewers rejected them. Gate only with backend source evidence cited in a one-line comment. - **Housekeeping.** Any cargo command against `tests/android/rust` rewrites `tests/android/rust/Cargo.lock`; `git checkout -- tests/android/rust/Cargo.lock` afterwards. Never use `git stash` (shared stack). Keep comments in `test_cases.rs` sparse; implementors tend to over-comment. +- **ESP32-S3 build check.** The board is `esp32s3_devkitc/esp32s3/procpu` (not `devkitm`); put `test-peripheral/.venv/bin` on `PATH` so the build finds `esptool`. +- **RTT / J-Link.** The DK's onboard J-Link runs old firmware (V1): `JLinkRTTLogger` never finds the RTT control block, and attaching `JLinkExe` at 4 MHz SWD stops BLE advertising (tests time out in `find_and_connect()`). Don't sink time into RTT capture; prove firmware behaviour from Zephyr source and test outcomes, or use macOS PacketLogger for ATT traffic. - **Plan text vs. source.** Check plan claims against the source before implementing: Step 6's `NotSupported("add_peripheral")` string and Step 7's `DeviceDisconnected` expectation were both wrong. ### Findings outside this plan (report to the user; not fixed) From b768000262d508246c4c55f3abf0aa46889ba524 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 23:21:49 -0700 Subject: [PATCH 15/32] test(zephyr): Verify write-without-response delivery order Cover #464: the existing burst test could not observe dropped or reordered writes. The firmware now records the count, last sequence byte, and out-of-order count of write-without-response payloads and exposes them through WRITE_LOG_CHAR; the burst test polls it and requires all 50 writes delivered in order. Co-Authored-By: Claude Opus 5.5 (1M context) --- test-peripheral/zephyr/src/control_service.c | 3 ++ test-peripheral/zephyr/src/gatt_profile.c | 7 ++++ test-peripheral/zephyr/src/gatt_profile.h | 6 ++++ test-peripheral/zephyr/src/test_handlers.c | 26 ++++++++++++++ tests/common/gatt_uuids.rs | 1 + tests/common/test_cases.rs | 38 ++++++++++++++++++-- 6 files changed, 78 insertions(+), 3 deletions(-) diff --git a/test-peripheral/zephyr/src/control_service.c b/test-peripheral/zephyr/src/control_service.c index 5e0f649f..93d38d50 100644 --- a/test-peripheral/zephyr/src/control_service.c +++ b/test-peripheral/zephyr/src/control_service.c @@ -95,6 +95,9 @@ void reset_peripheral_state(void) g_state.write_with_resp_len = 0; g_state.notify_payload_len = 0; g_state.rw_descriptor_len = 0; + g_state.wwr_count = 0; + g_state.wwr_last_seq = 0; + g_state.wwr_out_of_order = 0; memset(g_state.rw_value, 0, sizeof(g_state.rw_value)); memset(g_state.long_value, 0, sizeof(g_state.long_value)); memset(g_state.write_with_resp_value, 0, sizeof(g_state.write_with_resp_value)); diff --git a/test-peripheral/zephyr/src/gatt_profile.c b/test-peripheral/zephyr/src/gatt_profile.c index cfeb141a..30fb8aab 100644 --- a/test-peripheral/zephyr/src/gatt_profile.c +++ b/test-peripheral/zephyr/src/gatt_profile.c @@ -24,6 +24,8 @@ extern ssize_t read_error(struct bt_conn *, const struct bt_gatt_attr *, void *, uint16_t, uint16_t); extern ssize_t write_error(struct bt_conn *, const struct bt_gatt_attr *, const void *, uint16_t, uint16_t, uint8_t); +extern ssize_t read_write_log(struct bt_conn *, const struct bt_gatt_attr *, + void *, uint16_t, uint16_t); extern ssize_t read_descriptor_test_char(struct bt_conn *, const struct bt_gatt_attr *, void *, uint16_t, uint16_t); @@ -105,6 +107,11 @@ BT_GATT_SERVICE_DEFINE(rw_svc, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, read_error, write_error, NULL), + /* Write Log -- write-without-response ordering counters (#464) */ + BT_GATT_CHARACTERISTIC(BT_UUID_WRITE_LOG_CHAR, + BT_GATT_CHRC_READ, + BT_GATT_PERM_READ, + read_write_log, NULL, NULL), ); /* ============================================================ diff --git a/test-peripheral/zephyr/src/gatt_profile.h b/test-peripheral/zephyr/src/gatt_profile.h index 6df0be1d..a6979972 100644 --- a/test-peripheral/zephyr/src/gatt_profile.h +++ b/test-peripheral/zephyr/src/gatt_profile.h @@ -33,6 +33,8 @@ BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000206, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) #define BT_UUID_ERROR_CHAR \ BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000207, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) +#define BT_UUID_WRITE_LOG_CHAR \ + BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000208, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) /* --- Notification Test Service (0x00000003-...) --- */ #define BT_UUID_NOTIFY_SERVICE \ @@ -92,6 +94,10 @@ struct peripheral_state { uint16_t notify_payload_len; uint8_t rw_descriptor_value[256]; uint16_t rw_descriptor_len; + /* Write-without-response ordering log (#464); see WRITE_LOG_CHAR. */ + uint16_t wwr_count; + uint8_t wwr_last_seq; + uint8_t wwr_out_of_order; }; extern struct peripheral_state g_state; diff --git a/test-peripheral/zephyr/src/test_handlers.c b/test-peripheral/zephyr/src/test_handlers.c index 950705ee..acb356b2 100644 --- a/test-peripheral/zephyr/src/test_handlers.c +++ b/test-peripheral/zephyr/src/test_handlers.c @@ -59,6 +59,19 @@ ssize_t write_without_resp(struct bt_conn *conn, const struct bt_gatt_attr *attr */ memcpy(g_state.rw_value + offset, buf, len); g_state.rw_value_len = offset + len; + + /* Track delivery order (#464): first byte of each write is a sequence number. */ + if (len > 0) { + uint8_t seq = ((const uint8_t *)buf)[0]; + if (g_state.wwr_count != 0 && seq != (uint8_t)(g_state.wwr_last_seq + 1)) { + if (g_state.wwr_out_of_order < 0xFF) { + g_state.wwr_out_of_order++; + } + } + g_state.wwr_last_seq = seq; + g_state.wwr_count++; + } + LOG_INF("Write without response: %u bytes", len); return len; } @@ -121,6 +134,19 @@ ssize_t write_error(struct bt_conn *conn, const struct bt_gatt_attr *attr, return BT_GATT_ERR(ERROR_CHAR_ATT_ERROR); } +/* WRITE_LOG_CHAR: [count_lo, count_hi, last_seq, out_of_order] (#464). */ +ssize_t read_write_log(struct bt_conn *conn, const struct bt_gatt_attr *attr, + void *buf, uint16_t len, uint16_t offset) +{ + uint8_t data[4] = { + (uint8_t)(g_state.wwr_count & 0xFF), + (uint8_t)((g_state.wwr_count >> 8) & 0xFF), + g_state.wwr_last_seq, + g_state.wwr_out_of_order, + }; + return bt_gatt_attr_read(conn, attr, buf, len, offset, data, sizeof(data)); +} + /* --- Descriptor Test Service Callbacks --- */ static const uint8_t read_only_descriptor_val[] = {0xDE, 0xAD, 0xBE, 0xEF}; diff --git a/tests/common/gatt_uuids.rs b/tests/common/gatt_uuids.rs index f9ba48d0..d6633ac6 100644 --- a/tests/common/gatt_uuids.rs +++ b/tests/common/gatt_uuids.rs @@ -18,6 +18,7 @@ pub const WRITE_WITHOUT_RESPONSE: Uuid = Uuid::from_u128(0x00000204_b5a3_f393_e0 pub const READ_WRITE: Uuid = Uuid::from_u128(0x00000205_b5a3_f393_e0a9_e50e24dcca9e); pub const LONG_VALUE: Uuid = Uuid::from_u128(0x00000206_b5a3_f393_e0a9_e50e24dcca9e); pub const ERROR_CHAR: Uuid = Uuid::from_u128(0x00000207_b5a3_f393_e0a9_e50e24dcca9e); +pub const WRITE_LOG_CHAR: Uuid = Uuid::from_u128(0x00000208_b5a3_f393_e0a9_e50e24dcca9e); // --- Notification Test Service --- pub const NOTIFICATION_SERVICE: Uuid = Uuid::from_u128(0x00000003_b5a3_f393_e0a9_e50e24dcca9e); diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index c7d64217..9eb2470b 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -695,23 +695,55 @@ pub async fn test_write_without_response() { pub async fn test_write_without_response_burst() { use btleplug::api::WriteType; + use std::time::Duration; let peripheral = peripheral_finder::find_and_connect().await; peripheral_finder::reset_peripheral(&peripheral).await; let char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::WRITE_WITHOUT_RESPONSE); + let log_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::WRITE_LOG_CHAR); // Send a burst of writes to exercise flow control. Without proper // canSendWriteWithoutResponse handling, later writes would be silently - // dropped by CoreBluetooth. - let num_writes = 50; - for i in 0u8..num_writes { + // dropped or reordered by CoreBluetooth (#464). The first byte of each + // payload is a sequence number the firmware checks for ordering. + let num_writes = 50u8; + for i in 0..num_writes { let data = vec![i; 20]; peripheral .write(&char, &data, WriteType::WithoutResponse) .await .expect(&format!("write-without-response #{} should succeed", i)); } + + // Poll WRITE_LOG_CHAR ([count_lo, count_hi, last_seq, out_of_order]) until + // the firmware has observed all 50 writes. + let deadline = tokio::time::Instant::now() + Duration::from_secs(2); + let mut last: Option<[u8; 4]> = None; + let (count, last_seq, out_of_order) = loop { + let raw = tokio::time::timeout(Duration::from_secs(10), peripheral.read(&log_char)) + .await + .unwrap_or_else(|_| panic!("read(WRITE_LOG_CHAR) timed out; last observed {last:?}")) + .expect("read(WRITE_LOG_CHAR) should succeed"); + assert_eq!(raw.len(), 4, "WRITE_LOG_CHAR returned {raw:?}"); + let observed = [raw[0], raw[1], raw[2], raw[3]]; + last = Some(observed); + let count = u16::from_le_bytes([observed[0], observed[1]]); + if count >= u16::from(num_writes) { + break (count, observed[2], observed[3]); + } + if tokio::time::Instant::now() >= deadline { + panic!( + "WRITE_LOG_CHAR did not reach count {num_writes} within 2s; last observed {last:?}" + ); + } + tokio::time::sleep(Duration::from_millis(100)).await; + }; + + assert_eq!(count, u16::from(num_writes), "write count mismatch"); + assert_eq!(last_seq, num_writes - 1, "last_seq mismatch"); + assert_eq!(out_of_order, 0, "writes were observed out of order"); + peripheral.disconnect().await.unwrap(); } From e86ef074a6f3f778a04f7ae726dbe54ca800861d Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sat, 26 Sep 2026 23:21:49 -0700 Subject: [PATCH 16/32] doc: Record Step 9 progress Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index 50df56bb..6ef30256 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -61,7 +61,7 @@ Every step runs this loop. Do not start the next step until the current step is ## Progress and Handoff -Updated: 2026-09-26, after Step 8. +Updated: 2026-09-26, after Step 9. ### Status @@ -76,7 +76,8 @@ Updated: 2026-09-26, after Step 8. | 6 | `f4034b2` | | 7 | `05b1922` (library fix found by this step), `487a633` (stale-event fix to `test_clear_peripherals_rediscovers_device`), `20b5520` | | 8 | `84f9dc2` (refusing CCC is an unmanaged descriptor, not the plan's managed CCC: a managed CCC leaks Zephyr's only cfg slot on rejection) | -| 9-14 | Not started | +| 9 | `b768000` (no drops or reordering observed on macOS) | +| 10-14 | Not started | Baseline before Step 1: 32/32 hardware tests passing on macOS. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. @@ -90,6 +91,7 @@ Baseline before Step 1: 32/32 hardware tests passing on macOS. Only macOS has be - **No pre-emptive relaxation.** Implementors repeatedly added `cfg` relaxations for BlueZ/Android without evidence; reviewers rejected them. Gate only with backend source evidence cited in a one-line comment. - **Housekeeping.** Any cargo command against `tests/android/rust` rewrites `tests/android/rust/Cargo.lock`; `git checkout -- tests/android/rust/Cargo.lock` afterwards. Never use `git stash` (shared stack). Keep comments in `test_cases.rs` sparse; implementors tend to over-comment. - **ESP32-S3 build check.** The board is `esp32s3_devkitc/esp32s3/procpu` (not `devkitm`); put `test-peripheral/.venv/bin` on `PATH` so the build finds `esptool`. +- **Firmware threading.** Both boards build with `CONFIG_BT_RECV_WORKQ_BT=y`: ATT/GATT callbacks, including `CMD_RESET_STATE` (called directly from the Control Point write), run serially on the "BT RX WQ" thread. Only `periodic_notify_handler` and `disconnect_handler` run on the system workqueue. - **RTT / J-Link.** The DK's onboard J-Link runs old firmware (V1): `JLinkRTTLogger` never finds the RTT control block, and attaching `JLinkExe` at 4 MHz SWD stops BLE advertising (tests time out in `find_and_connect()`). Don't sink time into RTT capture; prove firmware behaviour from Zephyr source and test outcomes, or use macOS PacketLogger for ATT traffic. - **Plan text vs. source.** Check plan claims against the source before implementing: Step 6's `NotSupported("add_peripheral")` string and Step 7's `DeviceDisconnected` expectation were both wrong. From 79fb66b310331f91f7f92a9775441d1d2b5bee56 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 00:19:56 -0700 Subject: [PATCH 17/32] test(zephyr): Add included secondary service to the test profile Add secondary service 0x00000005 with a READ characteristic (0x00000501, value [0x05]) and include it from the Descriptor service. The new test_discovery_with_included_service checks that discovery and re-discovery complete with the include present and that the peripheral stays usable afterwards. Reintroducing either #487 panic site in src/corebluetooth/internal.rs (the unknown-service expect in set_characteristics, or the no-pending- future panic in check_discovered) makes the test fail on macOS hardware; test_discover_services does not reach the check_discovered panic. Co-Authored-By: Claude Opus 5.5 (1M context) --- test-peripheral/zephyr/src/gatt_profile.c | 18 +++++++ test-peripheral/zephyr/src/gatt_profile.h | 6 +++ test-peripheral/zephyr/src/test_handlers.c | 10 ++++ tests/android/rust/src/lib.rs | 4 ++ .../btleplug/test/BleIntegrationTest.kt | 3 ++ .../btleplug/test/NativeTests.kt | 3 ++ tests/common/gatt_uuids.rs | 4 ++ tests/common/test_cases.rs | 54 +++++++++++++++++++ tests/test_discovery_with_included_service.rs | 7 +++ 9 files changed, 109 insertions(+) create mode 100644 tests/test_discovery_with_included_service.rs diff --git a/test-peripheral/zephyr/src/gatt_profile.c b/test-peripheral/zephyr/src/gatt_profile.c index 30fb8aab..9a851e6d 100644 --- a/test-peripheral/zephyr/src/gatt_profile.c +++ b/test-peripheral/zephyr/src/gatt_profile.c @@ -42,6 +42,8 @@ extern ssize_t read_refused_ccc(struct bt_conn *, const struct bt_gatt_attr *, void *, uint16_t, uint16_t); extern ssize_t write_refused_ccc(struct bt_conn *, const struct bt_gatt_attr *, const void *, uint16_t, uint16_t, uint8_t); +extern ssize_t read_included_char(struct bt_conn *, const struct bt_gatt_attr *, + void *, uint16_t, uint16_t); /* Forward declaration for control point write handler */ extern ssize_t write_control_point(struct bt_conn *, const struct bt_gatt_attr *, @@ -150,11 +152,27 @@ BT_GATT_SERVICE_DEFINE(notify_svc, read_refused_ccc, write_refused_ccc, NULL), ); +/* ============================================================ + * Service 5: Included Secondary Service (#487) — must be defined + * before descriptor_svc includes it below. + * ============================================================ */ +BT_GATT_SERVICE_DEFINE(included_svc, + BT_GATT_SECONDARY_SERVICE(BT_UUID_INCLUDED_SERVICE), + /* Included Char */ + BT_GATT_CHARACTERISTIC(BT_UUID_INCLUDED_CHAR, + BT_GATT_CHRC_READ, + BT_GATT_PERM_READ, + read_included_char, NULL, NULL), +); + /* ============================================================ * Service 4: Descriptor Test Service * ============================================================ */ BT_GATT_SERVICE_DEFINE(descriptor_svc, BT_GATT_PRIMARY_SERVICE(BT_UUID_DESCRIPTOR_SERVICE), + /* Include Declaration — must immediately follow the service + * declaration (#487) */ + BT_GATT_INCLUDE_SERVICE((void *)attr_included_svc), /* Descriptor Test Char — Read, with custom descriptors */ BT_GATT_CHARACTERISTIC(BT_UUID_DESCRIPTOR_TEST_CHAR, BT_GATT_CHRC_READ, diff --git a/test-peripheral/zephyr/src/gatt_profile.h b/test-peripheral/zephyr/src/gatt_profile.h index a6979972..8b7e9490 100644 --- a/test-peripheral/zephyr/src/gatt_profile.h +++ b/test-peripheral/zephyr/src/gatt_profile.h @@ -58,6 +58,12 @@ #define BT_UUID_RW_DESCRIPTOR \ BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x000004A2, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) +/* --- Included Secondary Service (0x00000005-...) --- */ +#define BT_UUID_INCLUDED_SERVICE \ + BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000005, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) +#define BT_UUID_INCLUDED_CHAR \ + BT_UUID_DECLARE_128(BT_UUID_128_ENCODE(0x00000501, 0xb5a3, 0xf393, 0xe0a9, 0xe50e24dcca9e)) + /* --- Control Point Opcodes --- */ #define CMD_START_NOTIFICATIONS 0x01 #define CMD_STOP_NOTIFICATIONS 0x02 diff --git a/test-peripheral/zephyr/src/test_handlers.c b/test-peripheral/zephyr/src/test_handlers.c index acb356b2..561dc08c 100644 --- a/test-peripheral/zephyr/src/test_handlers.c +++ b/test-peripheral/zephyr/src/test_handlers.c @@ -231,3 +231,13 @@ ssize_t read_refused_ccc(struct bt_conn *conn, const struct bt_gatt_attr *attr, return bt_gatt_attr_read(conn, attr, buf, len, offset, ccc_value, sizeof(ccc_value)); } + +/* --- Included Secondary Service Callbacks (#487) --- */ + +ssize_t read_included_char(struct bt_conn *conn, const struct bt_gatt_attr *attr, + void *buf, uint16_t len, uint16_t offset) +{ + static const uint8_t included_char_val[] = {0x05}; + return bt_gatt_attr_read(conn, attr, buf, len, offset, + included_char_val, sizeof(included_char_val)); +} diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index e900db34..76b26b1a 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -279,3 +279,7 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testRefusedSubscribeReturnsError, test_cases::test_refused_subscribe_returns_error ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testDiscoveryWithIncludedService, + test_cases::test_discovery_with_included_service +); diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index a5fb5b23..c5a70360 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -121,4 +121,7 @@ class BleIntegrationTest { // ── GATT errors ────────────────────────────────────────────────── @Test fun testGattErrorStatusIsReported() = NativeTests.testGattErrorStatusIsReported() @Test fun testRefusedSubscribeReturnsError() = NativeTests.testRefusedSubscribeReturnsError() + + // ── GATT profile structure ─────────────────────────────────────── + @Test fun testDiscoveryWithIncludedService() = NativeTests.testDiscoveryWithIncludedService() } diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index b783608c..5f0b733c 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -63,4 +63,7 @@ object NativeTests { // GATT errors external fun testGattErrorStatusIsReported() external fun testRefusedSubscribeReturnsError() + + // GATT profile structure + external fun testDiscoveryWithIncludedService() } diff --git a/tests/common/gatt_uuids.rs b/tests/common/gatt_uuids.rs index d6633ac6..1fcc4144 100644 --- a/tests/common/gatt_uuids.rs +++ b/tests/common/gatt_uuids.rs @@ -33,6 +33,10 @@ pub const DESCRIPTOR_TEST_CHAR: Uuid = Uuid::from_u128(0x00000401_b5a3_f393_e0a9 pub const READ_ONLY_DESCRIPTOR: Uuid = Uuid::from_u128(0x000004a1_b5a3_f393_e0a9_e50e24dcca9e); pub const READ_WRITE_DESCRIPTOR: Uuid = Uuid::from_u128(0x000004a2_b5a3_f393_e0a9_e50e24dcca9e); +// --- Included Secondary Service (#487) --- +pub const INCLUDED_SERVICE: Uuid = Uuid::from_u128(0x00000005_b5a3_f393_e0a9_e50e24dcca9e); +pub const INCLUDED_CHAR: Uuid = Uuid::from_u128(0x00000501_b5a3_f393_e0a9_e50e24dcca9e); + // --- Control Point opcodes --- pub const CMD_START_NOTIFICATIONS: u8 = 0x01; pub const CMD_STOP_NOTIFICATIONS: u8 = 0x02; diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 9eb2470b..67b30783 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -1727,3 +1727,57 @@ pub async fn test_request_connection_parameters() { } peripheral.disconnect().await.unwrap(); } + +/// Covers #487: CoreBluetooth panics in `set_characteristics` (unknown included service) and in +/// `check_discovered` (re-discovery with no pending future). Secondary-service visibility is backend-dependent. +pub async fn test_discovery_with_included_service() { + use std::time::Duration; + use tokio::time::timeout; + + let peripheral = peripheral_finder::find_and_connect().await; + + let assert_primary_services = |peripheral: &btleplug::platform::Peripheral| { + let service_uuids: Vec<_> = peripheral.services().iter().map(|s| s.uuid).collect(); + assert!( + service_uuids.contains(&gatt_uuids::CONTROL_SERVICE), + "Control Service not found in {:?}", + service_uuids + ); + assert!( + service_uuids.contains(&gatt_uuids::READ_WRITE_SERVICE), + "Read/Write Service not found in {:?}", + service_uuids + ); + assert!( + service_uuids.contains(&gatt_uuids::NOTIFICATION_SERVICE), + "Notification Service not found in {:?}", + service_uuids + ); + assert!( + service_uuids.contains(&gatt_uuids::DESCRIPTOR_SERVICE), + "Descriptor Service not found in {:?}", + service_uuids + ); + }; + + assert_primary_services(&peripheral); + + timeout(Duration::from_secs(10), peripheral.discover_services()) + .await + .expect("discover_services() timed out") + .expect("discover_services() failed"); + assert_primary_services(&peripheral); + + let static_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::STATIC_READ); + let value = timeout(Duration::from_secs(10), peripheral.read(&static_char)) + .await + .expect("read(STATIC_READ) timed out") + .expect("read(STATIC_READ) failed"); + assert_eq!( + value, + gatt_uuids::STATIC_READ_VALUE, + "Static read should return [0x01, 0x02, 0x03, 0x04]" + ); + + peripheral.disconnect().await.unwrap(); +} diff --git a/tests/test_discovery_with_included_service.rs b/tests/test_discovery_with_included_service.rs new file mode 100644 index 00000000..301ee44e --- /dev/null +++ b/tests/test_discovery_with_included_service.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_discovery_with_included_service() { + common::test_cases::test_discovery_with_included_service().await; +} From 1658dd154ef9e9f51f935c2f4eebc5eb597c65d8 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 00:20:10 -0700 Subject: [PATCH 18/32] doc: Record Step 10 progress Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index 6ef30256..5d913139 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -61,7 +61,7 @@ Every step runs this loop. Do not start the next step until the current step is ## Progress and Handoff -Updated: 2026-09-26, after Step 9. +Updated: 2026-09-27, after Step 10. ### Status @@ -77,9 +77,10 @@ Updated: 2026-09-26, after Step 9. | 7 | `05b1922` (library fix found by this step), `487a633` (stale-event fix to `test_clear_peripherals_rediscovers_device`), `20b5520` | | 8 | `84f9dc2` (refusing CCC is an unmanaged descriptor, not the plan's managed CCC: a managed CCC leaks Zephyr's only cfg slot on rejection) | | 9 | `b768000` (no drops or reordering observed on macOS) | -| 10-14 | Not started | +| 10 | `79fb66b` (proved against both #487 panic sites separately; handles follow service variable names, see conventions) | +| 11-14 | Not started | -Baseline before Step 1: 32/32 hardware tests passing on macOS. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. +Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 10: 44/44. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. ### Working conventions learned so far @@ -94,6 +95,8 @@ Baseline before Step 1: 32/32 hardware tests passing on macOS. Only macOS has be - **Firmware threading.** Both boards build with `CONFIG_BT_RECV_WORKQ_BT=y`: ATT/GATT callbacks, including `CMD_RESET_STATE` (called directly from the Control Point write), run serially on the "BT RX WQ" thread. Only `periodic_notify_handler` and `disconnect_handler` run on the system workqueue. - **RTT / J-Link.** The DK's onboard J-Link runs old firmware (V1): `JLinkRTTLogger` never finds the RTT control block, and attaching `JLinkExe` at 4 MHz SWD stops BLE advertising (tests time out in `find_and_connect()`). Don't sink time into RTT capture; prove firmware behaviour from Zephyr source and test outcomes, or use macOS PacketLogger for ATT traffic. - **Plan text vs. source.** Check plan claims against the source before implementing: Step 6's `NotSupported("add_peripheral")` string and Step 7's `DeviceDisconnected` expectation were both wrong. +- **GATT handle order.** Static services are placed by `SORT_BY_NAME` on the `BT_GATT_SERVICE_DEFINE` variable name, not source order (current order: control, descriptor, included, notify, rw). Only relative `attrs[]` indices are safe to hardcode. +- **Regression proofs.** When a fix removed several panic or error sites, reintroduce them one at a time: an earlier site masks later ones. Add a control test (an existing test on the same broken code) to show the new test is what reaches the site. ### Findings outside this plan (report to the user; not fixed) @@ -255,7 +258,8 @@ Tests: ### Step 14: Final documentation and branch review - Update `tests/AGENTS.md` test categories and freshness date for the new tests (concurrency, errors, advertisement rotation). -- Update `test-peripheral/AGENTS.md` with the new characteristics, secondary service, opcode 0x04 behaviour, and payload size. +- Update `test-peripheral/AGENTS.md` with the new characteristics, secondary service, opcode 0x04 behaviour, and payload size. Its "Four GATT services" line (~:26) is now four primary plus one secondary; also note the `SORT_BY_NAME` handle ordering. +- `tests/AGENTS.md`: the Discovery glob `test_discover_*.rs` misses `test_discovery_with_included_service.rs`. - Run a final `opus` review over the whole range of commits from Step 1 to here, and a full hardware run of `./scripts/run-integration-tests.sh` on macOS with the nRF52840. Record any platform that could not be run. **Commit:** `doc: Update test suite and test peripheral docs for regression expansion` From d68ff66213183a29ebf5d598faf0aa6c3989fcd2 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 01:51:56 -0700 Subject: [PATCH 19/32] test(zephyr): Implement advertisement rotation with 128-bit service data CMD_CHANGE_ADVERTISEMENTS (0x04) now sets an atomic pending flag. The next advertising restart after a disconnect uses an alternate set: flags + name in the advertising data, and 128-bit service data (Control Service UUID + [0x01]) plus manufacturer data in the scan response. The flag clears on the next successful connection or on CMD_RESET_STATE. test_advertisement_service_data_128bit waits for a ServiceDataAdvertisement carrying exactly Control Service -> [0x01] (covering the #483 128-bit UUID byte order), checks properties().service_data, then reconnects and disconnects to restore the default set for scan-only tests. With 0x04 made a no-op the test fails at the service-data wait. Co-Authored-By: Claude Opus 5.5 (1M context) --- test-peripheral/AGENTS.md | 4 +- test-peripheral/zephyr/src/control_service.c | 10 ++- test-peripheral/zephyr/src/gatt_profile.h | 5 ++ test-peripheral/zephyr/src/main.c | 46 +++++++++++- tests/android/rust/src/lib.rs | 4 + .../btleplug/test/BleIntegrationTest.kt | 1 + .../btleplug/test/NativeTests.kt | 1 + tests/common/gatt_uuids.rs | 1 + tests/common/test_cases.rs | 74 +++++++++++++++++++ .../test_advertisement_service_data_128bit.rs | 7 ++ 10 files changed, 145 insertions(+), 8 deletions(-) create mode 100644 tests/test_advertisement_service_data_128bit.rs diff --git a/test-peripheral/AGENTS.md b/test-peripheral/AGENTS.md index 7ee0203e..c871b577 100644 --- a/test-peripheral/AGENTS.md +++ b/test-peripheral/AGENTS.md @@ -24,8 +24,8 @@ Zephyr firmware GATT test peripheral used by the integration tests in `tests/`. - The canonical UUID source of truth is `tests/common/gatt_uuids.rs`, mirrored in `zephyr/src/gatt_profile.h`. - Four GATT services: Control (`0x0001`), Read/Write (`0x0002`), Notification (`0x0003`), Descriptor (`0x0004`). -- Control Point opcodes: `0x01` start notifications, `0x02` stop, `0x03` disconnect, `0x04` change adverts (currently a no-op, logged only; not yet implemented), `0x05` reset, `0x06` set notification payload. -- Peripheral advertises as `"btleplug-test"` with the Control Service UUID in the scan response. +- Control Point opcodes: `0x01` start notifications, `0x02` stop, `0x03` disconnect, `0x04` change adverts (sets a pending flag; the alternate set — flags + name advertising data, 128-bit service data + manufacturer data scan response — is used on the next advertising restart after a disconnect, and the flag clears on the next `connected()` callback or `0x05`), `0x05` reset, `0x06` set notification payload. +- Peripheral advertises as `"btleplug-test"` with the Control Service UUID in the scan response. After `0x04` and a disconnect, the next advertising cycle instead carries 128-bit service data (Control Service UUID + `[0x01]`) with no service UUID list, reverting to the default set once a client connects and disconnects again. ## Invariants diff --git a/test-peripheral/zephyr/src/control_service.c b/test-peripheral/zephyr/src/control_service.c index 93d38d50..0d132cd9 100644 --- a/test-peripheral/zephyr/src/control_service.c +++ b/test-peripheral/zephyr/src/control_service.c @@ -98,6 +98,7 @@ void reset_peripheral_state(void) g_state.wwr_count = 0; g_state.wwr_last_seq = 0; g_state.wwr_out_of_order = 0; + atomic_clear(&g_state.alt_adv_pending); memset(g_state.rw_value, 0, sizeof(g_state.rw_value)); memset(g_state.long_value, 0, sizeof(g_state.long_value)); memset(g_state.write_with_resp_value, 0, sizeof(g_state.write_with_resp_value)); @@ -137,10 +138,11 @@ void control_handle_command(const uint8_t *data, uint16_t len) k_work_reschedule(&disconnect_work, K_MSEC(500)); break; case CMD_CHANGE_ADVERTISEMENTS: - /* Deferred: advertisement rotation is not tested in the initial - * integration test suite. Implement when CentralEvent::ServiceDataAdvertisement - * tests are added. */ - LOG_INF("Change advertisements (deferred — not tested in initial suite)"); + /* Take effect on the next advertising restart after a disconnect + * (see restart_adv_work_handler in main.c); cleared on the next + * connected() callback or by CMD_RESET_STATE. */ + atomic_set(&g_state.alt_adv_pending, 1); + LOG_INF("Alternate advertising set pending"); break; case CMD_RESET_STATE: reset_peripheral_state(); diff --git a/test-peripheral/zephyr/src/gatt_profile.h b/test-peripheral/zephyr/src/gatt_profile.h index 8b7e9490..573ad3c4 100644 --- a/test-peripheral/zephyr/src/gatt_profile.h +++ b/test-peripheral/zephyr/src/gatt_profile.h @@ -2,6 +2,7 @@ #define GATT_PROFILE_H_ #include +#include /* * btleplug Test GATT Profile @@ -104,6 +105,10 @@ struct peripheral_state { uint16_t wwr_count; uint8_t wwr_last_seq; uint8_t wwr_out_of_order; + /* #483: set by CMD_CHANGE_ADVERTISEMENTS, read by restart_adv_work_handler + * (system workqueue) and written from the BT RX workqueue (Control Point + * write, connected()), hence atomic_t rather than a plain bool. */ + atomic_t alt_adv_pending; }; extern struct peripheral_state g_state; diff --git a/test-peripheral/zephyr/src/main.c b/test-peripheral/zephyr/src/main.c index c5e985e2..63d3f9fc 100644 --- a/test-peripheral/zephyr/src/main.c +++ b/test-peripheral/zephyr/src/main.c @@ -39,6 +39,35 @@ static const struct bt_data sd[] = { ), }; +/* Alternate advertising set (#483): selected for one advertising cycle by + * CMD_CHANGE_ADVERTISEMENTS -- see restart_adv_work_handler() and + * g_state.alt_adv_pending (gatt_profile.h). */ + +/* Alternate advertising data: flags + complete device name only. */ +static const struct bt_data alt_ad[] = { + BT_DATA_BYTES(BT_DATA_FLAGS, (BT_LE_AD_GENERAL | BT_LE_AD_NO_BREDR)), + BT_DATA(BT_DATA_NAME_COMPLETE, CONFIG_BT_DEVICE_NAME, + sizeof(CONFIG_BT_DEVICE_NAME) - 1), +}; + +/* Alternate scan response data: + * - 128-bit service data (Control Service UUID little-endian + [0x01]); + * proves byte order for CentralEvent::ServiceDataAdvertisement (#483). + * - Manufacturer data: same bytes as the default scan response. + */ +static const struct bt_data alt_sd[] = { + BT_DATA_BYTES(BT_DATA_SVC_DATA128, + /* Control Service UUID (little-endian) */ + 0x9e, 0xca, 0xdc, 0x24, 0x0e, 0xe5, 0xa9, 0xe0, + 0x93, 0xf3, 0xa3, 0xb5, 0x01, 0x00, 0x00, 0x00, + 0x01 /* service data value */ + ), + BT_DATA_BYTES(BT_DATA_MANUFACTURER_DATA, + 0xFF, 0xFF, /* Company ID 0xFFFF (little-endian) */ + 0xBB, 0xCC, 0x01 /* "bt" + version */ + ), +}; + static void connected(struct bt_conn *conn, uint8_t err) { if (err) { @@ -48,6 +77,10 @@ static void connected(struct bt_conn *conn, uint8_t err) LOG_INF("Connected"); g_state.conn = bt_conn_ref(conn); + + /* #483: a connection following an alternate-advertising cycle restores + * the default set on the next disconnect. */ + atomic_clear(&g_state.alt_adv_pending); } static void restart_adv_work_handler(struct k_work *work); @@ -58,8 +91,17 @@ static void restart_adv_work_handler(struct k_work *work) /* Ensure advertising is stopped before restarting */ bt_le_adv_stop(); - int err = bt_le_adv_start(BT_LE_ADV_CONN_FAST_1, ad, ARRAY_SIZE(ad), - sd, ARRAY_SIZE(sd)); + int err; + + if (atomic_get(&g_state.alt_adv_pending)) { + LOG_INF("Advertising restart: using alternate set"); + err = bt_le_adv_start(BT_LE_ADV_CONN_FAST_1, alt_ad, ARRAY_SIZE(alt_ad), + alt_sd, ARRAY_SIZE(alt_sd)); + } else { + LOG_INF("Advertising restart: using default set"); + err = bt_le_adv_start(BT_LE_ADV_CONN_FAST_1, ad, ARRAY_SIZE(ad), + sd, ARRAY_SIZE(sd)); + } if (err) { LOG_ERR("Advertising restart failed (err %d)", err); } else { diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index 76b26b1a..06045b94 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -147,6 +147,10 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testAdvertisementServices, test_cases::test_advertisement_services ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testAdvertisementServiceData128bit, + test_cases::test_advertisement_service_data_128bit +); jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testRetrievePeripheralsNotSupported, test_cases::test_retrieve_peripherals_not_supported diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index c5a70360..8ec71f02 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -72,6 +72,7 @@ class BleIntegrationTest { @Test fun testScanFilterByServiceUuid() = NativeTests.testScanFilterByServiceUuid() @Test fun testAdvertisementManufacturerData() = NativeTests.testAdvertisementManufacturerData() @Test fun testAdvertisementServices() = NativeTests.testAdvertisementServices() + @Test fun testAdvertisementServiceData128bit() = NativeTests.testAdvertisementServiceData128bit() // ── Retrieval ──────────────────────────────────────────────────── @Test fun testRetrievePeripheralsNotSupported() = NativeTests.testRetrievePeripheralsNotSupported() diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index 5f0b733c..591f75e7 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -14,6 +14,7 @@ object NativeTests { external fun testScanFilterByServiceUuid() external fun testAdvertisementManufacturerData() external fun testAdvertisementServices() + external fun testAdvertisementServiceData128bit() // Retrieval external fun testRetrievePeripheralsNotSupported() diff --git a/tests/common/gatt_uuids.rs b/tests/common/gatt_uuids.rs index 1fcc4144..8452fc56 100644 --- a/tests/common/gatt_uuids.rs +++ b/tests/common/gatt_uuids.rs @@ -50,3 +50,4 @@ pub const STATIC_READ_VALUE: &[u8] = &[0x01, 0x02, 0x03, 0x04]; pub const TEST_PERIPHERAL_NAME: &str = "btleplug-test"; pub const TEST_APPEARANCE: u16 = 0x0340; pub const MANUFACTURER_COMPANY_ID: u16 = 0xFFFF; +pub const SERVICE_DATA_VALUE: &[u8] = &[0x01]; diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 67b30783..61aae972 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -338,6 +338,80 @@ pub async fn test_advertisement_services() { ); } +/// #483: a byte-for-byte match on the Control Service UUID in a 128-bit +/// service data advertisement proves the UUID byte order is correct. +pub async fn test_advertisement_service_data_128bit() { + use btleplug::api::{Central, CentralEvent, Peripheral as _}; + use std::time::Duration; + + let peripheral = peripheral_finder::find_and_connect().await; + peripheral_finder::reset_peripheral(&peripheral).await; + let target_id = peripheral.id(); + + // Subscribe before disconnect() so the advertisement event isn't missed. + let mut events = peripheral_finder::spawn_event_collector().await; + + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_CHANGE_ADVERTISEMENTS) + .await; + + tokio::time::timeout(Duration::from_secs(10), peripheral.disconnect()) + .await + .expect("disconnect() timed out") + .expect("disconnect() failed"); + + let matches_service_data = |event: &CentralEvent| { + matches!( + event, + CentralEvent::ServiceDataAdvertisement { id, service_data } + if *id == target_id + && service_data.get(&gatt_uuids::CONTROL_SERVICE).map(Vec::as_slice) + == Some(gatt_uuids::SERVICE_DATA_VALUE) + ) + }; + peripheral_finder::wait_for_event( + &mut events, + Duration::from_secs(15), + "ServiceDataAdvertisement(Control Service -> [0x01])", + matches_service_data, + ) + .await; + + // On CoreBluetooth this event only comes from the re-created peripheral + // (internal.rs on_peripheral_disconnect), so rediscovery already happened; + // wait_for_rediscovery would hang. + let adapter = peripheral_finder::get_adapter().await; + let rediscovered = adapter + .peripheral(&target_id) + .await + .expect("peripheral() failed for rediscovered id"); + + let props = rediscovered + .properties() + .await + .unwrap() + .expect("properties should be available after rediscovery"); + assert_eq!( + props + .service_data + .get(&gatt_uuids::CONTROL_SERVICE) + .map(Vec::as_slice), + Some(gatt_uuids::SERVICE_DATA_VALUE), + "properties().service_data should contain Control Service -> [0x01]" + ); + + // Restore the default advertising set: connecting clears alt_adv_pending + // in connected(), so the disconnect that follows restarts the default + // set (service UUID list, no service data) for later tests. + tokio::time::timeout(Duration::from_secs(10), rediscovered.connect()) + .await + .expect("connect() to restore default advertising timed out") + .expect("connect() to restore default advertising failed"); + tokio::time::timeout(Duration::from_secs(10), rediscovered.disconnect()) + .await + .expect("disconnect() to restore default advertising timed out") + .expect("disconnect() to restore default advertising failed"); +} + // ── Retrieval ────────────────────────────────────────────────────── pub async fn test_retrieve_peripherals_not_supported() { diff --git a/tests/test_advertisement_service_data_128bit.rs b/tests/test_advertisement_service_data_128bit.rs new file mode 100644 index 00000000..8636eba0 --- /dev/null +++ b/tests/test_advertisement_service_data_128bit.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_advertisement_service_data_128bit() { + common::test_cases::test_advertisement_service_data_128bit().await; +} From 12a0fd108e9bafb70c874488af4143ac6364b852 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 01:52:10 -0700 Subject: [PATCH 20/32] doc: Record Step 11 progress and backend findings Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index 5d913139..2f254b95 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -61,7 +61,7 @@ Every step runs this loop. Do not start the next step until the current step is ## Progress and Handoff -Updated: 2026-09-27, after Step 10. +Updated: 2026-09-27, after Step 11. ### Status @@ -78,9 +78,10 @@ Updated: 2026-09-27, after Step 10. | 8 | `84f9dc2` (refusing CCC is an unmanaged descriptor, not the plan's managed CCC: a managed CCC leaks Zephyr's only cfg slot on rejection) | | 9 | `b768000` (no drops or reordering observed on macOS) | | 10 | `79fb66b` (proved against both #487 panic sites separately; handles follow service variable names, see conventions) | -| 11-14 | Not started | +| 11 | `d68ff66` (test restores the default advertising set itself; skips `wait_for_rediscovery` because the service-data event implies rediscovery on CoreBluetooth) | +| 12-14 | Not started | -Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 10: 44/44. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. +Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 11: 45/45. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. ### Working conventions learned so far @@ -107,6 +108,10 @@ Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 10: 44 5. `AdapterManager` (`src/common/adapter_manager.rs`) uses a capacity-16 broadcast and `event_stream()` silently drops `Lagged`. Measured bursts of ~130 `DeviceUpdated` within ±500 ms of a disconnect during scanning. Any `events()` consumer can lose `DeviceConnected`/`DeviceDisconnected`. One unexplained cycle-2 `DeviceDisconnected` miss in Step 5 (1/5 runs, not reproduced in 25 instrumented runs) is attributed to this. 6. WinRT does not clear `ble_services` after a peripheral-triggered disconnect, so a read afterwards may trigger an implicit reconnect; check on Windows hardware. 7. `add_peripheral`'s `NotSupported` message ("Can't add a Peripheral from a PeripheralId") does not follow the operation-name convention `retrieve_peripherals` uses. +8. The BlueZ repeat-run behaviour of `test_advertisement_service_data_128bit` relies on `device_set_service_data` clearing and re-adding service data when scanning with `duplicate_data` (checked in BlueZ master; the release that added this is unknown). An older BlueZ may send no `PropertiesChanged` on a second run. +9. BlueZ never clears `ServiceData` when later advertisements omit it, so `properties().service_data` keeps showing stale entries while the device object exists. +10. If `test_advertisement_service_data_128bit` fails before its final reconnect, the alternate set persists until the next connecting test; a scan-only test run in between (`test_advertisement_services`) fails as a knock-on. +11. Firmware: `connected(err)` returns without restarting advertising, and Zephyr v4.4.2 has no auto-resume, so a failed connection would leave the peripheral silent (behaviour predates this plan). ## Steps From e8768db3a4ce050accb34ff432b77ea7832f0607 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 02:13:40 -0700 Subject: [PATCH 21/32] test(zephyr): Support and test MTU-sized notification payloads Grow the configurable notification payload from 20 to 244 bytes (CONFIG_BT_L2CAP_TX_MTU minus the ATT header). The new test_mtu_sized_notification_payload sets a distinct-byte pattern of min(mtu - 4, 243) bytes through the Control Point and asserts the notification carries it exactly. All four backends report the negotiated ATT MTU from mtu(), so the bound holds everywhere; at the default MTU the test still runs with a 19-byte payload. Co-Authored-By: Claude Opus 5.5 (1M context) --- test-peripheral/AGENTS.md | 4 +- test-peripheral/zephyr/src/gatt_profile.h | 8 ++- tests/android/rust/src/lib.rs | 4 ++ .../btleplug/test/BleIntegrationTest.kt | 1 + .../btleplug/test/NativeTests.kt | 1 + tests/common/test_cases.rs | 64 +++++++++++++++++++ tests/test_mtu_sized_notification_payload.rs | 7 ++ 7 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 tests/test_mtu_sized_notification_payload.rs diff --git a/test-peripheral/AGENTS.md b/test-peripheral/AGENTS.md index c871b577..56031dbd 100644 --- a/test-peripheral/AGENTS.md +++ b/test-peripheral/AGENTS.md @@ -1,6 +1,6 @@ # test-peripheral/ -- BLE Test Peripheral Firmware (Zephyr) -Freshness: 2026-09-26 +Freshness: 2026-09-27 ## Purpose @@ -24,7 +24,7 @@ Zephyr firmware GATT test peripheral used by the integration tests in `tests/`. - The canonical UUID source of truth is `tests/common/gatt_uuids.rs`, mirrored in `zephyr/src/gatt_profile.h`. - Four GATT services: Control (`0x0001`), Read/Write (`0x0002`), Notification (`0x0003`), Descriptor (`0x0004`). -- Control Point opcodes: `0x01` start notifications, `0x02` stop, `0x03` disconnect, `0x04` change adverts (sets a pending flag; the alternate set — flags + name advertising data, 128-bit service data + manufacturer data scan response — is used on the next advertising restart after a disconnect, and the flag clears on the next `connected()` callback or `0x05`), `0x05` reset, `0x06` set notification payload. +- Control Point opcodes: `0x01` start notifications, `0x02` stop, `0x03` disconnect, `0x04` change adverts (sets a pending flag; the alternate set — flags + name advertising data, 128-bit service data + manufacturer data scan response — is used on the next advertising restart after a disconnect, and the flag clears on the next `connected()` callback or `0x05`), `0x05` reset, `0x06` set notification payload (truncated to 244 bytes, `CONFIG_BT_L2CAP_TX_MTU` minus the 3-byte ATT header; a payload longer than the negotiated MTU minus 3 is not sent). - Peripheral advertises as `"btleplug-test"` with the Control Service UUID in the scan response. After `0x04` and a disconnect, the next advertising cycle instead carries 128-bit service data (Control Service UUID + `[0x01]`) with no service UUID list, reverting to the default set once a client connects and disconnects again. ## Invariants diff --git a/test-peripheral/zephyr/src/gatt_profile.h b/test-peripheral/zephyr/src/gatt_profile.h index 573ad3c4..d829381d 100644 --- a/test-peripheral/zephyr/src/gatt_profile.h +++ b/test-peripheral/zephyr/src/gatt_profile.h @@ -79,6 +79,12 @@ #define MANUFACTURER_COMPANY_ID 0xFFFF #define NOTIFICATION_INTERVAL_MS 1000 +/* Max configurable notification payload: CONFIG_BT_L2CAP_TX_MTU minus the + * 3-byte ATT header (opcode + handle), i.e. the largest ATT_HANDLE_VALUE_NTF + * payload at the firmware's maximum ATT MTU (247). Payloads longer than the + * negotiated MTU - 3 make bt_gatt_notify fail. */ +#define NOTIFY_PAYLOAD_MAX_LEN (CONFIG_BT_L2CAP_TX_MTU - 3) + /* Application error (0x80-0x9F); auth/encryption errors would trigger pairing. */ #define ERROR_CHAR_ATT_ERROR 0x80 @@ -97,7 +103,7 @@ struct peripheral_state { uint16_t long_value_len; uint8_t write_with_resp_value[256]; uint16_t write_with_resp_len; - uint8_t notify_payload[20]; + uint8_t notify_payload[NOTIFY_PAYLOAD_MAX_LEN]; uint16_t notify_payload_len; uint8_t rw_descriptor_value[256]; uint16_t rw_descriptor_len; diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index 06045b94..000ea251 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -223,6 +223,10 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testResubscribeDoesNotDuplicateNotifications, test_cases::test_resubscribe_does_not_duplicate_notifications ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testMtuSizedNotificationPayload, + test_cases::test_mtu_sized_notification_payload +); jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testReadOnlyDescriptor, test_cases::test_read_only_descriptor diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index 8ec71f02..79be713a 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -99,6 +99,7 @@ class BleIntegrationTest { @Test fun testUnsubscribeStopsNotifications() = NativeTests.testUnsubscribeStopsNotifications() @Test fun testConfigurableNotificationPayload() = NativeTests.testConfigurableNotificationPayload() @Test fun testResubscribeDoesNotDuplicateNotifications() = NativeTests.testResubscribeDoesNotDuplicateNotifications() + @Test fun testMtuSizedNotificationPayload() = NativeTests.testMtuSizedNotificationPayload() // ── Descriptors ───────────────────────────────────────────────── @Test fun testReadOnlyDescriptor() = NativeTests.testReadOnlyDescriptor() diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index 591f75e7..ef0df8f7 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -41,6 +41,7 @@ object NativeTests { external fun testUnsubscribeStopsNotifications() external fun testConfigurableNotificationPayload() external fun testResubscribeDoesNotDuplicateNotifications() + external fun testMtuSizedNotificationPayload() // Descriptors external fun testReadOnlyDescriptor() diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 61aae972..2898d260 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -1131,6 +1131,70 @@ pub async fn test_configurable_notification_payload() { peripheral.disconnect().await.unwrap(); } +/// Exercises a notification payload sized to the negotiated ATT MTU rather than a small constant. +pub async fn test_mtu_sized_notification_payload() { + use futures::StreamExt; + use std::time::Duration; + use tokio::time; + + let peripheral = peripheral_finder::find_and_connect().await; + peripheral_finder::reset_peripheral(&peripheral).await; + + let mtu = peripheral.mtu(); + // The Control Point write carries the opcode byte plus the payload in one ATT write (MTU - 4). + // 243 = the firmware's 247-byte max MTU - 4. + let len = std::cmp::min(mtu.saturating_sub(4) as usize, 243); + println!("test_mtu_sized_notification_payload: mtu={mtu}, payload_len={len}"); + + let payload: Vec = (0..len) + .map(|i| (i as u8).wrapping_mul(7).wrapping_add(1)) + .collect(); + + let config_char = + peripheral_finder::find_characteristic(&peripheral, gatt_uuids::CONFIGURABLE_NOTIFY); + let control_point = + peripheral_finder::find_characteristic(&peripheral, gatt_uuids::CONTROL_POINT); + + let mut cmd = vec![gatt_uuids::CMD_SET_NOTIFICATION_PAYLOAD]; + cmd.extend_from_slice(&payload); + peripheral + .write(&control_point, &cmd, btleplug::api::WriteType::WithResponse) + .await + .unwrap(); + + let mut stream = peripheral.notifications().await.unwrap(); + peripheral.subscribe(&config_char).await.unwrap(); + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_START_NOTIFICATIONS).await; + + let timeout = time::sleep(Duration::from_secs(15)); + tokio::pin!(timeout); + let mut matching = false; + let mut last_len = None; + + loop { + tokio::select! { + Some(n) = stream.next() => { + if n.uuid == gatt_uuids::CONFIGURABLE_NOTIFY { + last_len = Some(n.value.len()); + if n.value == payload { + matching = true; + break; + } + } + } + _ = &mut timeout => break, + } + } + + peripheral_finder::send_control_command(&peripheral, gatt_uuids::CMD_STOP_NOTIFICATIONS).await; + peripheral.unsubscribe(&config_char).await.unwrap(); + assert!( + matching, + "Should receive notification with the {len}-byte MTU-sized payload; last CONFIGURABLE_NOTIFY len seen: {last_len:?}" + ); + peripheral.disconnect().await.unwrap(); +} + /// Covers #326: subscribing to the same characteristic twice must not /// register a duplicate notification handler, which would deliver every /// notification more than once. diff --git a/tests/test_mtu_sized_notification_payload.rs b/tests/test_mtu_sized_notification_payload.rs new file mode 100644 index 00000000..8d6fc854 --- /dev/null +++ b/tests/test_mtu_sized_notification_payload.rs @@ -0,0 +1,7 @@ +mod common; + +#[tokio::test] +#[ignore = "requires BLE test peripheral"] +async fn test_mtu_sized_notification_payload() { + common::test_cases::test_mtu_sized_notification_payload().await; +} From c139a9d0ea1b38aef12a6ba71fc81a63371e05d8 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 02:13:40 -0700 Subject: [PATCH 22/32] scripts: Run Windows radio tests and raise integration test timeout On Windows (OS=Windows_NT or a MINGW/MSYS uname), also run the ignored winrtble::adapter::cleanup_tests radio tests (#476) serially, counted in the pass/fail summary and failed if the filter matches no tests. Build the selected test binaries once before the timed loop so compile time no longer counts against each test's timeout, and raise the default per-test TIMEOUT from 20 to 40 seconds for the reconnect and rescan tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- scripts/run-integration-tests.sh | 93 +++++++++++++++++++++++++++++--- 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cf0e22f8..ecd1dd3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,7 +15,7 @@ Last verified: 2026-09-26 - `cargo test` -- Run unit tests - `cargo test --test '*' -- --ignored` -- Run integration tests (requires test peripheral) - `scripts/run-jni-tests.sh` -- Compile Java sources and run JNI host tests on host JVM -- `scripts/run-integration-tests.sh` -- Run BLE integration tests (requires test peripheral) +- `scripts/run-integration-tests.sh` -- Run BLE integration tests (requires test peripheral); on Windows also runs the ignored `winrtble::adapter` radio lib tests (#476) - `scripts/run-integration-tests-android.sh` -- Run Android integration tests - `scripts/build-java.sh` -- Build Java/Android components diff --git a/scripts/run-integration-tests.sh b/scripts/run-integration-tests.sh index 88fb5bbd..b5d6bfc8 100755 --- a/scripts/run-integration-tests.sh +++ b/scripts/run-integration-tests.sh @@ -14,16 +14,31 @@ # BTLEPLUG_TEST_PERIPHERAL - peripheral name (default: btleplug-test) # RUST_LOG - log level (e.g. debug, btleplug=trace) # DELAY - seconds to wait between tests (default: 2) -# TIMEOUT - seconds before a test is killed (default: 20) +# TIMEOUT - seconds before a test is killed (default: 40) +# +# On Windows (Git Bash/MSYS), also runs the ignored winrtble::adapter radio +# tests from `src/winrtble/adapter.rs` (#476), which need a real Windows +# Bluetooth radio and are otherwise never exercised by this script. set -euo pipefail DELAY="${DELAY:-2}" -TIMEOUT="${TIMEOUT:-20}" +TIMEOUT="${TIMEOUT:-40}" PASSED=0 FAILED=0 FAILURES=() +# Windows detection: Git Bash/MSYS sets $OS=Windows_NT; `uname -s` there +# reports MINGW64_NT-... or MSYS_NT-.... Neither matches on macOS/Linux. +IS_WINDOWS=false +if [[ "${OS:-}" == "Windows_NT" ]] || [[ "$(uname -s)" =~ ^(MINGW|MSYS) ]]; then + IS_WINDOWS=true +fi + +# Sentinel test name for the Windows-only winrtble radio lib tests, run +# alongside the per-file integration tests below. +WINRTBLE_RADIO_TESTS="winrtble_radio_tests" + # Discover all test binaries (one per file). SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" TESTS_DIR="$(cd "$SCRIPT_DIR/../tests" && pwd)" @@ -46,6 +61,20 @@ for f in "$TESTS_DIR"/test_*.rs; do TEST_NAMES+=("$name") done +if $IS_WINDOWS; then + name="$WINRTBLE_RADIO_TESTS" + if [[ $# -gt 0 ]]; then + matched=false + for pattern in "$@"; do + # shellcheck disable=SC2254 + case "$name" in $pattern) matched=true ;; esac + done + $matched && TEST_NAMES+=("$name") + else + TEST_NAMES+=("$name") + fi +fi + if [[ ${#TEST_NAMES[@]} -eq 0 ]]; then echo "No tests matched." [[ $# -gt 0 ]] && echo "Filter: $*" @@ -57,21 +86,71 @@ echo "=== btleplug integration tests ===" echo "Running $total tests sequentially (${DELAY}s delay, ${TIMEOUT}s timeout per test)" echo "" +# Build all needed test binaries once, outside the timed loop below, so +# compile time doesn't eat into each test's TIMEOUT budget. This is +# especially important for the Windows lib-test target, which needs a +# whole-crate cfg(test) build that must otherwise fit entirely inside its +# 40s window alongside the actual test run. +echo "Building test binaries..." +prebuild_cmd=(cargo test --quiet --no-run) +run_lib_tests=false +for test_name in "${TEST_NAMES[@]}"; do + if [[ "$test_name" == "$WINRTBLE_RADIO_TESTS" ]]; then + run_lib_tests=true + else + prebuild_cmd+=(--test "$test_name") + fi +done +$run_lib_tests && prebuild_cmd+=(--lib) +"${prebuild_cmd[@]}" + test_num=0 for test_name in "${TEST_NAMES[@]}"; do test_num=$((test_num + 1)) printf "[%2d/%2d] %-55s " "$test_num" "$total" "$test_name" - if timeout "${TIMEOUT}s" cargo test --test "$test_name" -- --ignored 2>/tmp/btleplug-test-output.log; then - echo "PASS" - PASSED=$((PASSED + 1)) + if [[ "$test_name" == "$WINRTBLE_RADIO_TESTS" ]]; then + # Filter to the winrtble::adapter::cleanup_tests module path so this only + # picks up the #476 radio tests, not any other #[ignore]d lib test that + # might exist. Serialised (--test-threads=1) since these tests share a + # single physical Bluetooth radio. + cargo_cmd=(cargo test --lib "winrtble::adapter::cleanup_tests::" -- --ignored --test-threads=1) + else + cargo_cmd=(cargo test --test "$test_name" -- --ignored) + fi + + # The zero-tests guard below needs the libtest harness's "running N tests" + # line, which goes to stdout, not stderr -- so only the sentinel's + # invocation captures both streams to the log. Every other test keeps the + # existing stderr-only capture (stdout streams live to the terminal). + if [[ "$test_name" == "$WINRTBLE_RADIO_TESTS" ]]; then + timeout "${TIMEOUT}s" "${cargo_cmd[@]}" &>/tmp/btleplug-test-output.log && run_status=0 || run_status=$? + else + timeout "${TIMEOUT}s" "${cargo_cmd[@]}" 2>/tmp/btleplug-test-output.log && run_status=0 || run_status=$? + fi + + test_ok=true + if [[ $run_status -eq 0 ]]; then + # Even on a zero exit code, cargo reports "running 0 tests" if the + # module filter matched nothing -- don't let that pass silently. + if [[ "$test_name" == "$WINRTBLE_RADIO_TESTS" ]] && grep -q "running 0 tests" /tmp/btleplug-test-output.log; then + test_ok=false + echo "FAIL (0 tests matched)" + else + echo "PASS" + fi else - exit_code=$? - if [[ $exit_code -eq 124 ]]; then + test_ok=false + if [[ $run_status -eq 124 ]]; then echo "TIMEOUT (${TIMEOUT}s)" else echo "FAIL" fi + fi + + if $test_ok; then + PASSED=$((PASSED + 1)) + else FAILED=$((FAILED + 1)) FAILURES+=("$test_name") # Show output for failed tests. From a0751318a3c05e581067e969245e88f23e8912dd Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 02:14:06 -0700 Subject: [PATCH 23/32] doc: Record Steps 12 and 13 progress and pending hardware checks Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index 2f254b95..138a0a35 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -61,7 +61,7 @@ Every step runs this loop. Do not start the next step until the current step is ## Progress and Handoff -Updated: 2026-09-27, after Step 11. +Updated: 2026-09-27, after Step 13. ### Status @@ -79,14 +79,25 @@ Updated: 2026-09-27, after Step 11. | 9 | `b768000` (no drops or reordering observed on macOS) | | 10 | `79fb66b` (proved against both #487 panic sites separately; handles follow service variable names, see conventions) | | 11 | `d68ff66` (test restores the default advertising set itself; skips `wait_for_rediscovery` because the service-data event implies rediscovery on CoreBluetooth) | -| 12-14 | Not started | +| 12 | `e8768db` (**hardware run pending**; all four backends' `mtu()` return the negotiated ATT MTU, so `mtu - 4` holds) | +| 13 | `c139a9d` (prebuilds selected test binaries outside the timeout; radio tests filtered to `winrtble::adapter::cleanup_tests::`, serial, fail on zero matches; Windows path unexercised) | +| 14 | Not started | Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 11: 45/45. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. +### Pending hardware verification + +Steps 12 and 13 were committed on 2026-09-27 with the board detached, after build, lint, and review only. Before merging: + +1. Flash current firmware (`e8768db` or later) and run `./scripts/run-integration-tests.sh test_mtu_sized_notification_payload` 3 times, then the full suite (default timeout is now 40 s; this is the first run with the prebuild step). +2. Check the logged `mtu=` / payload length. `mtu=23` means the platform sent 19 bytes and did not exercise the larger buffer; on macOS an occasional 23 would mean CoreBluetooth sampled before the MTU exchange finished. Watch `test_configurable_notification_payload`, which shares `notify_payload`. +3. On failure, use PacketLogger to tell a truncated notification from none (the test message reports the last `CONFIGURABLE_NOTIFY` length seen). +4. Windows: clear the GATT cache after flashing (Steps 10 and 12 changed the layout and payload), and confirm `winrtble_radio_tests` runs 3 tests. + ### Working conventions learned so far - **Firmware tooling.** Workspace topdir is `test-peripheral/` (deps in `test-peripheral/deps/`, uv venv in `test-peripheral/.venv/`). Use `test-peripheral/.venv/bin/west` (or activate the venv; fish: `activate.fish`). Build from `test-peripheral/zephyr/`: `west build -b nrf52840dk/nrf52840 --pristine`; flash: `west flash --runner nrfjprog`. -- **Hardware runs.** Use `TIMEOUT=60 ./scripts/run-integration-tests.sh ` until Step 13 raises the default. Run new tests at least 3 times (5 for timing-dependent ones). Write repeat loops as bash scripts in the scratchpad; the tool shell is zsh and mangles things like `echo ====`. +- **Hardware runs.** `./scripts/run-integration-tests.sh ` (default per-test timeout 40 s since Step 13; binaries are prebuilt outside it). Run new tests at least 3 times (5 for timing-dependent ones). Write repeat loops as bash scripts in the scratchpad; the tool shell is zsh and mangles things like `echo ====`. - **Event helpers** in `tests/common/peripheral_finder.rs`: `spawn_event_collector` (always use it; the central event broadcast has capacity 16 and silently drops lagged events), `wait_for_event(rx, timeout, what, pred)`, `wait_for_connected` (skips stale BlueZ synthetic connection events), `wait_for_rediscovery` (no-op off Apple). Drain the receiver with `try_recv()` before starting a new phase that must not see earlier events. - **CoreBluetooth reconnect.** CoreBluetooth drops a peripheral on disconnect (issue #57); call `wait_for_rediscovery` before any reconnect after a disconnect. - **CoreBluetooth `clear_peripherals`** disconnects a connected peripheral without emitting `DeviceDisconnected` (documented contract since f3de711). From e2e30bb834e77e9cf81f9f7c437706004f5f977e Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 02:40:38 -0700 Subject: [PATCH 24/32] test: Bound follow-up operations and align Android test layout Wrap the follow-up read in test_discover_services_during_read and the unsubscribes in test_concurrent_operations_same_service in timeouts so a wedged queue fails instead of hanging (Android instrumentation has no per-test timeout). Move testAddPeripheralByAddress under Retrieval in the Android harness to match test_cases.rs, and drop the redundant Android cfg gate on test_retrieve_connected_peripheral_by_identifier to match its by_service sibling. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/android/rust/src/lib.rs | 8 +++---- .../btleplug/test/BleIntegrationTest.kt | 2 +- .../btleplug/test/NativeTests.kt | 2 +- tests/common/test_cases.rs | 22 ++++++++++++++----- ...ieve_connected_peripheral_by_identifier.rs | 1 - 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/tests/android/rust/src/lib.rs b/tests/android/rust/src/lib.rs index 000ea251..eb99433c 100644 --- a/tests/android/rust/src/lib.rs +++ b/tests/android/rust/src/lib.rs @@ -155,6 +155,10 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testRetrievePeripheralsNotSupported, test_cases::test_retrieve_peripherals_not_supported ); +jni_test!( + Java_com_nonpolynomial_btleplug_test_NativeTests_testAddPeripheralByAddress, + test_cases::test_add_peripheral_by_address +); jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testConnectAndDisconnect, test_cases::test_connect_and_disconnect @@ -275,10 +279,6 @@ jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testOperationsAcrossPeripheralTriggeredDisconnect, test_cases::test_operations_across_peripheral_triggered_disconnect ); -jni_test!( - Java_com_nonpolynomial_btleplug_test_NativeTests_testAddPeripheralByAddress, - test_cases::test_add_peripheral_by_address -); jni_test!( Java_com_nonpolynomial_btleplug_test_NativeTests_testGattErrorStatusIsReported, test_cases::test_gatt_error_status_is_reported diff --git a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt index 79be713a..32e5434c 100644 --- a/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt +++ b/tests/android/src/androidTest/kotlin/com/nonpolynomial/btleplug/test/BleIntegrationTest.kt @@ -76,6 +76,7 @@ class BleIntegrationTest { // ── Retrieval ──────────────────────────────────────────────────── @Test fun testRetrievePeripheralsNotSupported() = NativeTests.testRetrievePeripheralsNotSupported() + @Test fun testAddPeripheralByAddress() = NativeTests.testAddPeripheralByAddress() // ── Connection ────────────────────────────────────────────────── @Test fun testConnectAndDisconnect() = NativeTests.testConnectAndDisconnect() @@ -118,7 +119,6 @@ class BleIntegrationTest { @Test fun testConcurrentOperationsSameService() = NativeTests.testConcurrentOperationsSameService() @Test fun testDiscoverServicesDuringRead() = NativeTests.testDiscoverServicesDuringRead() @Test fun testOperationsAcrossPeripheralTriggeredDisconnect() = NativeTests.testOperationsAcrossPeripheralTriggeredDisconnect() - @Test fun testAddPeripheralByAddress() = NativeTests.testAddPeripheralByAddress() // ── GATT errors ────────────────────────────────────────────────── @Test fun testGattErrorStatusIsReported() = NativeTests.testGattErrorStatusIsReported() diff --git a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt index ef0df8f7..749ff8cb 100644 --- a/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt +++ b/tests/android/src/main/kotlin/com/nonpolynomial/btleplug/test/NativeTests.kt @@ -18,6 +18,7 @@ object NativeTests { // Retrieval external fun testRetrievePeripheralsNotSupported() + external fun testAddPeripheralByAddress() // Connection external fun testConnectAndDisconnect() @@ -60,7 +61,6 @@ object NativeTests { external fun testConcurrentOperationsSameService() external fun testDiscoverServicesDuringRead() external fun testOperationsAcrossPeripheralTriggeredDisconnect() - external fun testAddPeripheralByAddress() // GATT errors external fun testGattErrorStatusIsReported() diff --git a/tests/common/test_cases.rs b/tests/common/test_cases.rs index 2898d260..29c975da 100644 --- a/tests/common/test_cases.rs +++ b/tests/common/test_cases.rs @@ -497,8 +497,6 @@ pub async fn test_add_peripheral_by_address() { } } -/// Android does not support retrieval (covered by `test_retrieve_peripherals_not_supported`). -#[cfg(not(target_os = "android"))] pub async fn test_retrieve_connected_peripheral_by_identifier() { use btleplug::api::{Central, Peripheral as _, RetrievePeripheralsOptions}; @@ -1604,8 +1602,20 @@ pub async fn test_concurrent_operations_same_service() { "Static read should return [0x01, 0x02, 0x03, 0x04]" ); - peripheral.unsubscribe(¬ify_char).await.unwrap(); - peripheral.unsubscribe(&indicate_char).await.unwrap(); + timeout( + Duration::from_secs(10), + peripheral.unsubscribe(¬ify_char), + ) + .await + .expect("unsubscribe(NOTIFY_CHAR) timed out") + .unwrap(); + timeout( + Duration::from_secs(10), + peripheral.unsubscribe(&indicate_char), + ) + .await + .expect("unsubscribe(INDICATE_CHAR) timed out") + .unwrap(); peripheral.disconnect().await.unwrap(); } @@ -1631,9 +1641,9 @@ pub async fn test_discover_services_during_read() { discover_result.expect("discover_services() should succeed during concurrent read"); let static_char = peripheral_finder::find_characteristic(&peripheral, gatt_uuids::STATIC_READ); - let static_value = peripheral - .read(&static_char) + let static_value = timeout(Duration::from_secs(10), peripheral.read(&static_char)) .await + .expect("follow-up read(STATIC_READ) timed out") .expect("follow-up read(STATIC_READ) should succeed"); assert_eq!( static_value, diff --git a/tests/test_retrieve_connected_peripheral_by_identifier.rs b/tests/test_retrieve_connected_peripheral_by_identifier.rs index c0874e41..8cce5b81 100644 --- a/tests/test_retrieve_connected_peripheral_by_identifier.rs +++ b/tests/test_retrieve_connected_peripheral_by_identifier.rs @@ -1,6 +1,5 @@ mod common; -#[cfg(not(target_os = "android"))] #[tokio::test] #[ignore = "requires BLE test peripheral"] async fn test_retrieve_connected_peripheral_by_identifier() { From 7057ca56bcf3cc3298818ac6c60954f48d7b6010 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 02:40:38 -0700 Subject: [PATCH 25/32] doc: Update test suite and test peripheral docs for regression expansion tests/AGENTS.md: unambiguous test categories (each file matches one), event-helper and timeout contracts, the 40 s script timeout and prebuild, Windows radio lib tests, and the manually run CoreBluetooth lib tests. test-peripheral/AGENTS.md and README.md: per-service characteristics, the included secondary service, SORT_BY_NAME handle order, opcode 0x04 advertisement rotation, and the 244-byte payload cap. Record Step 14 progress, pending hardware checks, and firmware findings in the plan. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- docs/plan-zephyr-regression-tests.md | 10 ++++++++-- test-peripheral/AGENTS.md | 8 +++++++- test-peripheral/README.md | 5 +++-- tests/AGENTS.md | 28 ++++++++++++++++++---------- 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ecd1dd3c..22d21ec3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,6 @@ # btleplug -Last verified: 2026-09-26 +Last verified: 2026-09-27 ## Tech Stack diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index 138a0a35..bf30b951 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -61,7 +61,7 @@ Every step runs this loop. Do not start the next step until the current step is ## Progress and Handoff -Updated: 2026-09-27, after Step 13. +Updated: 2026-09-27, after Step 14. ### Status @@ -81,7 +81,7 @@ Updated: 2026-09-27, after Step 13. | 11 | `d68ff66` (test restores the default advertising set itself; skips `wait_for_rediscovery` because the service-data event implies rediscovery on CoreBluetooth) | | 12 | `e8768db` (**hardware run pending**; all four backends' `mtu()` return the negotiated ATT MTU, so `mtu - 4` holds) | | 13 | `c139a9d` (prebuilds selected test binaries outside the timeout; radio tests filtered to `winrtble::adapter::cleanup_tests::`, serial, fail on zero matches; Windows path unexercised) | -| 14 | Not started | +| 14 | follow-up fixes and docs below; final range review done (no Critical issues); macOS full hardware run **pending** (board detached) | Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 11: 45/45. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. @@ -93,6 +93,9 @@ Steps 12 and 13 were committed on 2026-09-27 with the board detached, after buil 2. Check the logged `mtu=` / payload length. `mtu=23` means the platform sent 19 bytes and did not exercise the larger buffer; on macOS an occasional 23 would mean CoreBluetooth sampled before the MTU exchange finished. Watch `test_configurable_notification_payload`, which shares `notify_payload`. 3. On failure, use PacketLogger to tell a truncated notification from none (the test message reports the last `CONFIGURABLE_NOTIFY` length seen). 4. Windows: clear the GATT cache after flashing (Steps 10 and 12 changed the layout and payload), and confirm `winrtble_radio_tests` runs 3 tests. +5. `cargo test --lib corebluetooth::internal::tests -- --ignored` on macOS (Bluetooth permission, no peripheral): the four `05b1922` unit tests are never run by the script. +6. Run `test_advertisement_service_data_128bit` then `test_advertisement_services` back to back a few times to confirm the default advertising set returns within the script's inter-test gap. +7. First runs on other backends: concurrent connect on BlueZ (the `InProgress` gate), `test_refused_subscribe_returns_error` on BlueZ/WinRT/Android, `test_add_peripheral_by_address` on Windows/Android, `test_retrieve_connected_peripheral_by_identifier` on BlueZ/WinRT, `Error::NotConnected` in `test_operations_across_peripheral_triggered_disconnect` on Android, and service-data repeat runs on BlueZ (findings 8, 9). ### Working conventions learned so far @@ -123,6 +126,8 @@ Steps 12 and 13 were committed on 2026-09-27 with the board detached, after buil 9. BlueZ never clears `ServiceData` when later advertisements omit it, so `properties().service_data` keeps showing stale entries while the device object exists. 10. If `test_advertisement_service_data_128bit` fails before its final reconnect, the alternate set persists until the next connecting test; a scan-only test run in between (`test_advertisement_services`) fails as a knock-on. 11. Firmware: `connected(err)` returns without restarting advertising, and Zephyr v4.4.2 has no auto-resume, so a failed connection would leave the peripheral silent (behaviour predates this plan). +12. Firmware: `write_control_point` ignores `offset`. Zephyr v4.4.2 queues prepare fragments without calling the callback (the attribute lacks `BT_GATT_PERM_PREPARE_WRITE`, `att.c:2247`), then calls it once on execute with the joined value and the first fragment's offset. A long Control Point write therefore works, but a prepare queue starting at a non-zero offset would be run as if it started at offset 0. Fix: reject `offset != 0`. The MTU test caps at `mtu - 4` so the command fits in one Write Request. +13. Firmware: `disconnected()` unrefs `g_state.conn` before stopping periodic notifications, which may still be using it on the system workqueue (fix: stop notifications before the unref). `k_work_cancel_delayable` does not wait for a running handler, so reset can clear `notify_payload` during `bt_gatt_notify` (fix: `k_work_cancel_delayable_sync`). 0x06 writes the payload from the BT RX WQ with no synchronisation against the handler at all (fix: stage the payload and swap it on the system workqueue, or guard it with a lock). All predate this plan; tests avoid them by setting the payload after reset and before 0x01. ## Steps @@ -276,6 +281,7 @@ Tests: - Update `tests/AGENTS.md` test categories and freshness date for the new tests (concurrency, errors, advertisement rotation). - Update `test-peripheral/AGENTS.md` with the new characteristics, secondary service, opcode 0x04 behaviour, and payload size. Its "Four GATT services" line (~:26) is now four primary plus one secondary; also note the `SORT_BY_NAME` handle ordering. - `tests/AGENTS.md`: the Discovery glob `test_discover_*.rs` misses `test_discovery_with_included_service.rs`. +- `test-peripheral/README.md`: 0x04 row, 0x06 payload size, and the Services table (added by the final range review). - Run a final `opus` review over the whole range of commits from Step 1 to here, and a full hardware run of `./scripts/run-integration-tests.sh` on macOS with the nRF52840. Record any platform that could not be run. **Commit:** `doc: Update test suite and test peripheral docs for regression expansion` diff --git a/test-peripheral/AGENTS.md b/test-peripheral/AGENTS.md index 56031dbd..a2e4ed68 100644 --- a/test-peripheral/AGENTS.md +++ b/test-peripheral/AGENTS.md @@ -23,7 +23,12 @@ Zephyr firmware GATT test peripheral used by the integration tests in `tests/`. ## Contracts - The canonical UUID source of truth is `tests/common/gatt_uuids.rs`, mirrored in `zephyr/src/gatt_profile.h`. -- Four GATT services: Control (`0x0001`), Read/Write (`0x0002`), Notification (`0x0003`), Descriptor (`0x0004`). +- Five profile GATT services (plus Zephyr's built-in GAP/GATT services): four primary services (Control `0x0001`, Read/Write `0x0002`, Notification `0x0003`, Descriptor `0x0004`) and one secondary service (Included `0x0005`). The Included service is included by the Descriptor service. +- Control service characteristics: Control Point `0x0101` (write; opcodes below), Control Response `0x0102` (notify; never sent). +- Read/Write service characteristics: Static Read `0x0201` (read-only `[0x01, 0x02, 0x03, 0x04]`), Counter Read `0x0202` (read-only, increments on each read, u32 LE, 0 after reset), Write With Response `0x0203` (write-only; value not readable), Write Without Response `0x0204` (write-without-response only; stores into the Read/Write `0x0205` value and updates Write Log), Read/Write `0x0205`, Long Value `0x0206` (read/write, up to 512 bytes; empty after reset), Error Char `0x0207` (read and write always fail with ATT application error `0x80`), Write Log Char `0x0208` (read-only `[count_lo, count_hi, last_seq, out_of_order]` for write-without-response ordering). +- Notification service characteristics: Notify `0x0301`, Indicate `0x0302`, Configurable Notify `0x0303`, Refused Notify `0x0304` (its CCC is an unmanaged descriptor: reads `0x0000`, every write rejected with Write Request Rejected). +- Descriptor service: include declaration for the Included service at `attrs[1]`, then Descriptor Test Char `0x0401` (read-only `[0x00]`, with Read-Only Descriptor `0x04A1` `[DE AD BE EF]` and Read/Write Descriptor `0x04A2`). +- Included service: Included Char `0x0501` (read-only `[0x05]`). - Control Point opcodes: `0x01` start notifications, `0x02` stop, `0x03` disconnect, `0x04` change adverts (sets a pending flag; the alternate set — flags + name advertising data, 128-bit service data + manufacturer data scan response — is used on the next advertising restart after a disconnect, and the flag clears on the next `connected()` callback or `0x05`), `0x05` reset, `0x06` set notification payload (truncated to 244 bytes, `CONFIG_BT_L2CAP_TX_MTU` minus the 3-byte ATT header; a payload longer than the negotiated MTU minus 3 is not sent). - Peripheral advertises as `"btleplug-test"` with the Control Service UUID in the scan response. After `0x04` and a disconnect, the next advertising cycle instead carries 128-bit service data (Control Service UUID + `[0x01]`) with no service UUID list, reverting to the default set once a client connects and disconnects again. @@ -31,3 +36,4 @@ Zephyr firmware GATT test peripheral used by the integration tests in `tests/`. - Adding a new GATT characteristic requires updating both `tests/common/gatt_uuids.rs` and `zephyr/src/gatt_profile.h`. - New characteristics are appended to the end of existing services so the hardcoded `notify_svc.attrs[]` indices in `control_service.c` stay valid, and update the index table comment in `periodic_notify_handler` (`control_service.c`) when `notify_svc` grows. +- Static GATT services get handles in `SORT_BY_NAME` order of their `BT_GATT_SERVICE_DEFINE` variable names (control, descriptor, included, notify, rw), not source order. Zephyr's `_1_gatt_svc`/`_2_gap_svc` sort first and take the lowest handles. Only relative `attrs[]` indices within a service (as `control_service.c` uses for `notify_svc`) are safe to hardcode. diff --git a/test-peripheral/README.md b/test-peripheral/README.md index f7f4025c..0f03a418 100644 --- a/test-peripheral/README.md +++ b/test-peripheral/README.md @@ -82,6 +82,7 @@ The Zephyr firmware implements the test GATT profile. The canonical UUID definit | Read/Write Test | `00000002-b5a3-f393-e0a9-e50e24dcca9e` | Read and write characteristic operations | | Notification Test | `00000003-b5a3-f393-e0a9-e50e24dcca9e` | Notify and indicate operations | | Descriptor Test | `00000004-b5a3-f393-e0a9-e50e24dcca9e` | Descriptor read/write operations | +| Included (secondary) | `00000005-b5a3-f393-e0a9-e50e24dcca9e` | Included Char `00000501-...` (read-only `[0x05]`); included from the Descriptor service | ### Control Commands @@ -92,9 +93,9 @@ Write these opcodes to the Control Point characteristic (`00000101-...`): | `0x01` | Start Notifications | Begin periodic notifications (1 Hz) | | `0x02` | Stop Notifications | Stop all periodic notifications | | `0x03` | Trigger Disconnect | Peripheral disconnects after 500ms | -| `0x04` | Change Advertisements | Reserved; currently a no-op (logged only) | +| `0x04` | Change Advertisements | Sets a pending flag; on the next advertising restart after a disconnect, the peripheral advertises an alternate set (flags + name advertising data, 128-bit service data + manufacturer data scan response) instead of the default. Cleared by the next connection or by `0x05` | | `0x05` | Reset State | Stop notifications, clear all buffers | -| `0x06` | Set Notification Payload | Remaining bytes become the notification payload | +| `0x06` | Set Notification Payload | Remaining bytes become the notification payload, truncated to 244 bytes (`CONFIG_BT_L2CAP_TX_MTU` minus the 3-byte ATT header) | ## Environment Variables diff --git a/tests/AGENTS.md b/tests/AGENTS.md index fa4db3a3..0977f23b 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -1,6 +1,6 @@ # tests/ -- Integration Test Suite -Freshness: 2026-09-26 +Freshness: 2026-09-27 ## Purpose @@ -12,7 +12,7 @@ Each test is its own file (and therefore its own binary), ensuring process isola - `common/` -- shared test helpers (imported via `mod common;`) - `gatt_uuids.rs` -- canonical UUID constants for the test GATT profile (base UUID: `XXXXXXXX-b5a3-f393-e0a9-e50e24dcca9e`) - - `peripheral_finder.rs` -- discover, connect, and control the test peripheral + - `peripheral_finder.rs` -- discover, connect, and control the test peripheral; event helpers (`spawn_event_collector`, `wait_for_event`, `wait_for_connected`, `wait_for_rediscovery`) - `test_cases.rs` -- async test bodies shared between desktop and Android harnesses - `mod.rs` -- also contains `find_descriptor()` helper for descriptor tests - `test_*.rs` -- one test per file, thin wrapper calling `common::test_cases::*` @@ -24,20 +24,26 @@ Each test is its own file (and therefore its own binary), ensuring process isola ### Test categories -- **Discovery**: `test_discover_*.rs`, `test_scan_*.rs`, `test_advertisement_*.rs` +- **Discovery**: `test_discover_characteristics.rs`, `test_discover_peripheral_by_name.rs`, `test_discover_services.rs`, `test_discovery_with_included_service.rs`, `test_scan_*.rs`, `test_advertisement_*.rs` - **Connection**: `test_connect_*.rs`, `test_reconnect_*.rs`, `test_peripheral_triggered_*.rs` -- **Read/Write**: `test_read_*.rs`, `test_write_*.rs`, `test_long_value_*.rs`, `test_characteristic_properties.rs` -- **Notifications**: `test_subscribe_*.rs`, `test_unsubscribe_*.rs`, `test_configurable_notification_*.rs`, `test_resubscribe_*.rs` -- **Descriptors**: `test_*_descriptor*.rs` -- **Device Info**: `test_mtu_*.rs`, `test_read_rssi.rs`, `test_properties_*.rs`, `test_connection_parameters.rs`, `test_request_connection_parameters.rs` +- **Read/Write**: `test_read_static_value.rs`, `test_read_counter_increments.rs`, `test_read_write_roundtrip.rs`, `test_write_*.rs`, `test_long_value_*.rs`, `test_characteristic_properties.rs` +- **Notifications**: `test_subscribe_*.rs`, `test_unsubscribe_*.rs`, `test_configurable_notification_*.rs`, `test_resubscribe_*.rs`, `test_mtu_sized_notification_payload.rs` +- **Descriptors**: `test_*descriptor*.rs` +- **Device Info**: `test_mtu_after_service_discovery.rs`, `test_read_rssi.rs`, `test_properties_*.rs`, `test_connection_parameters.rs`, `test_request_connection_parameters.rs` +- **Concurrency**: `test_concurrent_*.rs`, `test_discover_services_during_read.rs` +- **Errors**: `test_gatt_error_*.rs`, `test_refused_*.rs`, `test_operations_across_peripheral_triggered_disconnect.rs` +- **Retrieval/Adapter**: `test_adapter_*.rs`, `test_add_peripheral_*.rs`, `test_clear_peripherals_*.rs`, `test_retrieve_*.rs` ## Contracts -- Every peripheral-backed test file uses `find_and_connect()` from `peripheral_finder.rs` to get a connected peripheral with services discovered. -- Adapter-only scenarios may use `get_adapter()` without `find_and_connect()`; they still require local adapter hardware but do not require the btleplug test peripheral. -- Tests that mutate peripheral state must call `reset_peripheral()` in setup to ensure clean state. +- Tests that need a connection use `find_and_connect()`; scan/advertisement tests use `get_adapter()` and scan, but still require the peripheral advertising. +- Tests that mutate peripheral state must call `reset_peripheral()` right after `find_and_connect()` to ensure clean state; tests that send `0x04` restore the default advertising set themselves. - Control commands are sent via the Control Point characteristic (UUID `00000101-...`) using `send_control_command()`. - The env var `BTLEPLUG_TEST_PERIPHERAL` overrides the default peripheral name (`btleplug-test`). +- Observe adapter events only through `spawn_event_collector()` (the capacity-16 broadcast drops lagged events). Drain with `try_recv()` before a phase that must not see earlier events. +- After any disconnect, call `wait_for_rediscovery()` before reconnecting (CoreBluetooth drops the peripheral on disconnect). +- Wrap concurrent and error-path operations in `tokio::time::timeout` (typically 10-15 s; shorter where the test expects a quick failure or fast path) so a regression fails instead of hanging. +- Platform `cfg` gates need a one-line reason citing backend source; never weaken an assertion without evidence. ## Dependencies @@ -53,3 +59,5 @@ Each test is its own file (and therefore its own binary), ensuring process isola - The scan timeout is 15 seconds (hardcoded in `peripheral_finder.rs`). - When adding a new test, also add the corresponding JNI export in `android/rust/src/lib.rs`, native declaration in `NativeTests.kt`, and `@Test` in `BleIntegrationTest.kt`. Tests backed by APIs Android does not support (e.g. `test_retrieve_connected_peripheral_by_service`, `test_retrieve_connected_peripheral_by_identifier`) and macOS-only tests intentionally have no JNI export. - Adapter-only tests require local adapter hardware but do not require the btleplug test peripheral; their desktop assertions are target-specific because CoreBluetooth and ordinary Android intentionally return `Ok(None)`. +- Each test must finish within `scripts/run-integration-tests.sh`'s 40 s per-test `TIMEOUT` (binaries are prebuilt outside it); on Windows the script also runs the ignored `winrtble::adapter::cleanup_tests` radio lib tests. +- The ignored CoreBluetooth lib tests in `src/corebluetooth/internal/tests.rs` (`#[ignore = "requires CoreBluetooth (creates a real CBCentralManager)"]`) are run manually on macOS with `cargo test --lib corebluetooth::internal::tests -- --ignored` (needs Bluetooth permission, no peripheral). From 012fb06776f27ceb942b052cb8af55397e48b624 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 11:50:28 -0700 Subject: [PATCH 26/32] doc: Record macOS hardware results for Steps 12-14 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index bf30b951..f1f27a0b 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -79,15 +79,15 @@ Updated: 2026-09-27, after Step 14. | 9 | `b768000` (no drops or reordering observed on macOS) | | 10 | `79fb66b` (proved against both #487 panic sites separately; handles follow service variable names, see conventions) | | 11 | `d68ff66` (test restores the default advertising set itself; skips `wait_for_rediscovery` because the service-data event implies rediscovery on CoreBluetooth) | -| 12 | `e8768db` (**hardware run pending**; all four backends' `mtu()` return the negotiated ATT MTU, so `mtu - 4` holds) | +| 12 | `e8768db` (macOS: `mtu=247`, 243-byte payload, 5/5 after one post-replug discovery failure; all four backends' `mtu()` return the negotiated ATT MTU, so `mtu - 4` holds) | | 13 | `c139a9d` (prebuilds selected test binaries outside the timeout; radio tests filtered to `winrtble::adapter::cleanup_tests::`, serial, fail on zero matches; Windows path unexercised) | -| 14 | follow-up fixes and docs below; final range review done (no Critical issues); macOS full hardware run **pending** (board detached) | +| 14 | follow-up fixes and docs below; final range review done (no Critical issues); macOS full hardware run 46/46 on 2026-09-27 | -Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 11: 45/45. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. +Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 11: 45/45. After Step 14: 46/46. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. ### Pending hardware verification -Steps 12 and 13 were committed on 2026-09-27 with the board detached, after build, lint, and review only. Before merging: +Steps 12 and 13 were committed on 2026-09-27 with the board detached. macOS results the same day, after reflashing: items 1, 2, 5, and 6 done (full suite 46/46 with the prebuild and 40 s timeout; `mtu=247`, payload 243; CoreBluetooth ignored lib tests 4/4; advertisement pair 3/3). The first MTU run after replugging the board failed in `find_and_connect()` with `discover_services: Device disconnected`; not reproduced in two reflash-then-connect cycles or later runs. Items 3, 4, and 7 remain for other platforms or failures: 1. Flash current firmware (`e8768db` or later) and run `./scripts/run-integration-tests.sh test_mtu_sized_notification_payload` 3 times, then the full suite (default timeout is now 40 s; this is the first run with the prebuild step). 2. Check the logged `mtu=` / payload length. `mtu=23` means the platform sent 19 bytes and did not exercise the larger buffer; on macOS an occasional 23 would mean CoreBluetooth sampled before the MTU exchange finished. Watch `test_configurable_notification_payload`, which shares `notify_payload`. From 39a7e2d62cc0294fa143d408c75293b6d14087d0 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 12:05:21 -0700 Subject: [PATCH 27/32] fix(bluez): Treat unsubscribe without a notify session as success BlueZ tracks notify sessions per D-Bus client and answers StopNotify from a client with no session with org.bluez.Error.Failed "No notify session started", so a repeated unsubscribe failed on Linux while it succeeds on CoreBluetooth, WinRT, and Android. Treat that specific error as success and document that unsubscribing a characteristic that is not subscribed is not an error. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/api/mod.rs | 1 + src/bluez/peripheral.rs | 38 ++++++++++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/api/mod.rs b/src/api/mod.rs index dc71028a..ad7dab7d 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -406,6 +406,7 @@ pub trait Peripheral: Send + Sync + Clone + Debug { async fn subscribe(&self, characteristic: &Characteristic) -> Result<()>; /// Disables either notify or indicate (depending on support) for the specified characteristic. + /// Unsubscribing a characteristic that is not currently subscribed is not an error. async fn unsubscribe(&self, characteristic: &Characteristic) -> Result<()>; /// Returns a stream of notifications for characteristic value updates. The stream will receive diff --git a/src/bluez/peripheral.rs b/src/bluez/peripheral.rs index 412212ad..5fa134f2 100644 --- a/src/bluez/peripheral.rs +++ b/src/bluez/peripheral.rs @@ -1,8 +1,8 @@ use async_trait::async_trait; use bluez_async::{ - BluetoothEvent, BluetoothSession, CharacteristicEvent, CharacteristicFlags, CharacteristicId, - CharacteristicInfo, DescriptorInfo, DeviceId, DeviceInfo, MacAddress, ServiceInfo, - WriteOptions, + BluetoothError, BluetoothEvent, BluetoothSession, CharacteristicEvent, CharacteristicFlags, + CharacteristicId, CharacteristicInfo, DescriptorInfo, DeviceId, DeviceInfo, MacAddress, + ServiceInfo, WriteOptions, }; use futures::future::{join_all, ready}; use futures::stream::{Stream, StreamExt}; @@ -267,7 +267,10 @@ impl api::Peripheral for Peripheral { async fn unsubscribe(&self, characteristic: &Characteristic) -> Result<()> { let characteristic_info = self.characteristic_info(characteristic)?; - Ok(self.session.stop_notify(&characteristic_info.id).await?) + match self.session.stop_notify(&characteristic_info.id).await { + Err(BluetoothError::DbusError(e)) if is_no_notify_session(&e) => Ok(()), + result => Ok(result?), + } } async fn notifications(&self) -> Result + Send>>> { @@ -439,3 +442,30 @@ impl From for CharPropFlags { result } } + +/// BlueZ tracks notify sessions per D-Bus client and rejects `StopNotify` when this client has none +/// (e.g. a repeated unsubscribe). There is nothing for this client to stop, so treat it as success. +fn is_no_notify_session(error: &dbus::Error) -> bool { + error.name() == Some("org.bluez.Error.Failed") + && error.message() == Some("No notify session started") +} + +#[cfg(test)] +mod tests { + use super::is_no_notify_session; + + #[test] + fn no_notify_session_error_is_recognised() { + let error = dbus::Error::new_custom("org.bluez.Error.Failed", "No notify session started"); + assert!(is_no_notify_session(&error)); + } + + #[test] + fn other_bluez_failures_are_not_recognised() { + let error = dbus::Error::new_custom("org.bluez.Error.Failed", "Not connected"); + assert!(!is_no_notify_session(&error)); + let error = + dbus::Error::new_custom("org.bluez.Error.NotPermitted", "No notify session started"); + assert!(!is_no_notify_session(&error)); + } +} From 8d70ba156a86f1b2ccf42a9d16809c16233e6811 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 12:05:21 -0700 Subject: [PATCH 28/32] doc: Record Linux run and cross-check tooling Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/plan-zephyr-regression-tests.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/plan-zephyr-regression-tests.md b/docs/plan-zephyr-regression-tests.md index f1f27a0b..e093b1f8 100644 --- a/docs/plan-zephyr-regression-tests.md +++ b/docs/plan-zephyr-regression-tests.md @@ -83,7 +83,7 @@ Updated: 2026-09-27, after Step 14. | 13 | `c139a9d` (prebuilds selected test binaries outside the timeout; radio tests filtered to `winrtble::adapter::cleanup_tests::`, serial, fail on zero matches; Windows path unexercised) | | 14 | follow-up fixes and docs below; final range review done (no Critical issues); macOS full hardware run 46/46 on 2026-09-27 | -Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 11: 45/45. After Step 14: 46/46. Only macOS has been run on hardware; Windows, Linux, and Android branches are unverified. +Baseline before Step 1: 32/32 hardware tests passing on macOS. After Step 11: 45/45. After Step 14: 46/46. Hardware coverage: macOS full suite; Linux full suite run by the user on 2026-09-27 with one failure (`test_unsubscribe_stops_notifications`, fixed in the BlueZ backend, re-run pending). Windows and Android are unverified. ### Pending hardware verification @@ -112,6 +112,7 @@ Steps 12 and 13 were committed on 2026-09-27 with the board detached. macOS resu - **Plan text vs. source.** Check plan claims against the source before implementing: Step 6's `NotSupported("add_peripheral")` string and Step 7's `DeviceDisconnected` expectation were both wrong. - **GATT handle order.** Static services are placed by `SORT_BY_NAME` on the `BT_GATT_SERVICE_DEFINE` variable name, not source order (current order: control, descriptor, included, notify, rw). Only relative `attrs[]` indices are safe to hardcode. - **Regression proofs.** When a fix removed several panic or error sites, reintroduce them one at a time: an earlier site masks later ones. Add a control test (an existing test on the same broken code) to show the new test is what reaches the site. +- **Linux type-check on macOS.** `libdbus-sys` refuses to cross-build, but a check-only build works with a stub `dbus-1.pc` (`Name: dbus-1`, `Version: 1.14.10`, `Libs: -ldbus-1`): `PKG_CONFIG_PATH= PKG_CONFIG_ALLOW_CROSS=1 cargo clippy --target x86_64-unknown-linux-gnu --lib --tests`. Linking and running still need Linux. ### Findings outside this plan (report to the user; not fixed) From 3741af001940fce398640c919b5aa3c382dd9971 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 13:37:30 -0700 Subject: [PATCH 29/32] examples: Add WinRT connected-device retrieval timing diagnostic Connects to the test peripheral, then times each WinRT call that retrieve_peripherals makes for service-filtered retrieval (connected device enumeration, FromIdAsync, and cached GATT service lookup) per connected device. Co-Authored-By: Claude Opus 5.5 (1M context) --- examples/retrieve_timing.rs | 97 +++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 examples/retrieve_timing.rs diff --git a/examples/retrieve_timing.rs b/examples/retrieve_timing.rs new file mode 100644 index 00000000..27412099 --- /dev/null +++ b/examples/retrieve_timing.rs @@ -0,0 +1,97 @@ +#[cfg(target_os = "windows")] +#[tokio::main] +async fn main() -> Result<(), Box> { + use btleplug::api::{Central, Manager as _, Peripheral as _, ScanFilter}; + use btleplug::platform::Manager; + use std::future::IntoFuture; + use std::time::{Duration, Instant}; + use windows::Devices::Bluetooth::{ + BluetoothCacheMode, BluetoothConnectionStatus, BluetoothLEDevice, + }; + use windows::Devices::Enumeration::DeviceInformation; + + let name = std::env::var("BTLEPLUG_TEST_PERIPHERAL").unwrap_or("btleplug-test".into()); + let manager = Manager::new().await?; + let adapter = manager + .adapters() + .await? + .into_iter() + .next() + .ok_or("no adapter")?; + adapter.start_scan(ScanFilter::default()).await?; + let deadline = Instant::now() + Duration::from_secs(15); + let peripheral = loop { + let mut found = None; + for p in adapter.peripherals().await? { + if let Some(props) = p.properties().await? { + if props.local_name.as_deref() == Some(name.as_str()) { + found = Some(p); + } + } + } + if let Some(p) = found { + break p; + } + if Instant::now() > deadline { + return Err(format!("{name} not found").into()); + } + tokio::time::sleep(Duration::from_millis(250)).await; + }; + adapter.stop_scan().await?; + let t = Instant::now(); + peripheral.connect().await?; + peripheral.discover_services().await?; + println!( + "connected to {name} ({}) in {:?}", + peripheral.id(), + t.elapsed() + ); + + let t = Instant::now(); + let selector = BluetoothLEDevice::GetDeviceSelectorFromConnectionStatus( + BluetoothConnectionStatus::Connected, + )?; + let devices = DeviceInformation::FindAllAsyncAqsFilter(&selector)? + .into_future() + .await?; + println!( + "FindAllAsyncAqsFilter: {} devices in {:?}", + devices.Size()?, + t.elapsed() + ); + + for info in devices { + let label = format!("{} [{}]", info.Name()?, info.Id()?); + let t = Instant::now(); + let device = BluetoothLEDevice::FromIdAsync(&info.Id()?)? + .into_future() + .await?; + println!( + " {label}\n FromIdAsync: {:?} (address {:012x})", + t.elapsed(), + device.BluetoothAddress()? + ); + let t = Instant::now(); + match device + .GetGattServicesWithCacheModeAsync(BluetoothCacheMode::Cached)? + .into_future() + .await + { + Ok(result) => println!( + " GetGattServices(Cached): {:?}, status {:?}, {} services", + t.elapsed(), + result.Status()?, + result.Services()?.Size()? + ), + Err(e) => println!(" GetGattServices(Cached): {:?}, error {e}", t.elapsed()), + } + } + + peripheral.disconnect().await?; + Ok(()) +} + +#[cfg(not(target_os = "windows"))] +fn main() { + eprintln!("retrieve_timing only runs on Windows"); +} From d5735e146189fe97d8246a13a6f0af54db240dc4 Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 13:42:57 -0700 Subject: [PATCH 30/32] scripts: Bring the PowerShell test runner in line with the bash runner Default the per-test timeout to 40 s, prebuild the selected test binaries outside the timed loop, and run the ignored winrtble radio lib tests (#476) serially with a zero-matched-tests guard. Kill timed-out tests with taskkill /T: Process.Kill(bool) does not exist on Windows PowerShell 5.1, so the old kill threw into an empty catch and the script waited for the test to finish while reporting TIMEOUT. All post-kill waits are bounded, and native command exit codes are checked explicitly. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 1 + scripts/run-integration-tests.ps1 | 95 ++++++++++++++++++++++++++++--- tests/AGENTS.md | 2 +- 3 files changed, 88 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 22d21ec3..72ddbc8c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,7 @@ Last verified: 2026-09-27 - `cargo test --test '*' -- --ignored` -- Run integration tests (requires test peripheral) - `scripts/run-jni-tests.sh` -- Compile Java sources and run JNI host tests on host JVM - `scripts/run-integration-tests.sh` -- Run BLE integration tests (requires test peripheral); on Windows also runs the ignored `winrtble::adapter` radio lib tests (#476) +- `scripts/run-integration-tests.ps1` -- Windows PowerShell equivalent of the above (same prebuild, radio-test sentinel, and timeout behavior) - `scripts/run-integration-tests-android.sh` -- Run Android integration tests - `scripts/build-java.sh` -- Build Java/Android components diff --git a/scripts/run-integration-tests.ps1 b/scripts/run-integration-tests.ps1 index 2e12c777..a7d31b7c 100644 --- a/scripts/run-integration-tests.ps1 +++ b/scripts/run-integration-tests.ps1 @@ -13,7 +13,12 @@ # BTLEPLUG_TEST_PERIPHERAL - peripheral name (default: btleplug-test) # RUST_LOG - log level (e.g. debug, btleplug=trace) # DELAY - seconds to wait between tests (default: 2) -# TIMEOUT - seconds before a test is killed (default: 20) +# TIMEOUT - seconds before a test is killed (default: 40) +# +# This script only runs on Windows, so it also always runs the ignored +# winrtble::adapter radio tests from `src/winrtble/adapter.rs` (#476), which +# need a real Windows Bluetooth radio and are otherwise never exercised by +# this script. [CmdletBinding()] param( @@ -22,9 +27,11 @@ param( ) $ErrorActionPreference = 'Stop' +# Native command exit codes are checked explicitly; never let PS 7.3+ turn them into exceptions. +$PSNativeCommandUseErrorActionPreference = $false $Delay = if ($env:DELAY) { [int]$env:DELAY } else { 2 } -$Timeout = if ($env:TIMEOUT) { [int]$env:TIMEOUT } else { 20 } +$Timeout = if ($env:TIMEOUT) { [int]$env:TIMEOUT } else { 40 } $Passed = 0 $Failed = 0 $Failures = @() @@ -37,6 +44,14 @@ $TestNames = Get-ChildItem -Path $TestsDir -Filter 'test_*.rs' -Name | ForEach-Object { $_ -replace '\.rs$', '' } | Sort-Object +# Sentinel test name for the Windows-only winrtble radio lib tests, run +# alongside the per-file integration tests below. This script only runs on +# Windows, so always add it, subject to the same -like filter as the +# file-based tests (e.g. a filter of `test_read_*` excludes it, and a filter +# of `winrtble*` selects only it). +$WinrtbleRadioTests = 'winrtble_radio_tests' +$TestNames = @($TestNames) + $WinrtbleRadioTests + # Apply filter if provided. if ($Filter) { $TestNames = $TestNames | Where-Object { @@ -56,6 +71,30 @@ Write-Host "=== btleplug integration tests ===" Write-Host "Running $Total tests sequentially (${Delay}s delay, ${Timeout}s timeout per test)" Write-Host "" +# Build all needed test binaries once, outside the timed loop, so compile time +# doesn't count against each test's TIMEOUT. Output streams to the console; the +# child scope's 'Continue' keeps hosts that capture native stderr (ISE, +# remoting, a redirected caller) from turning cargo warnings into errors. +Write-Host "Building test binaries..." +$PrebuildArgs = @('test', '--quiet', '--no-run') +$RunLibTests = $false +foreach ($testName in $TestNames) { + if ($testName -eq $WinrtbleRadioTests) { + $RunLibTests = $true + } else { + $PrebuildArgs += @('--test', $testName) + } +} +if ($RunLibTests) { $PrebuildArgs += '--lib' } + +& { + $ErrorActionPreference = 'Continue' + & cargo @PrebuildArgs +} +if ($LASTEXITCODE -ne 0) { + exit $LASTEXITCODE +} + $LogFile = Join-Path $env:TEMP 'btleplug-test-output.log' $TestNum = 0 @@ -63,12 +102,22 @@ foreach ($testName in $TestNames) { $TestNum++ Write-Host -NoNewline ("[{0,2}/{1,2}] {2,-55} " -f $TestNum, $Total, $testName) + if ($testName -eq $WinrtbleRadioTests) { + # Filter to the winrtble::adapter::cleanup_tests module path so this + # only picks up the #476 radio tests, not any other #[ignore]d lib + # test that might exist. Serialised (--test-threads=1) since these + # tests share a single physical Bluetooth radio. + $CargoArgsStr = 'test --lib winrtble::adapter::cleanup_tests:: -- --ignored --test-threads=1' + } else { + $CargoArgsStr = "test --test $testName -- --ignored" + } + # Run cargo test with a timeout. # Use [System.Diagnostics.Process] directly: Start-Process -PassThru opens # the handle without PROCESS_QUERY_INFORMATION, making ExitCode unavailable. $psi = [System.Diagnostics.ProcessStartInfo]@{ FileName = 'cargo' - Arguments = "test --test $testName -- --ignored" + Arguments = $CargoArgsStr UseShellExecute = $false RedirectStandardOutput = $true RedirectStandardError = $true @@ -83,20 +132,41 @@ foreach ($testName in $TestNames) { $stderrTask = $proc.StandardError.ReadToEndAsync() $finished = $proc.WaitForExit($Timeout * 1000) + $killFailed = $false if (-not $finished) { - try { $proc.Kill($true) } catch {} + # Process.Kill($true) is .NET Core 3.0+ only, so it throws on Windows + # PowerShell 5.1. taskkill /T kills the whole tree on both editions, + # including the test binary that holds the radio (a child of cargo). + & { + $ErrorActionPreference = 'Continue' + & taskkill.exe /T /F /PID $proc.Id *> $null + } + $killFailed = -not $proc.WaitForExit(5000) + # A reader can outlive the kill if an orphaned grandchild still holds + # the pipe, so bound this wait too. + [void][System.Threading.Tasks.Task]::WaitAll(@($stdoutTask, $stderrTask), 5000) + } else { + # No-arg WaitForExit ensures the process and its readers are fully done. + $proc.WaitForExit() + [System.Threading.Tasks.Task]::WaitAll($stdoutTask, $stderrTask) } - # No-arg WaitForExit ensures the process and its readers are fully done. - $proc.WaitForExit() - [System.Threading.Tasks.Task]::WaitAll($stdoutTask, $stderrTask) + + # Only read a task's Result once it has actually completed -- the + # getter blocks until completion otherwise, which would undo the + # bounded waits above for a reader that never finished. + $stdoutText = if ($stdoutTask.IsCompleted) { $stdoutTask.Result } else { '' } + $stderrText = if ($stderrTask.IsCompleted) { $stderrTask.Result } else { '' } # Persist captured output for potential display below. - [System.IO.File]::WriteAllText("$LogFile.stdout", $stdoutTask.Result) - [System.IO.File]::WriteAllText($LogFile, $stderrTask.Result) + [System.IO.File]::WriteAllText("$LogFile.stdout", $stdoutText) + [System.IO.File]::WriteAllText($LogFile, $stderrText) $showOutput = $false if (-not $finished) { Write-Host "TIMEOUT (${Timeout}s)" + if ($killFailed) { + Write-Host " WARNING: process $($proc.Id) still alive after taskkill" -ForegroundColor Yellow + } $Failed++ $Failures += $testName $showOutput = $true @@ -105,6 +175,13 @@ foreach ($testName in $TestNames) { $Failed++ $Failures += $testName $showOutput = $true + } elseif ($testName -eq $WinrtbleRadioTests -and $stdoutText -match 'running 0 tests') { + # Even on a zero exit code, cargo reports "running 0 tests" if the + # module filter matched nothing -- don't let that pass silently. + Write-Host "FAIL (0 tests matched)" + $Failed++ + $Failures += $testName + $showOutput = $true } else { Write-Host "PASS" $Passed++ diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 0977f23b..8e07f9ea 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -59,5 +59,5 @@ Each test is its own file (and therefore its own binary), ensuring process isola - The scan timeout is 15 seconds (hardcoded in `peripheral_finder.rs`). - When adding a new test, also add the corresponding JNI export in `android/rust/src/lib.rs`, native declaration in `NativeTests.kt`, and `@Test` in `BleIntegrationTest.kt`. Tests backed by APIs Android does not support (e.g. `test_retrieve_connected_peripheral_by_service`, `test_retrieve_connected_peripheral_by_identifier`) and macOS-only tests intentionally have no JNI export. - Adapter-only tests require local adapter hardware but do not require the btleplug test peripheral; their desktop assertions are target-specific because CoreBluetooth and ordinary Android intentionally return `Ok(None)`. -- Each test must finish within `scripts/run-integration-tests.sh`'s 40 s per-test `TIMEOUT` (binaries are prebuilt outside it); on Windows the script also runs the ignored `winrtble::adapter::cleanup_tests` radio lib tests. +- Each test must finish within `scripts/run-integration-tests.sh`'s (or, on Windows, `scripts/run-integration-tests.ps1`'s) 40 s per-test `TIMEOUT` (binaries are prebuilt outside it); on Windows the script also runs the ignored `winrtble::adapter::cleanup_tests` radio lib tests. - The ignored CoreBluetooth lib tests in `src/corebluetooth/internal/tests.rs` (`#[ignore = "requires CoreBluetooth (creates a real CBCentralManager)"]`) are run manually on macOS with `cargo test --lib corebluetooth::internal::tests -- --ignored` (needs Bluetooth permission, no peripheral). From 7bf64ca6807329ed11c3e2c8066679e2a6f2146d Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 13:48:28 -0700 Subject: [PATCH 31/32] scripts: Share JDK detection between the Android build and test runs run-integration-tests-android.sh re-detected JAVA_HOME with macOS-only paths after build-java.sh, so on Linux the instrumentation step fell back to the system java; with JDK 25 there, Gradle 8.9 failed with only the version string as its error. Both scripts now source one find_java_home helper that prefers JDK 17 or 21 on macOS and Linux (including arm64 and JDK 21 package paths) instead of the system default. Also print the completion banner before exiting instead of after. Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/build-java.sh | 42 +++---------------- scripts/lib/java-home.sh | 53 ++++++++++++++++++++++++ scripts/run-integration-tests-android.sh | 20 ++++----- 3 files changed, 68 insertions(+), 47 deletions(-) create mode 100644 scripts/lib/java-home.sh diff --git a/scripts/build-java.sh b/scripts/build-java.sh index 37eded53..b023cb09 100755 --- a/scripts/build-java.sh +++ b/scripts/build-java.sh @@ -11,6 +11,9 @@ SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" JAVA_DIR="$PROJECT_ROOT/src/droidplug/java" +# shellcheck source=lib/java-home.sh +source "$SCRIPT_DIR/lib/java-home.sh" + # --- Colors (if terminal) --------------------------------------------------- if [ -t 1 ]; then RED='\033[0;31m'; GREEN='\033[0;32m'; YELLOW='\033[0;33m'; NC='\033[0m' @@ -88,42 +91,9 @@ ensure_java() { # --- JAVA_HOME --------------------------------------------------------------- ensure_java_home() { - if [ -n "${JAVA_HOME:-}" ] && [ -d "$JAVA_HOME" ]; then - return 0 - fi - - case "$OS" in - Darwin) - JAVA_HOME="$(/usr/libexec/java_home 2>/dev/null || true)" - # Fallback: check Homebrew openjdk directly - if [ -z "${JAVA_HOME:-}" ] || [ ! -d "${JAVA_HOME:-}" ]; then - for v in 17 21 11; do - local brew_jdk - brew_jdk="$(brew --prefix "openjdk@$v" 2>/dev/null || true)" - if [ -n "$brew_jdk" ] && [ -d "$brew_jdk/libexec/openjdk.jdk/Contents/Home" ]; then - JAVA_HOME="$brew_jdk/libexec/openjdk.jdk/Contents/Home" - break - fi - done - fi - ;; - Linux) - # Common locations - for candidate in \ - /usr/lib/jvm/java-17-openjdk-amd64 \ - /usr/lib/jvm/java-17-openjdk \ - /usr/lib/jvm/java-17 \ - /usr/lib/jvm/default-java; do - if [ -d "$candidate" ]; then - JAVA_HOME="$candidate" - break - fi - done - ;; - esac - - if [ -z "${JAVA_HOME:-}" ]; then - die "Could not determine JAVA_HOME. Set it manually and re-run." + JAVA_HOME="$(find_java_home)" + if [ -z "$JAVA_HOME" ]; then + die "Could not find a JDK 17 or 21. Set JAVA_HOME and re-run." fi export JAVA_HOME info "JAVA_HOME=$JAVA_HOME" diff --git a/scripts/lib/java-home.sh b/scripts/lib/java-home.sh new file mode 100644 index 00000000..5e660956 --- /dev/null +++ b/scripts/lib/java-home.sh @@ -0,0 +1,53 @@ +# shellcheck shell=bash +# +# Sourced by build-java.sh and run-integration-tests-android.sh so both Gradle +# invocations use the same JDK. +# +# Prints a JDK home for the Android Gradle build: $JAVA_HOME if it is set and +# exists, otherwise a JDK 17 or 21 from the platform's usual locations. Prints +# nothing if none is found. Gradle 8.9 cannot run on newer JDKs, so the system +# default java is not used. +find_java_home() { + if [ -n "${JAVA_HOME:-}" ] && [ -d "$JAVA_HOME" ]; then + echo "$JAVA_HOME" + return + fi + + local candidate + case "$(uname -s)" in + Darwin) + local v brew_jdk + for v in 17 21; do + candidate="$(/usr/libexec/java_home -v "$v" 2>/dev/null || true)" + if [ -n "$candidate" ] && [ -d "$candidate" ]; then + echo "$candidate" + return + fi + done + for v in 17 21 11; do + brew_jdk="$(brew --prefix "openjdk@$v" 2>/dev/null || true)" + if [ -n "$brew_jdk" ] && [ -d "$brew_jdk/libexec/openjdk.jdk/Contents/Home" ]; then + echo "$brew_jdk/libexec/openjdk.jdk/Contents/Home" + return + fi + done + ;; + Linux) + for candidate in \ + /usr/lib/jvm/java-17-openjdk-amd64 \ + /usr/lib/jvm/java-17-openjdk-arm64 \ + /usr/lib/jvm/java-17-openjdk \ + /usr/lib/jvm/java-17 \ + /usr/lib/jvm/java-21-openjdk-amd64 \ + /usr/lib/jvm/java-21-openjdk-arm64 \ + /usr/lib/jvm/java-21-openjdk \ + /usr/lib/jvm/java-21 \ + /usr/lib/jvm/default-java; do + if [ -d "$candidate" ]; then + echo "$candidate" + return + fi + done + ;; + esac +} diff --git a/scripts/run-integration-tests-android.sh b/scripts/run-integration-tests-android.sh index 42f828ff..e768a401 100755 --- a/scripts/run-integration-tests-android.sh +++ b/scripts/run-integration-tests-android.sh @@ -127,16 +127,15 @@ echo "sdk.dir=$(cd "$ANDROID_HOME" && pwd)" > "$ANDROID_TEST_DIR/local.propertie echo "" echo ">>> Step 4/4: Running instrumentation tests..." -# Ensure JAVA_HOME is set (build-java.sh sets it internally but it doesn't persist) -if [[ -z "${JAVA_HOME:-}" ]]; then - if [[ -d "/opt/homebrew/opt/openjdk@17" ]]; then - export JAVA_HOME="/opt/homebrew/opt/openjdk@17" - elif [[ -d "/usr/local/opt/openjdk@17" ]]; then - export JAVA_HOME="/usr/local/opt/openjdk@17" - elif command -v java &>/dev/null; then - export JAVA_HOME=$(/usr/libexec/java_home 2>/dev/null || true) - fi +# build-java.sh ran in a subprocess, so resolve the same JDK again here. +# shellcheck source=lib/java-home.sh +source "$SCRIPT_DIR/lib/java-home.sh" +JAVA_HOME="$(find_java_home)" +if [[ -z "$JAVA_HOME" ]]; then + echo "ERROR: Could not find a JDK 17 or 21. Set JAVA_HOME and re-run." + exit 1 fi +export JAVA_HOME echo " JAVA_HOME: ${JAVA_HOME:-}" # Ensure Gradle wrapper is available @@ -161,7 +160,6 @@ TEST_EXIT=$? # Restore default stay-on behavior adb shell svc power stayon false -exit $TEST_EXIT - echo "" echo "=== Android integration tests complete ===" +exit $TEST_EXIT From 621648004c56dfc39fad92d80d4f98006a7229ca Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Sun, 27 Sep 2026 13:53:01 -0700 Subject: [PATCH 32/32] fix(winrtble): Bound GATT lookups in service-filtered retrieval retrieve_peripherals enumerates every system-wide connected LE device and read each one's cached GATT services in turn. A stale "connected" device took 51 s to report Unreachable, so a service-filtered retrieval took over a minute. Look up devices concurrently and bound each cached service lookup to 5 s, cancelling the WinRT operation on timeout; a device that does not answer is treated as having no services, as an Unreachable status already was. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/winrtble/adapter.rs | 102 +++++++++++++++++++++++++++++----------- 1 file changed, 75 insertions(+), 27 deletions(-) diff --git a/src/winrtble/adapter.rs b/src/winrtble/adapter.rs index 36d8a2a4..a7348f8d 100644 --- a/src/winrtble/adapter.rs +++ b/src/winrtble/adapter.rs @@ -20,12 +20,15 @@ use crate::{ common::adapter_manager::AdapterManager, }; use async_trait::async_trait; +use futures::future::join_all; use futures::stream::Stream; use std::convert::TryFrom; use std::fmt::{self, Debug, Formatter}; use std::future::IntoFuture; use std::pin::Pin; use std::sync::{Arc, Mutex}; +use std::time::Duration; +use tokio::time::timeout; use windows::{ Devices::{ Bluetooth::{ @@ -61,6 +64,18 @@ impl Drop for RadioStateHandler { } } +/// Timeout for a single device's Cached-mode GATT service lookup during +/// [`Adapter::retrieve_peripherals`]'s service/combined path. +/// +/// WinRT's system-wide "connected" device selector can keep reporting a device as +/// connected long after it has gone out of range; `GetGattServicesWithCacheModeAsync` +/// on such a stale device has been observed to hang for tens of seconds (~51s on real +/// hardware) instead of returning `Unreachable` promptly. A device that does not +/// answer within this window is treated the same as one that returned a non-`Success` +/// status: no services, so it cannot match a service filter. Kept separate from +/// `ble::device::GATT_CACHE_TIMEOUT` (#325), which bounds a different, uncached call. +const RETRIEVE_GATT_TIMEOUT: Duration = Duration::from_secs(5); + fn winrt_error(error: E) -> Error { Error::Other(format!("{error:?}").into()) } @@ -216,6 +231,9 @@ impl Central for Adapter { /// WinRT's connected-device selector is system-wide and cannot be restricted to /// this `Radio`; callers must treat results as belonging to the Windows BLE /// subsystem rather than to one physical adapter when multiple radios exist. + /// For service and combined lookups, a device whose GATT services cannot be read + /// within 5 seconds is treated as having no services, so it cannot match the + /// service filter (it can still match by identifier in a combined lookup). async fn retrieve_peripherals( &self, options: RetrievePeripheralsOptions, @@ -269,9 +287,12 @@ impl Central for Adapter { .map_err(winrt_error)? .into_iter() .collect::>(); - let mut result = Vec::new(); - for info in devices { + // Look up devices concurrently; each lookup is bounded by RETRIEVE_GATT_TIMEOUT, + // so stale devices cost about one timeout in total. + let manager = &self.manager; + let options = &options; + let device_futures = devices.into_iter().map(|info| async move { let id = info.Id().map_err(winrt_error)?; let device = BluetoothLEDevice::FromIdAsync(&id) .map_err(winrt_error)? @@ -281,37 +302,64 @@ impl Central for Adapter { let address = checked_address(device.BluetoothAddress().map_err(winrt_error)?)?; let candidate_id = PeripheralId::from(address); - let service_result = device + let service_op = device .GetGattServicesWithCacheModeAsync(BluetoothCacheMode::Cached) - .map_err(winrt_error)? - .into_future() - .await .map_err(winrt_error)?; - let service_uuids = if service_result.Status().map_err(winrt_error)? - == GattCommunicationStatus::Success + let service_uuids = match timeout( + RETRIEVE_GATT_TIMEOUT, + service_op.clone().into_future(), + ) + .await { - service_result - .Services() - .map_err(winrt_error)? - .into_iter() - .map(|service| { - service - .Uuid() - .map(|uuid| crate::winrtble::utils::to_uuid(&uuid)) - }) - .collect::>>() - .map_err(winrt_error)? - } else { - Vec::new() + Ok(service_result) => { + let service_result = service_result.map_err(winrt_error)?; + if service_result.Status().map_err(winrt_error)? + == GattCommunicationStatus::Success + { + service_result + .Services() + .map_err(winrt_error)? + .into_iter() + .map(|service| { + service + .Uuid() + .map(|uuid| crate::winrtble::utils::to_uuid(&uuid)) + }) + .collect::>>() + .map_err(winrt_error)? + } else { + Vec::new() + } + } + Err(_) => { + log::debug!( + "GATT service lookup for {candidate_id:?} did not complete within \ + {RETRIEVE_GATT_TIMEOUT:?}; treating it as having no services" + ); + if let Err(cancel_err) = service_op.Cancel() { + log::warn!( + "Failed to cancel timed-out GATT service lookup for {candidate_id:?}: \ + {cancel_err:?}" + ); + } + Vec::new() + } }; - if !api::matches_retrieval_selectors(&candidate_id, &service_uuids, &options) { - continue; + if !api::matches_retrieval_selectors(&candidate_id, &service_uuids, options) { + return Ok(None); } - let peripheral = self.manager.peripheral(&candidate_id).unwrap_or_else(|| { - let peripheral = Peripheral::new(Arc::downgrade(&self.manager), address); - self.manager.add_peripheral(peripheral) + let peripheral = manager.peripheral(&candidate_id).unwrap_or_else(|| { + let peripheral = Peripheral::new(Arc::downgrade(manager), address); + manager.add_peripheral(peripheral) }); - result.push(peripheral); + Ok::<_, Error>(Some(peripheral)) + }); + + let mut result = Vec::new(); + for outcome in join_all(device_futures).await { + if let Some(peripheral) = outcome? { + result.push(peripheral); + } } Ok(api::merge_retrieved_peripherals(result, |peripheral| { crate::api::Peripheral::id(peripheral)