From 228a26db3dccacc043fc6936e251a860c42f9537 Mon Sep 17 00:00:00 2001 From: FlopBut Date: Sun, 28 Jun 2026 23:37:28 +0200 Subject: [PATCH 1/7] feat(triki-controller): pluggable transport + Node NobleTransport Extract a small TrikiTransport interface so TrikiController is no longer hardwired to Web Bluetooth. The browser path stays the default (WebBluetoothTransport); a NobleTransport (Node, @abandonware/noble, optional + lazily imported) lets the token be received outside the browser via the new `triki-controller/node` entry point. Adds a Node example and docs. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/guide/library.md | 62 +++- packages/triki-controller/example/node.mjs | 31 ++ packages/triki-controller/package.json | 16 +- packages/triki-controller/src/controller.ts | 196 ++++--------- packages/triki-controller/src/events.ts | 7 + packages/triki-controller/src/index.ts | 2 + packages/triki-controller/src/node.ts | 11 + packages/triki-controller/src/transport.ts | 60 ++++ .../triki-controller/src/transports/noble.ts | 266 ++++++++++++++++++ .../src/transports/web-bluetooth.ts | 201 +++++++++++++ packages/triki-controller/tsup.config.ts | 4 +- 11 files changed, 704 insertions(+), 152 deletions(-) create mode 100644 packages/triki-controller/example/node.mjs create mode 100644 packages/triki-controller/src/node.ts create mode 100644 packages/triki-controller/src/transport.ts create mode 100644 packages/triki-controller/src/transports/noble.ts create mode 100644 packages/triki-controller/src/transports/web-bluetooth.ts diff --git a/docs/guide/library.md b/docs/guide/library.md index e7a8d6e..11d2b79 100644 --- a/docs/guide/library.md +++ b/docs/guide/library.md @@ -11,7 +11,9 @@ controller. - **npm:** [`triki-controller`](https://www.npmjs.com/package/triki-controller) - **Source + README:** [`packages/triki-controller`](https://github.com/Flopsstuff/triki/tree/main/packages/triki-controller#readme) -- Zero runtime dependencies, ESM-only, ships its own `.d.ts`. +- ESM-only, ships its own `.d.ts`. The browser core has **zero runtime dependencies**; + the optional Node transport uses `@abandonware/noble` (see + [Node.js](#nodejs-receive-outside-the-browser)). The visualizer page and this package share the same parsing and fusion math; the [BLE protocol](./ble-protocol) and [IMU streaming](./imu-streaming) pages are the @@ -120,6 +122,7 @@ interface OrientationEvent { | `gyroScale` | `14.286` | gyro scale in LSB per deg/s (LSM6DSL ±2000 dps) | | `gyroBias` | `{0,0,0}` | per-axis gyro correction (deg/s) subtracted from every sample | | `accelBias` | `{0,0,0}` | per-axis accel correction (g) subtracted from every sample | +| `transport` | Web Bluetooth | BLE transport; pass a `NobleTransport` (Node) or any `TrikiTransport` | | Member | Description | |---|---| @@ -141,6 +144,63 @@ interface OrientationEvent { `try/catch` to surface picker errors. `reconnect()` reuses the cached device and throws if `connect()` was never called. +## Node.js (receive outside the browser) + +Web Bluetooth only exists in the browser. To receive the token from a plain Node +process, pass a **`NobleTransport`** — a transport backed by +[`@abandonware/noble`](https://github.com/abandonware/noble) — from the +`triki-controller/node` entry point. Everything else (events, fusion, `setLed`, +`setRate`, `resetHeading`) behaves exactly as in the browser. + +```sh +npm install triki-controller @abandonware/noble +``` + +```ts +import { TrikiController, NobleTransport } from "triki-controller/node"; + +const triki = new TrikiController({ transport: new NobleTransport() }); + +triki.on("orientation", (o) => console.log(o.euler)); // { roll, pitch, yaw } +await triki.connect(); // scans for a "TRIKI" token, connects, starts streaming +``` + +A runnable demo lives at +[`packages/triki-controller/example/node.mjs`](https://github.com/Flopsstuff/triki/blob/main/packages/triki-controller/example/node.mjs). + +`@abandonware/noble` is an **optional peer dependency**: install it alongside +`triki-controller` (as above). It is imported lazily, only when `connect()` runs, so +`triki-controller` never pulls the native module on its own. On macOS the first run +prompts the terminal (or your IDE) for Bluetooth access. + +**`new NobleTransport(options?)`:** + +| Option | Default | Meaning | +|---|---|---| +| `namePrefix` | `"TRIKI"` | match devices whose advertised name starts with this prefix | +| `address` | — | match a specific BLE address instead of by name | +| `noble` | `@abandonware/noble` | a noble-API-compatible module to use instead | + +::: warning Native build +`@abandonware/noble` compiles a native addon, and its bundled `node-gyp` is old enough +to fail on the newest Node releases. Use a current **Node LTS** (which has prebuilt +binaries), or pass a drop-in fork through the `noble` option: + +```ts +import noble from "@stoprocent/noble"; +const triki = new TrikiController({ transport: new NobleTransport({ noble }) }); +``` +::: + +### Custom transports + +`NobleTransport` and the browser `WebBluetoothTransport` both implement the small +**`TrikiTransport`** interface (`connect`, `writeRx`, `writeCtrl`, `onFrame`, +`onDisconnect`, `disconnect`, `hasLed`). Implement it yourself to drive the controller +over any link — a WebSocket bridge, a serial dongle, or a replay of recorded frames — +while reusing all of the parsing and fusion. The transport is a dumb NUS pipe; the +controller owns the Triki protocol, so the protocol logic lives in exactly one place. + ## Standalone primitives The parsing, fusion and protocol code is pure and **environment-agnostic** (no DOM, no diff --git a/packages/triki-controller/example/node.mjs b/packages/triki-controller/example/node.mjs new file mode 100644 index 0000000..7790584 --- /dev/null +++ b/packages/triki-controller/example/node.mjs @@ -0,0 +1,31 @@ +/** + * Receive the Triki IMU stream in Node — no browser required. + * + * Prerequisites (from the package directory): + * npm install @abandonware/noble # optional native dependency + * npm run build # produces dist/node.js + * + * Then run: + * node example/node.mjs + * + * macOS grants Bluetooth access per-app, so the first run prompts the terminal + * (or your IDE) for permission. + */ +import { TrikiController, NobleTransport } from "../dist/node.js"; + +const triki = new TrikiController({ transport: new NobleTransport() }); + +triki.on("connectionchange", (state) => console.log("[state]", state)); +triki.on("rate", (hz) => console.log("[rate]", hz, "Hz")); +triki.on("orientation", ({ euler }) => { + const f = (n) => n.toFixed(1).padStart(7); + console.log(`roll ${f(euler.roll)} pitch ${f(euler.pitch)} yaw ${f(euler.yaw)}`); +}); + +process.on("SIGINT", () => { + triki.disconnect(); + process.exit(0); +}); + +console.log("Scanning for a TRIKI token… (Ctrl+C to quit)"); +await triki.connect(); diff --git a/packages/triki-controller/package.json b/packages/triki-controller/package.json index 583102b..c92e1d9 100644 --- a/packages/triki-controller/package.json +++ b/packages/triki-controller/package.json @@ -1,7 +1,7 @@ { "name": "triki-controller", "version": "0.2.0", - "description": "Web Bluetooth driver for the Żabka Triki token (nRF52810 + LSM6DSL) — connect, stream the IMU, and read fused orientation (selectable Madgwick or VQF) in the browser to reuse the token as a motion controller.", + "description": "Driver for the Żabka Triki token (nRF52810 + LSM6DSL) — connect, stream the IMU, and read fused orientation (selectable Madgwick or VQF) to reuse the token as a motion controller. Web Bluetooth in the browser, or @abandonware/noble in Node.", "type": "module", "license": "MIT", "author": "Flopsstuff", @@ -26,6 +26,8 @@ "gyroscope", "accelerometer", "motion-controller", + "noble", + "nodejs", "triki", "zabka" ], @@ -37,6 +39,10 @@ ".": { "types": "./dist/index.d.ts", "import": "./dist/index.js" + }, + "./node": { + "types": "./dist/node.d.ts", + "import": "./dist/node.js" } }, "files": [ @@ -61,6 +67,14 @@ "typescript": "^5.7.2", "vitest": "^4.1.9" }, + "peerDependencies": { + "@abandonware/noble": "^1.9.2-26" + }, + "peerDependenciesMeta": { + "@abandonware/noble": { + "optional": true + } + }, "publishConfig": { "access": "public" } diff --git a/packages/triki-controller/src/controller.ts b/packages/triki-controller/src/controller.ts index bf77c6e..e572c46 100644 --- a/packages/triki-controller/src/controller.ts +++ b/packages/triki-controller/src/controller.ts @@ -1,8 +1,10 @@ /** - * Web Bluetooth client for the Żabka Triki BLE token. Connects over the Nordic - * UART Service, starts the IMU stream, parses motion frames, and (optionally) fuses - * orientation with a 6-axis Madgwick filter. This is the only module that touches - * `navigator.bluetooth` and `performance`. + * Client for the Żabka Triki BLE token. Owns the IMU protocol (START/LED commands), + * parses motion frames, and (optionally) fuses orientation with a selectable 6-axis + * filter (Madgwick / VQF / accel-only). The actual BLE link is delegated to a + * {@link TrikiTransport} — by default a `WebBluetoothTransport` (browser), or a + * `NobleTransport` (Node) — so this module is transport-agnostic and never touches + * `navigator.bluetooth` directly. */ import { TypedEmitter } from "./emitter"; import { FrameParser } from "./parser"; @@ -11,12 +13,6 @@ import { MadgwickAHRS, AccelAHRS, quatMul, quatAboutZ, yawRadOf, eulerOf } from import type { OrientationFilter, Quaternion } from "./fusion"; import { VqfAHRS, DEFAULT_TAU_ACC } from "./vqf"; import { - NUS_SERVICE, - NUS_RX, - NUS_TX, - NUS_CTRL, - BATTERY_SERVICE, - BATTERY_LEVEL, DEFAULT_ACCEL_SCALE, DEFAULT_GYRO_SCALE, DEFAULT_RATE_HZ, @@ -24,6 +20,8 @@ import { startCmd, ledCmd, } from "./protocol"; +import { WebBluetoothTransport } from "./transports/web-bluetooth"; +import type { TrikiTransport } from "./transport"; import type { ConnectionState, FusionAlgorithm, @@ -59,12 +57,7 @@ const RATE_WINDOW_MS = 1000; const MAX_DT_S = 0.2; export class TrikiController extends TypedEmitter { - #device: BluetoothDevice | null = null; - #gatt: BluetoothRemoteGATTServer | null = null; - #rxChar: BluetoothRemoteGATTCharacteristic | null = null; - #txChar: BluetoothRemoteGATTCharacteristic | null = null; - #ctrlChar: BluetoothRemoteGATTCharacteristic | null = null; - #batteryChar: BluetoothRemoteGATTCharacteristic | null = null; + #transport: TrikiTransport; #battery: number | null = null; #parser = new FrameParser(); @@ -92,6 +85,10 @@ export class TrikiController extends TypedEmitter { this.#beta = options.beta ?? DEFAULT_BETA; this.#tauAcc = options.tauAcc ?? DEFAULT_TAU_ACC; this.#filter = this.#makeFilter(this.#algo); + this.#transport = options.transport ?? new WebBluetoothTransport(); + this.#transport.onFrame(this.#ingest); + this.#transport.onDisconnect(this.#handleTransportDisconnect); + this.#transport.onBattery(this.#handleBattery); } #makeFilter(algo: FusionAlgorithm): OrientationFilter | null { @@ -101,9 +98,9 @@ export class TrikiController extends TypedEmitter { return null; } - /** True when Web Bluetooth is available (safe to call during SSR). */ + /** True when the default Web Bluetooth transport is available (safe during SSR). */ static isSupported(): boolean { - return typeof navigator !== "undefined" && !!navigator.bluetooth; + return WebBluetoothTransport.isSupported(); } /** True while streaming. */ @@ -123,7 +120,7 @@ export class TrikiController extends TypedEmitter { /** True when the token exposes the LED control characteristic. */ get hasLed(): boolean { - return this.#ctrlChar !== null; + return this.#transport.hasLed; } /** Last known battery level in percent, or `null` if not read yet. */ @@ -142,22 +139,17 @@ export class TrikiController extends TypedEmitter { } /** - * Show the browser device picker, connect, and start the IMU stream. - * Must be called from a user gesture (e.g. a click handler). Rejects (after - * cleaning up) if pairing or the handshake fails. + * Connect through the transport and start the IMU stream. With the default Web + * Bluetooth transport this shows the device picker and must be called from a user + * gesture (e.g. a click handler). Rejects (after cleaning up) if the transport or + * the handshake fails. */ async connect(): Promise { - if (!TrikiController.isSupported()) { - throw new Error("Web Bluetooth is not available in this environment."); - } if (this.#state !== "disconnected") return; try { this.#setState("pairing"); - this.#device = await navigator.bluetooth.requestDevice({ - filters: [{ namePrefix: "TRIKI" }, { namePrefix: "Triki" }], - optionalServices: [NUS_SERVICE, BATTERY_SERVICE], - }); - await this.#startSession(); + await this.#transport.connect(); + await this.#startStreaming(); } catch (err) { this.#cleanup(); throw err; @@ -165,15 +157,18 @@ export class TrikiController extends TypedEmitter { } /** - * Reconnect to the previously paired device without showing the picker. - * Throws if {@link connect} was never called. + * Reconnect to the previously selected device without re-prompting. Throws if the + * transport does not support reconnecting (e.g. {@link connect} was never called). */ async reconnect(): Promise { - if (!this.#device) throw new Error("No device to reconnect to; call connect() first."); if (this.#state !== "disconnected") return; + if (!this.#transport.reconnect) { + throw new Error("This transport does not support reconnect()."); + } try { this.#setState("pairing"); - await this.#startSession(); + await this.#transport.reconnect(); + await this.#startStreaming(); } catch (err) { this.#cleanup(); throw err; @@ -182,14 +177,14 @@ export class TrikiController extends TypedEmitter { /** Disconnect from the token. Triggers a `connectionchange` to `disconnected`. */ disconnect(): void { - if (this.#gatt && this.#gatt.connected) this.#gatt.disconnect(); - else this.#onDisconnected(); + if (this.#state === "disconnected") return; + this.#transport.disconnect(); } /** Turn the green LED on or off. Throws when the LED characteristic is unavailable. */ async setLed(on: boolean): Promise { - if (!this.#ctrlChar) throw new Error("LED control characteristic is not available."); - await this.#ctrlChar.writeValue(ledCmd(on)); + if (!this.#transport.hasLed) throw new Error("LED control characteristic is not available."); + await this.#transport.writeCtrl(ledCmd(on)); } /** @@ -200,7 +195,7 @@ export class TrikiController extends TypedEmitter { this.#rateHz = hz; // Keep VQF's accel low-pass aligned with the new sample period. if (this.#filter instanceof VqfAHRS) this.#filter.setSamplePeriod(1 / hz); - if (this.#rxChar) await this.#write(this.#rxChar, startCmd(hz), true); + if (this.#state === "streaming") await this.#transport.writeRx(startCmd(hz), true); } /** Re-zero the heading (yaw). No-op when fusion is disabled. Tilt stays absolute. */ @@ -252,85 +247,27 @@ export class TrikiController extends TypedEmitter { // --- internal ---------------------------------------------------------------- - async #startSession(): Promise { - const device = this.#device; - if (!device || !device.gatt) throw new Error("Device GATT is not available."); - // Same listener reference, so re-adding on reconnect is a no-op. - device.addEventListener("gattserverdisconnected", this.#onDisconnected); - - this.#gatt = await device.gatt.connect(); - const svc = await this.#gatt.getPrimaryService(NUS_SERVICE); - this.#rxChar = await svc.getCharacteristic(NUS_RX); - this.#txChar = await svc.getCharacteristic(NUS_TX); - try { - this.#ctrlChar = await svc.getCharacteristic(NUS_CTRL); - } catch { - this.#ctrlChar = null; - } - - await this.#txChar.startNotifications(); - this.#txChar.addEventListener("characteristicvaluechanged", this.#onNotify); - - await this.#write(this.#rxChar, startCmd(this.#rateHz), true); - + /** Start the stream once the transport is connected: send START, begin timers. */ + async #startStreaming(): Promise { + await this.#transport.writeRx(startCmd(this.#rateHz), true); this.#startRateTimer(); - void this.#startBattery(this.#gatt); // async, best-effort; never blocks streaming this.#setState("streaming"); } - /** - * Read the battery level once and subscribe to updates, if the service exists. - * Fire-and-forget: `gatt` pins the session, and we bail after each await once it is - * no longer the active connection so a stale read/listener can't outlive cleanup. - */ - async #startBattery(gatt: BluetoothRemoteGATTServer | null): Promise { - if (!gatt) return; - try { - const svc = await gatt.getPrimaryService(BATTERY_SERVICE); - if (this.#gatt !== gatt) return; - const char = await svc.getCharacteristic(BATTERY_LEVEL); - if (this.#gatt !== gatt) return; - const value = await char.readValue(); - if (this.#gatt !== gatt) return; - this.#emitBattery(value); - this.#batteryChar = char; - try { - await char.startNotifications(); - if (this.#gatt !== gatt) { - this.#batteryChar = null; // disconnected mid-subscribe; don't leak a listener - return; - } - char.addEventListener("characteristicvaluechanged", this.#onBattery); - } catch { - this.#batteryChar = null; // notifications unsupported; the one-time read stands - } - } catch { - this.#batteryChar = null; // no Battery service on this token - } - } - - #onBattery = (event: Event): void => { - const char = event.target as BluetoothRemoteGATTCharacteristic; - if (char.value) this.#emitBattery(char.value); + /** Transport callback: decode inbound notification bytes into frames. */ + #ingest = (bytes: Uint8Array): void => { + for (const frame of this.#parser.push(bytes)) this.#handleFrame(frame); }; - #emitBattery(view: DataView): void { - const pct = view.getUint8(0); - this.#battery = pct; - this.emit("battery", pct); - } - - #onNotify = (event: Event): void => { - const char = event.target as BluetoothRemoteGATTCharacteristic; - const view = char.value; - if (!view) return; - // byteOffset/byteLength are load-bearing: the underlying buffer may be larger. - const bytes = new Uint8Array(view.buffer, view.byteOffset, view.byteLength); - for (const frame of this.#parser.push(bytes)) this.#handleFrame(frame); + /** Transport callback: the link dropped (lost or via {@link disconnect}). */ + #handleTransportDisconnect = (): void => { + if (this.#state !== "disconnected") this.#cleanup(); }; - #onDisconnected = (): void => { - this.#cleanup(); + /** Transport callback: a battery-level reading (percent). */ + #handleBattery = (percent: number): void => { + this.#battery = percent; + this.emit("battery", percent); }; #handleFrame(raw: RawFrame): void { @@ -392,50 +329,11 @@ export class TrikiController extends TypedEmitter { } #cleanup(): void { - if (this.#txChar) { - try { - this.#txChar.removeEventListener("characteristicvaluechanged", this.#onNotify); - } catch { - /* ignore */ - } - } - if (this.#batteryChar) { - try { - this.#batteryChar.removeEventListener("characteristicvaluechanged", this.#onBattery); - } catch { - /* ignore */ - } - } - if (this.#device) { - try { - this.#device.removeEventListener("gattserverdisconnected", this.#onDisconnected); - } catch { - /* ignore */ - } - } this.#stopRateTimer(); - this.#rxChar = null; - this.#txChar = null; - this.#ctrlChar = null; - this.#batteryChar = null; this.#battery = null; - this.#gatt = null; - // #device is retained so reconnect() can reuse it without the picker. this.#parser.reset(); this.#frameTimes = []; this.#lastFrameTs = 0; this.#setState("disconnected"); } - - async #write( - char: BluetoothRemoteGATTCharacteristic, - data: Uint8Array, - withoutResponse: boolean, - ): Promise { - if (withoutResponse && typeof char.writeValueWithoutResponse === "function") { - await char.writeValueWithoutResponse(data); - } else { - await char.writeValue(data); - } - } } diff --git a/packages/triki-controller/src/events.ts b/packages/triki-controller/src/events.ts index de74a11..cdb161d 100644 --- a/packages/triki-controller/src/events.ts +++ b/packages/triki-controller/src/events.ts @@ -1,5 +1,6 @@ /** Public event payloads, the controller event map, and constructor options. */ import type { Quaternion, EulerAngles } from "./fusion"; +import type { TrikiTransport } from "./transport"; /** Connection lifecycle state. */ export type ConnectionState = "disconnected" | "pairing" | "streaming"; @@ -76,4 +77,10 @@ export interface TrikiControllerOptions { gyroBias?: Vec3; /** Per-axis accel correction (g) subtracted from every sample. Default zeros. */ accelBias?: Vec3; + /** + * BLE transport. Defaults to a `WebBluetoothTransport` (browser). Pass a + * `NobleTransport` (from `triki-controller/node`) to receive outside the browser, + * or any object implementing {@link TrikiTransport}. + */ + transport?: TrikiTransport; } diff --git a/packages/triki-controller/src/index.ts b/packages/triki-controller/src/index.ts index b47e4b2..6376674 100644 --- a/packages/triki-controller/src/index.ts +++ b/packages/triki-controller/src/index.ts @@ -1,6 +1,7 @@ /** Public API for triki-controller. */ export { TrikiController } from "./controller"; +export { WebBluetoothTransport } from "./transports/web-bluetooth"; export { MadgwickAHRS, AccelAHRS, quatMul, quatAboutZ, yawRadOf, eulerOf } from "./fusion"; export { VqfAHRS, DEFAULT_TAU_ACC } from "./vqf"; export { FrameParser, decodeCounts } from "./parser"; @@ -24,6 +25,7 @@ export { SUPPORTED_RATES_HZ, } from "./protocol"; +export type { TrikiTransport } from "./transport"; export type { Quaternion, EulerAngles, MadgwickOptions, OrientationFilter } from "./fusion"; export type { VqfOptions } from "./vqf"; export type { RawFrame } from "./parser"; diff --git a/packages/triki-controller/src/node.ts b/packages/triki-controller/src/node.ts new file mode 100644 index 0000000..5b541a5 --- /dev/null +++ b/packages/triki-controller/src/node.ts @@ -0,0 +1,11 @@ +/** + * Node entry point for triki-controller (`triki-controller/node`). + * + * Re-exports the full browser API and adds {@link NobleTransport}, a BLE transport + * backed by the optional `@abandonware/noble` dependency, so the token can be received + * outside the browser. Keeping it on a separate entry means the default browser bundle + * never imports noble. + */ +export * from "./index"; +export { NobleTransport } from "./transports/noble"; +export type { NobleTransportOptions } from "./transports/noble"; diff --git a/packages/triki-controller/src/transport.ts b/packages/triki-controller/src/transport.ts new file mode 100644 index 0000000..eae30a4 --- /dev/null +++ b/packages/triki-controller/src/transport.ts @@ -0,0 +1,60 @@ +/** + * Transport abstraction for the Żabka Triki token. + * + * A transport is a dumb Nordic UART Service (NUS) pipe: it connects to the device, + * writes command bytes to RX, exposes the optional control (LED) characteristic, and + * streams TX notification bytes back. It knows nothing about the Triki protocol — the + * {@link TrikiController} builds the START/LED commands and parses the motion frames — + * so protocol logic lives in exactly one place and new transports stay tiny. + * + * `TrikiController` defaults to a `WebBluetoothTransport` (browser). A `NobleTransport` + * (Node, via `@abandonware/noble`) is available from the `triki-controller/node` entry. + */ +export interface TrikiTransport { + /** + * Find/select the device, connect, discover the NUS characteristics, and start TX + * notifications. Resolves once frames can flow; rejects (after cleaning up) on failure. + */ + connect(): Promise; + + /** + * Reconnect to the previously selected device without re-prompting. Optional — + * transports that cannot remember a device may omit it, in which case + * {@link TrikiController.reconnect} throws a clear error. + */ + reconnect?(): Promise; + + /** Write a command to the NUS RX characteristic (host -> token). */ + writeRx(data: Uint8Array, withoutResponse?: boolean): Promise; + + /** Whether the control (LED) characteristic is present on the connected device. */ + readonly hasLed: boolean; + + /** Write to the control (LED) characteristic. Rejects when {@link hasLed} is false. */ + writeCtrl(data: Uint8Array): Promise; + + /** + * Register the handler for inbound TX notification chunks (raw bytes). Called once by + * the controller; the transport invokes it for every notification while connected. + */ + onFrame(handler: (bytes: Uint8Array) => void): void; + + /** + * Register the handler for a disconnect — whether a lost link or the result of + * {@link disconnect}. Called once by the controller. + */ + onDisconnect(handler: () => void): void; + + /** + * Register the handler for battery-level readings (percent, 0–100). Called once by + * the controller. Transports that can read the standard Battery service invoke it on + * connect and on each update; others may never call it. + */ + onBattery(handler: (percent: number) => void): void; + + /** + * Tear down the connection. Guarantees the {@link onDisconnect} handler fires once + * when a connection was active; a safe no-op otherwise. + */ + disconnect(): void; +} diff --git a/packages/triki-controller/src/transports/noble.ts b/packages/triki-controller/src/transports/noble.ts new file mode 100644 index 0000000..79a988c --- /dev/null +++ b/packages/triki-controller/src/transports/noble.ts @@ -0,0 +1,266 @@ +/** + * Node implementation of {@link TrikiTransport}, backed by `@abandonware/noble`. + * + * This is a working skeleton: it scans for a TRIKI token by advertised name (or a + * fixed address), connects, discovers the NUS characteristics, and streams TX + * notifications back to the {@link TrikiController}. It is intentionally minimal — see + * the `TODO`s for scan timeouts and richer error handling before production use. + * + * `@abandonware/noble` is an **optional peer dependency**: it is imported lazily and + * only required when {@link NobleTransport.connect} runs, so importing + * `triki-controller/node` never forces the native module to be installed. Install it to + * use this transport: + * + * ```sh + * npm install @abandonware/noble + * ``` + */ +import { NUS_SERVICE, NUS_RX, NUS_TX, NUS_CTRL } from "../protocol"; +import type { TrikiTransport } from "../transport"; + +// Node's Buffer, declared locally so this file needs no `@types/node` (noble's writes +// want a Buffer; it is a Uint8Array subclass at runtime). +declare const Buffer: { + from(buffer: ArrayBufferLike, byteOffset?: number, length?: number): Uint8Array; +}; + +/** Options for {@link NobleTransport}. */ +export interface NobleTransportOptions { + /** + * Match devices whose advertised name starts with this prefix. Default `"TRIKI"` + * (the token advertises as `TRIKI `). Ignored when {@link address} is set. + */ + namePrefix?: string; + /** Match a specific device by BLE address (case-insensitive) instead of by name. */ + address?: string; + /** + * A noble-API-compatible module to use instead of lazily importing + * `@abandonware/noble`. Pass e.g. `import noble from "@stoprocent/noble"` when the + * default fork won't build on your Node version, or a mock for tests. + */ + noble?: unknown; +} + +// --- minimal slice of the @abandonware/noble surface we use ---------------------- +// Declared locally so `typecheck`/`build` need no noble install (it is lazily imported +// and externalized at build time). Buffers are Uint8Arrays, so we type them as such. + +interface NobleCharacteristic { + readonly uuid: string; + writeAsync(data: Uint8Array, withoutResponse: boolean): Promise; + readAsync(): Promise; + subscribeAsync(): Promise; + on(event: "data", listener: (data: Uint8Array, isNotification: boolean) => void): this; +} + +interface NoblePeripheral { + readonly address: string; + readonly advertisement: { localName?: string }; + connectAsync(): Promise; + disconnectAsync(): Promise; + discoverSomeServicesAndCharacteristicsAsync( + serviceUuids: string[], + characteristicUuids: string[], + ): Promise<{ characteristics: NobleCharacteristic[] }>; + once(event: "disconnect", listener: () => void): this; + removeAllListeners(event?: string): this; +} + +interface Noble { + readonly state: string; + on(event: "stateChange", listener: (state: string) => void): this; + on(event: "discover", listener: (peripheral: NoblePeripheral) => void): this; + removeListener(event: string, listener: (...args: never[]) => void): this; + startScanningAsync(serviceUuids?: string[], allowDuplicates?: boolean): Promise; + stopScanningAsync(): Promise; +} + +/** noble matches UUIDs as lowercase hex without dashes. */ +const nobleUuid = (uuid: string): string => uuid.replace(/-/g, "").toLowerCase(); +const NUS_SERVICE_ID = nobleUuid(NUS_SERVICE); +const NUS_RX_ID = nobleUuid(NUS_RX); +const NUS_TX_ID = nobleUuid(NUS_TX); +const NUS_CTRL_ID = nobleUuid(NUS_CTRL); +/** Standard Battery Service (0x180F) and Battery Level characteristic (0x2A19). */ +const BATTERY_SERVICE_ID = "180f"; +const BATTERY_LEVEL_ID = "2a19"; + +async function loadNoble(): Promise { + try { + // Non-literal specifier: keeps `tsc` from resolving the optional module at + // typecheck time and keeps the bundler from inlining it (also marked external). + const specifier: string = "@abandonware/noble"; + const mod: { default?: Noble } & Noble = await import(specifier); + return mod.default ?? mod; + } catch (err) { + // ES2020 lib has no Error `cause` option; attach it manually for debuggability. + const error = new Error( + "NobleTransport requires the optional peer dependency '@abandonware/noble'. " + + "Install it with: npm install @abandonware/noble", + ); + (error as { cause?: unknown }).cause = err; + throw error; + } +} + +export class NobleTransport implements TrikiTransport { + #namePrefix: string; + #address: string | null; + #injectedNoble: Noble | null; + #peripheral: NoblePeripheral | null = null; + #rxChar: NobleCharacteristic | null = null; + #txChar: NobleCharacteristic | null = null; + #ctrlChar: NobleCharacteristic | null = null; + #frameHandler: ((bytes: Uint8Array) => void) | null = null; + #disconnectHandler: (() => void) | null = null; + #batteryHandler: ((percent: number) => void) | null = null; + + constructor(options: NobleTransportOptions = {}) { + this.#namePrefix = options.namePrefix ?? "TRIKI"; + this.#address = options.address ? options.address.toLowerCase() : null; + this.#injectedNoble = (options.noble as Noble | undefined) ?? null; + } + + get hasLed(): boolean { + return this.#ctrlChar !== null; + } + + onFrame(handler: (bytes: Uint8Array) => void): void { + this.#frameHandler = handler; + } + + onDisconnect(handler: () => void): void { + this.#disconnectHandler = handler; + } + + onBattery(handler: (percent: number) => void): void { + this.#batteryHandler = handler; + } + + async connect(): Promise { + const noble = this.#injectedNoble ?? (await loadNoble()); + try { + await this.#waitPoweredOn(noble); + const peripheral = await this.#scan(noble); + this.#peripheral = peripheral; + peripheral.once("disconnect", this.#onDisconnected); + await peripheral.connectAsync(); + + const { characteristics } = await peripheral.discoverSomeServicesAndCharacteristicsAsync( + [NUS_SERVICE_ID], + [NUS_RX_ID, NUS_TX_ID, NUS_CTRL_ID], + ); + this.#rxChar = characteristics.find((c) => c.uuid === NUS_RX_ID) ?? null; + this.#txChar = characteristics.find((c) => c.uuid === NUS_TX_ID) ?? null; + this.#ctrlChar = characteristics.find((c) => c.uuid === NUS_CTRL_ID) ?? null; + if (!this.#rxChar || !this.#txChar) { + throw new Error("Triki NUS RX/TX characteristics not found on the device."); + } + + this.#txChar.on("data", (data) => this.#frameHandler?.(data)); + await this.#txChar.subscribeAsync(); + + await this.#startBattery(peripheral); // best-effort; never blocks streaming + } catch (err) { + this.#teardown(); + throw err; + } + } + + /** Read the Battery service once and subscribe to updates, if the device exposes it. */ + async #startBattery(peripheral: NoblePeripheral): Promise { + try { + const { characteristics } = await peripheral.discoverSomeServicesAndCharacteristicsAsync( + [BATTERY_SERVICE_ID], + [BATTERY_LEVEL_ID], + ); + const batt = characteristics.find((c) => c.uuid === BATTERY_LEVEL_ID); + if (!batt) return; + const value = await batt.readAsync(); + if (value.length) this.#batteryHandler?.(value[0]!); + batt.on("data", (data) => { + if (data.length) this.#batteryHandler?.(data[0]!); + }); + await batt.subscribeAsync(); + } catch { + /* no Battery service, or notifications unsupported — the read (if any) stands */ + } + } + + /** Re-scan and connect again (skeleton: same as a fresh connect). */ + async reconnect(): Promise { + await this.connect(); + } + + async writeRx(data: Uint8Array, withoutResponse = false): Promise { + if (!this.#rxChar) throw new Error("Not connected."); + await this.#rxChar.writeAsync(toBuffer(data), withoutResponse); + } + + async writeCtrl(data: Uint8Array): Promise { + if (!this.#ctrlChar) throw new Error("LED control characteristic is not available."); + await this.#ctrlChar.writeAsync(toBuffer(data), false); + } + + disconnect(): void { + const peripheral = this.#peripheral; + if (peripheral) void peripheral.disconnectAsync(); + else this.#onDisconnected(); + } + + // --- internal ---------------------------------------------------------------- + + #waitPoweredOn(noble: Noble): Promise { + if (noble.state === "poweredOn") return Promise.resolve(); + return new Promise((resolve, reject) => { + const onState = (state: string): void => { + if (state === "poweredOn") { + noble.removeListener("stateChange", onState); + resolve(); + } else if (state === "unsupported" || state === "unauthorized") { + noble.removeListener("stateChange", onState); + reject(new Error(`Bluetooth adapter is ${state}.`)); + } + }; + noble.on("stateChange", onState); + }); + } + + #scan(noble: Noble): Promise { + return new Promise((resolve, reject) => { + const onDiscover = (peripheral: NoblePeripheral): void => { + if (!this.#matches(peripheral)) return; + noble.removeListener("discover", onDiscover); + void noble.stopScanningAsync(); + resolve(peripheral); + }; + noble.on("discover", onDiscover); + // TODO: add a scan timeout that removes the listener and rejects. + noble.startScanningAsync([], false).catch(reject); + }); + } + + #matches(peripheral: NoblePeripheral): boolean { + if (this.#address) return peripheral.address?.toLowerCase() === this.#address; + const name = peripheral.advertisement?.localName ?? ""; + return name.toLowerCase().startsWith(this.#namePrefix.toLowerCase()); + } + + #onDisconnected = (): void => { + this.#teardown(); + this.#disconnectHandler?.(); + }; + + #teardown(): void { + this.#peripheral?.removeAllListeners("disconnect"); + this.#rxChar = null; + this.#txChar = null; + this.#ctrlChar = null; + this.#peripheral = null; + } +} + +/** noble's writes want a Node Buffer; build one without leaking Buffer into the API. */ +function toBuffer(data: Uint8Array): Uint8Array { + return Buffer.from(data.buffer, data.byteOffset, data.byteLength); +} diff --git a/packages/triki-controller/src/transports/web-bluetooth.ts b/packages/triki-controller/src/transports/web-bluetooth.ts new file mode 100644 index 0000000..634624d --- /dev/null +++ b/packages/triki-controller/src/transports/web-bluetooth.ts @@ -0,0 +1,201 @@ +/** + * Web Bluetooth implementation of {@link TrikiTransport}. This is the only module that + * touches `navigator.bluetooth`, and it is the default transport used by + * {@link TrikiController} in the browser. + */ +import { NUS_SERVICE, NUS_RX, NUS_TX, NUS_CTRL, BATTERY_SERVICE, BATTERY_LEVEL } from "../protocol"; +import type { TrikiTransport } from "../transport"; + +export class WebBluetoothTransport implements TrikiTransport { + #device: BluetoothDevice | null = null; + #gatt: BluetoothRemoteGATTServer | null = null; + #rxChar: BluetoothRemoteGATTCharacteristic | null = null; + #txChar: BluetoothRemoteGATTCharacteristic | null = null; + #ctrlChar: BluetoothRemoteGATTCharacteristic | null = null; + #batteryChar: BluetoothRemoteGATTCharacteristic | null = null; + #frameHandler: ((bytes: Uint8Array) => void) | null = null; + #disconnectHandler: (() => void) | null = null; + #batteryHandler: ((percent: number) => void) | null = null; + + /** True when Web Bluetooth is available (safe to call during SSR). */ + static isSupported(): boolean { + return typeof navigator !== "undefined" && !!navigator.bluetooth; + } + + get hasLed(): boolean { + return this.#ctrlChar !== null; + } + + onFrame(handler: (bytes: Uint8Array) => void): void { + this.#frameHandler = handler; + } + + onDisconnect(handler: () => void): void { + this.#disconnectHandler = handler; + } + + onBattery(handler: (percent: number) => void): void { + this.#batteryHandler = handler; + } + + /** + * Show the browser device picker and open a session. Must be called from a user + * gesture (e.g. a click handler). + */ + async connect(): Promise { + if (!WebBluetoothTransport.isSupported()) { + throw new Error("Web Bluetooth is not available in this environment."); + } + this.#device = await navigator.bluetooth.requestDevice({ + filters: [{ namePrefix: "TRIKI" }, { namePrefix: "Triki" }], + optionalServices: [NUS_SERVICE, BATTERY_SERVICE], + }); + await this.#openSession(); + } + + /** Reconnect to the previously paired device without showing the picker. */ + async reconnect(): Promise { + if (!this.#device) throw new Error("No device to reconnect to; call connect() first."); + await this.#openSession(); + } + + async writeRx(data: Uint8Array, withoutResponse = false): Promise { + if (!this.#rxChar) throw new Error("Not connected."); + await this.#write(this.#rxChar, data, withoutResponse); + } + + async writeCtrl(data: Uint8Array): Promise { + if (!this.#ctrlChar) throw new Error("LED control characteristic is not available."); + await this.#ctrlChar.writeValue(data as BufferSource); + } + + disconnect(): void { + if (this.#gatt && this.#gatt.connected) this.#gatt.disconnect(); + else this.#onGattDisconnected(); + } + + // --- internal ---------------------------------------------------------------- + + async #openSession(): Promise { + const device = this.#device; + if (!device || !device.gatt) throw new Error("Device GATT is not available."); + try { + // Same listener reference, so re-adding on reconnect is a no-op. + device.addEventListener("gattserverdisconnected", this.#onGattDisconnected); + + this.#gatt = await device.gatt.connect(); + const svc = await this.#gatt.getPrimaryService(NUS_SERVICE); + this.#rxChar = await svc.getCharacteristic(NUS_RX); + this.#txChar = await svc.getCharacteristic(NUS_TX); + try { + this.#ctrlChar = await svc.getCharacteristic(NUS_CTRL); + } catch { + this.#ctrlChar = null; + } + + await this.#txChar.startNotifications(); + this.#txChar.addEventListener("characteristicvaluechanged", this.#onNotify); + + void this.#startBattery(this.#gatt); // best-effort; never blocks streaming + } catch (err) { + this.#teardown(); + throw err; + } + } + + /** + * Read the battery level once and subscribe to updates, if the service exists. + * Fire-and-forget: `gatt` pins the session, and we bail after each await once it is + * no longer the active connection so a stale read/listener can't outlive teardown. + */ + async #startBattery(gatt: BluetoothRemoteGATTServer | null): Promise { + if (!gatt) return; + try { + const svc = await gatt.getPrimaryService(BATTERY_SERVICE); + if (this.#gatt !== gatt) return; + const char = await svc.getCharacteristic(BATTERY_LEVEL); + if (this.#gatt !== gatt) return; + const value = await char.readValue(); + if (this.#gatt !== gatt) return; + this.#emitBattery(value); + this.#batteryChar = char; + try { + await char.startNotifications(); + if (this.#gatt !== gatt) { + this.#batteryChar = null; // disconnected mid-subscribe; don't leak a listener + return; + } + char.addEventListener("characteristicvaluechanged", this.#onBattery); + } catch { + this.#batteryChar = null; // notifications unsupported; the one-time read stands + } + } catch { + this.#batteryChar = null; // no Battery service on this token + } + } + + #onBattery = (event: Event): void => { + const char = event.target as BluetoothRemoteGATTCharacteristic; + if (char.value) this.#emitBattery(char.value); + }; + + #emitBattery(view: DataView): void { + this.#batteryHandler?.(view.getUint8(0)); + } + + #onNotify = (event: Event): void => { + const char = event.target as BluetoothRemoteGATTCharacteristic; + const view = char.value; + if (!view) return; + // byteOffset/byteLength are load-bearing: the underlying buffer may be larger. + const bytes = new Uint8Array(view.buffer, view.byteOffset, view.byteLength); + this.#frameHandler?.(bytes); + }; + + #onGattDisconnected = (): void => { + this.#teardown(); + this.#disconnectHandler?.(); + }; + + #teardown(): void { + if (this.#txChar) { + try { + this.#txChar.removeEventListener("characteristicvaluechanged", this.#onNotify); + } catch { + /* ignore */ + } + } + if (this.#batteryChar) { + try { + this.#batteryChar.removeEventListener("characteristicvaluechanged", this.#onBattery); + } catch { + /* ignore */ + } + } + if (this.#device) { + try { + this.#device.removeEventListener("gattserverdisconnected", this.#onGattDisconnected); + } catch { + /* ignore */ + } + } + this.#rxChar = null; + this.#txChar = null; + this.#ctrlChar = null; + this.#batteryChar = null; + this.#gatt = null; + // #device is retained so reconnect() can reuse it without the picker. + } + + async #write( + char: BluetoothRemoteGATTCharacteristic, + data: Uint8Array, + withoutResponse: boolean, + ): Promise { + if (withoutResponse && typeof char.writeValueWithoutResponse === "function") { + await char.writeValueWithoutResponse(data as BufferSource); + } else { + await char.writeValue(data as BufferSource); + } + } +} diff --git a/packages/triki-controller/tsup.config.ts b/packages/triki-controller/tsup.config.ts index 08aef33..6b3b8eb 100644 --- a/packages/triki-controller/tsup.config.ts +++ b/packages/triki-controller/tsup.config.ts @@ -1,7 +1,7 @@ import { defineConfig } from "tsup"; export default defineConfig({ - entry: ["src/index.ts"], + entry: ["src/index.ts", "src/node.ts"], format: ["esm"], target: "es2020", dts: true, @@ -10,4 +10,6 @@ export default defineConfig({ splitting: false, clean: true, outDir: "dist", + // Optional native dependency for the Node transport — resolved at runtime, never bundled. + external: ["@abandonware/noble"], }); From ebce3222fcfd6150c5e4e47e54a996219b3cf4b8 Mon Sep 17 00:00:00 2001 From: Flop Butylkin Date: Sat, 4 Jul 2026 18:34:41 +0200 Subject: [PATCH 2/7] =?UTF-8?q?feat(triki-controller):=20TrikiTransport=20?= =?UTF-8?q?v2=20=E2=80=94=20attach/detach=20contract,=20controller=20harde?= =?UTF-8?q?ning?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The transport interface becomes explicitly owned and its contract honest: - attach(handlers)/detach() replace the three single-slot handler setters: a transport receives one handlers object (onFrame/onDisconnect/onBattery) and only calls what it has data for, so a plain-JS custom transport can no longer crash the controller constructor, and a second controller can't silently hijack an attached transport (attach throws). - The disconnect() contract is now implementable: onDisconnect fires exactly once per successful connect and NEVER when no connect succeeded; a pending connect() must reject when disconnect() is called mid-flight. - Optional preflight(op) hook: sync precondition checks (no Web Bluetooth, reconnect-before-connect) run before setState("pairing"), so those failures reject with zero spurious connectionchange events, as on main. Controller state machine fixes: - connect()/reconnect() share one #open() flow; a failure after the transport connected (e.g. the START write rejects) now also closes the transport, so no live link keeps feeding events into a "disconnected" controller. - disconnect() lost its state early-return — a leaked link is always closable — and gained a local-cleanup fallback against buggy transports. - #ingest ignores frames outside "streaming"; #handleBattery drops readings after cleanup (but keeps pairing-time reads); #setState dedupes transitions. - assertTransport() validates a custom transport's shape up front with a TypeError naming every missing member. protocol.ts now carries canonical 128-bit battery UUIDs (usable beyond Web Bluetooth's alias strings) and a shared DEVICE_NAME_PREFIX. New coverage: scripted FakeTransport testkit + 12 controller<->transport seam tests (failure cleanup, cancellation, guards, ownership) and 2 zero-event preflight regressions. Co-Authored-By: Claude Fable 5 --- .../triki-controller/src/controller.test.ts | 22 ++ .../src/controller.transport.test.ts | 250 ++++++++++++++++++ packages/triki-controller/src/controller.ts | 120 ++++++--- packages/triki-controller/src/events.ts | 3 +- packages/triki-controller/src/index.ts | 3 +- packages/triki-controller/src/protocol.ts | 9 +- .../src/testkit/fakeTransport.ts | 115 ++++++++ packages/triki-controller/src/transport.ts | 85 ++++-- .../src/transports/web-bluetooth.ts | 118 +++++++-- 9 files changed, 637 insertions(+), 88 deletions(-) create mode 100644 packages/triki-controller/src/controller.transport.test.ts create mode 100644 packages/triki-controller/src/testkit/fakeTransport.ts diff --git a/packages/triki-controller/src/controller.test.ts b/packages/triki-controller/src/controller.test.ts index c268e73..55e7e11 100644 --- a/packages/triki-controller/src/controller.test.ts +++ b/packages/triki-controller/src/controller.test.ts @@ -291,6 +291,28 @@ describe("lifecycle", () => { h.gatt.disconnect(); // simulate the peripheral dropping the link expect(ctrl.state).toBe("disconnected"); }); + + test("connect() without Web Bluetooth rejects with zero connectionchange events", async () => { + vi.stubGlobal("navigator", {}); // no navigator.bluetooth + const ctrl = make({ fusion: "none" }); + const states: ConnectionState[] = []; + ctrl.on("connectionchange", (s) => states.push(s)); + + await expect(ctrl.connect()).rejects.toThrow("Web Bluetooth is not available"); + expect(states).toEqual([]); // preflight fails before any state change + expect(ctrl.state).toBe("disconnected"); + }); + + test("reconnect() before any connect() rejects with zero connectionchange events", async () => { + install(); + const ctrl = make({ fusion: "none" }); + const states: ConnectionState[] = []; + ctrl.on("connectionchange", (s) => states.push(s)); + + await expect(ctrl.reconnect()).rejects.toThrow("No device to reconnect to"); + expect(states).toEqual([]); + expect(ctrl.state).toBe("disconnected"); + }); }); describe("rate measurement", () => { diff --git a/packages/triki-controller/src/controller.transport.test.ts b/packages/triki-controller/src/controller.transport.test.ts new file mode 100644 index 0000000..b3fc2bc --- /dev/null +++ b/packages/triki-controller/src/controller.transport.test.ts @@ -0,0 +1,250 @@ +/** + * Controller <-> transport seam tests, driven through an injected scripted + * {@link FakeTransport} (no Web Bluetooth involved): failure cleanup, cancellation, + * state guards, and the constructor's transport validation. + */ +import { describe, test, expect, vi, afterEach } from "vitest"; +import { TrikiController } from "./controller"; +import { startCmd } from "./protocol"; +import { FakeTransport, deferred } from "./testkit/fakeTransport"; +import { frame } from "./testkit/frames"; +import type { ConnectionState, FrameEvent, TrikiControllerOptions } from "./events"; +import type { TrikiTransport } from "./transport"; + +const created: TrikiController[] = []; + +/** Construct a controller and track it so afterEach tears down its rate timer. */ +function make(options?: TrikiControllerOptions): TrikiController { + const c = new TrikiController(options); + created.push(c); + return c; +} + +/** Controller + fake transport + recorded connectionchange history. */ +function rig(): { ctrl: TrikiController; t: FakeTransport; states: ConnectionState[] } { + const t = new FakeTransport(); + const ctrl = make({ fusion: "none", transport: t }); + const states: ConnectionState[] = []; + ctrl.on("connectionchange", (s) => states.push(s)); + return { ctrl, t, states }; +} + +afterEach(() => { + for (const c of created.splice(0)) c.disconnect(); + vi.restoreAllMocks(); +}); + +describe("happy path through an injected transport", () => { + test("connect sends START, streams frames, disconnect tears down once", async () => { + const { ctrl, t, states } = rig(); + + await ctrl.connect(); + expect(states).toEqual(["pairing", "streaming"]); + expect(Array.from(t.rxWrites[0]!.data)).toEqual(Array.from(startCmd(104))); + + const frames: FrameEvent[] = []; + ctrl.on("frame", (e) => frames.push(e)); + t.emitFrame(frame(0, 0, 0, 0, 0, 2048)); + expect(frames).toHaveLength(1); + + ctrl.disconnect(); + expect(states).toEqual(["pairing", "streaming", "disconnected"]); + }); + + test("a minimal object-literal transport (per the docs) is enough", async () => { + // Exactly the members the Custom Transports docs list — no battery, no reconnect. + let handlers: Parameters[0] | null = null; + const minimal: TrikiTransport = { + attach: (h) => { + handlers = h; + }, + detach: () => { + handlers = null; + }, + connect: async () => {}, + writeRx: async () => {}, + writeCtrl: async () => {}, + hasLed: false, + disconnect: () => { + handlers?.onDisconnect(); + }, + }; + const ctrl = make({ fusion: "none", transport: minimal }); + + await ctrl.connect(); + expect(ctrl.state).toBe("streaming"); + expect(ctrl.battery).toBeNull(); + + const frames: FrameEvent[] = []; + ctrl.on("frame", (e) => frames.push(e)); + handlers!.onFrame(frame(0, 0, 0, 0, 0, 2048)); + expect(frames).toHaveLength(1); + }); +}); + +describe("connect failure cleanup", () => { + test("transport.connect() rejection: pairing -> disconnected, transport closed", async () => { + const { ctrl, t, states } = rig(); + t.connectError = new Error("no adapter"); + + await expect(ctrl.connect()).rejects.toThrow("no adapter"); + expect(states).toEqual(["pairing", "disconnected"]); + expect(ctrl.state).toBe("disconnected"); + expect(t.disconnectCount).toBeGreaterThanOrEqual(1); // half-open link closed + }); + + test("START write rejection: transport closed, no zombie frames, still recoverable", async () => { + const { ctrl, t, states } = rig(); + t.writeRxError = new Error("GATT write failed"); + + await expect(ctrl.connect()).rejects.toThrow("GATT write failed"); + expect(states).toEqual(["pairing", "disconnected"]); + // The transport was told to tear down the connected-but-not-streaming link. + expect(t.disconnectCount).toBeGreaterThanOrEqual(1); + expect(t.connected).toBe(false); + + // Notifications from the (hypothetically) lingering link never become events. + const frames: FrameEvent[] = []; + ctrl.on("frame", (e) => frames.push(e)); + t.emitFrame(frame(1, 0, 0, 0, 0, 0)); + expect(frames).toHaveLength(0); + + // And the controller is not wedged: a later connect() works. + t.writeRxError = null; + await ctrl.connect(); + expect(ctrl.state).toBe("streaming"); + }); +}); + +describe("disconnect during a pending connect", () => { + test("cancels: connect() rejects, state never reaches streaming", async () => { + const { ctrl, t, states } = rig(); + const gate = deferred(); + t.connectGate = gate.promise; + + const pending = ctrl.connect(); + expect(ctrl.state).toBe("pairing"); + + ctrl.disconnect(); + await expect(pending).rejects.toThrow("cancelled"); + expect(ctrl.state).toBe("disconnected"); + expect(states).toEqual(["pairing", "disconnected"]); + + // A late gate resolution must not resurrect the session. + gate.resolve(); + await new Promise((r) => setTimeout(r, 0)); + expect(ctrl.state).toBe("disconnected"); + expect(states).toEqual(["pairing", "disconnected"]); + }); +}); + +describe("state guards", () => { + test("frames outside streaming are ignored (pairing and after disconnect)", async () => { + const { ctrl, t } = rig(); + const frames: FrameEvent[] = []; + ctrl.on("frame", (e) => frames.push(e)); + + const gate = deferred(); + t.connectGate = gate.promise; + const pending = ctrl.connect(); + t.emitFrame(frame(1, 0, 0, 0, 0, 0)); // during pairing + expect(frames).toHaveLength(0); + + gate.resolve(); + await pending; + t.emitFrame(frame(1, 0, 0, 0, 0, 0)); // streaming: counts + expect(frames).toHaveLength(1); + + ctrl.disconnect(); + t.emitFrame(frame(1, 0, 0, 0, 0, 0)); // after disconnect: ignored + expect(frames).toHaveLength(1); + }); + + test("battery during pairing is kept; battery after disconnect is dropped", async () => { + const { ctrl, t } = rig(); + const readings: number[] = []; + ctrl.on("battery", (p) => readings.push(p)); + + const gate = deferred(); + t.connectGate = gate.promise; + const pending = ctrl.connect(); + t.emitBattery(77); // fire-and-forget reads may land before streaming + expect(readings).toEqual([77]); + expect(ctrl.battery).toBe(77); + + gate.resolve(); + await pending; + ctrl.disconnect(); + expect(ctrl.battery).toBeNull(); // cleanup resets it + + t.emitBattery(42); // stale late reading + expect(readings).toEqual([77]); + expect(ctrl.battery).toBeNull(); + }); + + test("duplicate transport disconnects collapse to one connectionchange", async () => { + const { ctrl, t, states } = rig(); + await ctrl.connect(); + + t.emitDisconnect(); + t.emitDisconnect(); // a buggy/racy transport double-fires + ctrl.disconnect(); // and the app disconnects on top + + expect(states).toEqual(["pairing", "streaming", "disconnected"]); + }); +}); + +describe("reconnect through the transport", () => { + test("drives transport.reconnect() and restarts the stream", async () => { + const { ctrl, t, states } = rig(); + await ctrl.connect(); + t.emitDisconnect(); + expect(ctrl.state).toBe("disconnected"); + + await ctrl.reconnect(); + expect(t.reconnectCount).toBe(1); + expect(states).toEqual(["pairing", "streaming", "disconnected", "pairing", "streaming"]); + // START was re-sent on the new session. + expect(Array.from(t.rxWrites[1]!.data)).toEqual(Array.from(startCmd(104))); + }); + + test("a transport without reconnect() rejects with zero events", async () => { + let handlers: Parameters[0] | null = null; + const minimal: TrikiTransport = { + attach: (h) => { + handlers = h; + }, + detach: () => { + handlers = null; + }, + connect: async () => {}, + writeRx: async () => {}, + writeCtrl: async () => {}, + hasLed: false, + disconnect: () => { + handlers?.onDisconnect(); + }, + }; + const ctrl = make({ fusion: "none", transport: minimal }); + const states: ConnectionState[] = []; + ctrl.on("connectionchange", (s) => states.push(s)); + + await expect(ctrl.reconnect()).rejects.toThrow("does not support reconnect"); + expect(states).toEqual([]); + }); +}); + +describe("transport ownership and validation", () => { + test("the constructor rejects a malformed transport with a member list", () => { + expect(() => make({ transport: {} as unknown as TrikiTransport })).toThrow(TypeError); + expect(() => make({ transport: {} as unknown as TrikiTransport })).toThrow( + /attach.*detach.*connect.*writeRx.*writeCtrl.*disconnect.*hasLed/, + ); + }); + + test("a second controller cannot silently claim an attached transport", () => { + const t = new FakeTransport(); + make({ fusion: "none", transport: t }); + expect(() => make({ fusion: "none", transport: t })).toThrow("already attached"); + }); +}); diff --git a/packages/triki-controller/src/controller.ts b/packages/triki-controller/src/controller.ts index e572c46..d5ea46a 100644 --- a/packages/triki-controller/src/controller.ts +++ b/packages/triki-controller/src/controller.ts @@ -53,6 +53,29 @@ function resolveFusion(opt: boolean | FusionAlgorithm | undefined): FusionAlgori return opt; } +/** + * Validate a custom transport's shape up front: one TypeError naming every missing + * member beats a property-access crash deep inside connect() (plain-JS transports + * get no compiler help). + */ +function assertTransport(t: TrikiTransport): void { + const isFn = (name: string): boolean => + typeof (t as unknown as Record)[name] === "function"; + const problems: string[] = []; + for (const name of ["attach", "detach", "connect", "writeRx", "writeCtrl", "disconnect"]) { + if (!isFn(name)) problems.push(name); + } + if (!("hasLed" in t)) problems.push("hasLed"); + for (const name of ["reconnect", "preflight"] as const) { + if (t[name] !== undefined && typeof t[name] !== "function") problems.push(name); + } + if (problems.length) { + throw new TypeError( + `Invalid TrikiTransport: missing or non-function member(s): ${problems.join(", ")}.`, + ); + } +} + const RATE_WINDOW_MS = 1000; const MAX_DT_S = 0.2; @@ -86,9 +109,12 @@ export class TrikiController extends TypedEmitter { this.#tauAcc = options.tauAcc ?? DEFAULT_TAU_ACC; this.#filter = this.#makeFilter(this.#algo); this.#transport = options.transport ?? new WebBluetoothTransport(); - this.#transport.onFrame(this.#ingest); - this.#transport.onDisconnect(this.#handleTransportDisconnect); - this.#transport.onBattery(this.#handleBattery); + assertTransport(this.#transport); + this.#transport.attach({ + onFrame: this.#ingest, + onDisconnect: this.#handleTransportDisconnect, + onBattery: this.#handleBattery, + }); } #makeFilter(algo: FusionAlgorithm): OrientationFilter | null { @@ -141,44 +167,40 @@ export class TrikiController extends TypedEmitter { /** * Connect through the transport and start the IMU stream. With the default Web * Bluetooth transport this shows the device picker and must be called from a user - * gesture (e.g. a click handler). Rejects (after cleaning up) if the transport or - * the handshake fails. + * gesture (e.g. a click handler). Precondition failures (e.g. no Web Bluetooth) + * reject with zero `connectionchange` events; after a real attempt starts, failure + * cleans up (transport included) and rejects after a `disconnected` event. */ async connect(): Promise { - if (this.#state !== "disconnected") return; - try { - this.#setState("pairing"); - await this.#transport.connect(); - await this.#startStreaming(); - } catch (err) { - this.#cleanup(); - throw err; - } + return this.#open("connect", () => this.#transport.connect()); } /** - * Reconnect to the previously selected device without re-prompting. Throws if the - * transport does not support reconnecting (e.g. {@link connect} was never called). + * Reconnect to the previously selected device without re-prompting. Rejects with + * zero events if the transport does not support reconnecting or has no device to + * reconnect to (e.g. {@link connect} was never called). */ async reconnect(): Promise { - if (this.#state !== "disconnected") return; - if (!this.#transport.reconnect) { + if (typeof this.#transport.reconnect !== "function") { throw new Error("This transport does not support reconnect()."); } - try { - this.#setState("pairing"); - await this.#transport.reconnect(); - await this.#startStreaming(); - } catch (err) { - this.#cleanup(); - throw err; - } + return this.#open("reconnect", () => this.#transport.reconnect!()); } - /** Disconnect from the token. Triggers a `connectionchange` to `disconnected`. */ + /** + * Disconnect from the token. Idempotent and safe in any state — including a pending + * {@link connect} (which then rejects). Triggers a `connectionchange` to + * `disconnected` when leaving `pairing`/`streaming`. + */ disconnect(): void { - if (this.#state === "disconnected") return; - this.#transport.disconnect(); + try { + this.#transport.disconnect(); + } catch { + /* still reset local state below */ + } + // The transport's onDisconnect normally drives this; the fallback keeps the + // controller recoverable even against a buggy transport. + if (this.#state !== "disconnected") this.#cleanup(); } /** Turn the green LED on or off. Throws when the LED characteristic is unavailable. */ @@ -247,15 +269,42 @@ export class TrikiController extends TypedEmitter { // --- internal ---------------------------------------------------------------- - /** Start the stream once the transport is connected: send START, begin timers. */ - async #startStreaming(): Promise { - await this.#transport.writeRx(startCmd(this.#rateHz), true); - this.#startRateTimer(); - this.#setState("streaming"); + /** + * Shared connect/reconnect flow: sync pre-flight (zero events on failure), then + * `pairing` → establish → START → `streaming`; any failure (including a + * {@link disconnect} racing the awaits) cleans up locally, closes the transport + * best-effort, and rethrows. + */ + async #open(op: "connect" | "reconnect", establish: () => Promise): Promise { + if (this.#state !== "disconnected") return; + this.#transport.preflight?.(op); // throw → rejection with zero connectionchange events + this.#setState("pairing"); + try { + await establish(); + this.#assertPairing(); + await this.#transport.writeRx(startCmd(this.#rateHz), true); + this.#assertPairing(); + this.#startRateTimer(); + this.#setState("streaming"); + } catch (err) { + this.#cleanup(); + try { + this.#transport.disconnect(); // close a half-open link; #setState dedupes + } catch { + /* best-effort */ + } + throw err; + } + } + + /** A disconnect (or lost link) intervened while connecting. */ + #assertPairing(): void { + if (this.#state !== "pairing") throw new Error("Connection cancelled while connecting."); } /** Transport callback: decode inbound notification bytes into frames. */ #ingest = (bytes: Uint8Array): void => { + if (this.#state !== "streaming") return; // never emit frames outside streaming for (const frame of this.#parser.push(bytes)) this.#handleFrame(frame); }; @@ -266,6 +315,8 @@ export class TrikiController extends TypedEmitter { /** Transport callback: a battery-level reading (percent). */ #handleBattery = (percent: number): void => { + // Allow "pairing" — a fire-and-forget battery read may land before streaming. + if (this.#state === "disconnected") return; this.#battery = percent; this.emit("battery", percent); }; @@ -324,6 +375,7 @@ export class TrikiController extends TypedEmitter { } #setState(state: ConnectionState): void { + if (this.#state === state) return; // dedupe: multi-path cleanup emits once this.#state = state; this.emit("connectionchange", state); } diff --git a/packages/triki-controller/src/events.ts b/packages/triki-controller/src/events.ts index cdb161d..d16d792 100644 --- a/packages/triki-controller/src/events.ts +++ b/packages/triki-controller/src/events.ts @@ -80,7 +80,8 @@ export interface TrikiControllerOptions { /** * BLE transport. Defaults to a `WebBluetoothTransport` (browser). Pass a * `NobleTransport` (from `triki-controller/node`) to receive outside the browser, - * or any object implementing {@link TrikiTransport}. + * or any object implementing {@link TrikiTransport}. The controller claims the + * transport via `attach()` in its constructor — one controller per transport. */ transport?: TrikiTransport; } diff --git a/packages/triki-controller/src/index.ts b/packages/triki-controller/src/index.ts index 6376674..2766d7c 100644 --- a/packages/triki-controller/src/index.ts +++ b/packages/triki-controller/src/index.ts @@ -14,6 +14,7 @@ export { NUS_CTRL, BATTERY_SERVICE, BATTERY_LEVEL, + DEVICE_NAME_PREFIX, HEADER0, HEADER1, FRAME_LEN, @@ -25,7 +26,7 @@ export { SUPPORTED_RATES_HZ, } from "./protocol"; -export type { TrikiTransport } from "./transport"; +export type { TrikiTransport, TrikiTransportHandlers } from "./transport"; export type { Quaternion, EulerAngles, MadgwickOptions, OrientationFilter } from "./fusion"; export type { VqfOptions } from "./vqf"; export type { RawFrame } from "./parser"; diff --git a/packages/triki-controller/src/protocol.ts b/packages/triki-controller/src/protocol.ts index 25c75d9..1bb0575 100644 --- a/packages/triki-controller/src/protocol.ts +++ b/packages/triki-controller/src/protocol.ts @@ -14,10 +14,13 @@ export const NUS_TX = "6e400003-b5a3-f393-e0a9-e50e24dcca9e"; /** Control register — 1 byte, drives the green LED (0x01 on / 0x00 off). */ export const NUS_CTRL = "6e400004-b5a3-f393-e0a9-e50e24dcca9e"; -/** Standard Battery Service (0x180F). */ -export const BATTERY_SERVICE = "battery_service"; +/** Standard Battery Service (0x180F), as a canonical 128-bit UUID. */ +export const BATTERY_SERVICE = "0000180f-0000-1000-8000-00805f9b34fb"; /** Standard Battery Level characteristic (0x2A19) — uint8 percent. */ -export const BATTERY_LEVEL = "battery_level"; +export const BATTERY_LEVEL = "00002a19-0000-1000-8000-00805f9b34fb"; + +/** Advertised device-name prefix ("TRIKI "; some firmware advertises "Triki"). */ +export const DEVICE_NAME_PREFIX = "TRIKI"; /** * Base bytes of the 8-byte IMU start command. Bytes 5-6 carry the sample rate diff --git a/packages/triki-controller/src/testkit/fakeTransport.ts b/packages/triki-controller/src/testkit/fakeTransport.ts new file mode 100644 index 0000000..ab0d801 --- /dev/null +++ b/packages/triki-controller/src/testkit/fakeTransport.ts @@ -0,0 +1,115 @@ +/** + * A scripted in-memory {@link TrikiTransport} for controller tests: no BLE stack at + * all — tests gate/fail connect() and drive frames/battery/disconnects by hand. It + * honors the transport contract (exactly-once onDisconnect, reject-on-cancel, + * idempotent disconnect) so controller tests exercise the same seams a real + * transport would. + */ +import type { TrikiTransport, TrikiTransportHandlers } from "../transport"; + +/** A promise with its settle functions exposed — the test-side gate primitive. */ +export function deferred(): { + promise: Promise; + resolve: (value: T) => void; + reject: (err: unknown) => void; +} { + let resolve!: (value: T) => void; + let reject!: (err: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +export class FakeTransport implements TrikiTransport { + handlers: TrikiTransportHandlers | null = null; + hasLed = true; + connected = false; + + /** When set, connect()/reconnect() reject with this error. */ + connectError: Error | null = null; + /** When set, connect()/reconnect() await this gate (cancellable by disconnect()). */ + connectGate: Promise | null = null; + /** When set, writeRx() rejects with this error (models a failed START handshake). */ + writeRxError: Error | null = null; + + rxWrites: { data: Uint8Array; withoutResponse: boolean }[] = []; + ctrlWrites: Uint8Array[] = []; + connectCount = 0; + reconnectCount = 0; + disconnectCount = 0; + + #cancelPending: ((err: Error) => void) | null = null; + + attach(handlers: TrikiTransportHandlers): void { + if (this.handlers) throw new Error("FakeTransport is already attached."); + this.handlers = handlers; + } + + detach(): void { + this.handlers = null; + } + + async connect(): Promise { + this.connectCount++; + await this.#establish(); + } + + async reconnect(): Promise { + this.reconnectCount++; + await this.#establish(); + } + + async #establish(): Promise { + if (this.connectError) throw this.connectError; + if (this.connectGate) { + await Promise.race([ + this.connectGate, + new Promise((_, reject) => { + this.#cancelPending = reject; + }), + ]); + this.#cancelPending = null; + } + this.connected = true; + } + + async writeRx(data: Uint8Array, withoutResponse = false): Promise { + if (this.writeRxError) throw this.writeRxError; + this.rxWrites.push({ data, withoutResponse }); + } + + async writeCtrl(data: Uint8Array): Promise { + this.ctrlWrites.push(data); + } + + disconnect(): void { + this.disconnectCount++; + if (this.#cancelPending) { + const cancel = this.#cancelPending; + this.#cancelPending = null; + cancel(new Error("Connection cancelled by disconnect().")); + return; + } + if (!this.connected) return; // idempotent; never connected -> no onDisconnect + this.connected = false; + this.handlers?.onDisconnect(); + } + + // --- test drivers -------------------------------------------------------------- + + emitFrame(bytes: Uint8Array): void { + this.handlers?.onFrame(bytes); + } + + /** Simulate a remote link drop (fires the handler regardless of bookkeeping). */ + emitDisconnect(): void { + this.connected = false; + this.handlers?.onDisconnect(); + } + + emitBattery(percent: number): void { + this.handlers?.onBattery(percent); + } +} diff --git a/packages/triki-controller/src/transport.ts b/packages/triki-controller/src/transport.ts index eae30a4..0387e11 100644 --- a/packages/triki-controller/src/transport.ts +++ b/packages/triki-controller/src/transport.ts @@ -7,20 +7,73 @@ * {@link TrikiController} builds the START/LED commands and parses the motion frames — * so protocol logic lives in exactly one place and new transports stay tiny. * + * Ownership is explicit: the controller claims a transport with {@link + * TrikiTransport.attach} (one owner at a time) and hands over a {@link + * TrikiTransportHandlers} object. A transport only ever calls the handlers it has data + * for, so minimal transports (no battery, no reconnect) need no stub members. + * * `TrikiController` defaults to a `WebBluetoothTransport` (browser). A `NobleTransport` * (Node, via `@abandonware/noble`) is available from the `triki-controller/node` entry. */ + +/** Callbacks a {@link TrikiTransport} owner registers via {@link TrikiTransport.attach}. */ +export interface TrikiTransportHandlers { + /** Inbound TX notification bytes (raw). Only called while a connection is active. */ + onFrame(bytes: Uint8Array): void; + + /** + * The link went down — lost, or torn down by {@link TrikiTransport.disconnect}. + * Fires exactly once per successful `connect()`/`reconnect()`, and never when no + * connect has succeeded (a failed connect signals through its rejection instead). + */ + onDisconnect(): void; + + /** + * A battery-level reading (percent, 0–100). Transports that can read the standard + * Battery service call it on connect and on each update; others never call it. + */ + onBattery(percent: number): void; +} + export interface TrikiTransport { + /** + * Claim this transport and register the owner's callbacks. A transport serves one + * owner at a time: throws if already attached (call {@link detach} first). The + * controller attaches once, in its constructor, before any {@link connect}. + */ + attach(handlers: TrikiTransportHandlers): void; + + /** + * Release the transport so it can be attached again. Does not tear down an active + * link — call {@link disconnect} first. Safe no-op when not attached. + */ + detach(): void; + + /** + * Optional synchronous pre-flight check: throw a descriptive Error when the given + * operation cannot possibly succeed (e.g. Web Bluetooth missing, or `reconnect` + * before any successful connect). The controller calls it before entering the + * `pairing` state, so failures reject without emitting any `connectionchange` + * events. + */ + preflight?(op: "connect" | "reconnect"): void; + /** * Find/select the device, connect, discover the NUS characteristics, and start TX - * notifications. Resolves once frames can flow; rejects (after cleaning up) on failure. + * notifications. Resolves once frames can flow. + * + * Contract: on failure it rejects AFTER cleaning up its own resources and MUST NOT + * invoke `handlers.onDisconnect()` (no connect succeeded — the rejection is the + * signal). If {@link disconnect} is called while `connect()` is pending, `connect()` + * must reject. */ connect(): Promise; /** * Reconnect to the previously selected device without re-prompting. Optional — * transports that cannot remember a device may omit it, in which case - * {@link TrikiController.reconnect} throws a clear error. + * {@link TrikiController.reconnect} rejects with a clear error. Same contract as + * {@link connect}. */ reconnect?(): Promise; @@ -34,27 +87,13 @@ export interface TrikiTransport { writeCtrl(data: Uint8Array): Promise; /** - * Register the handler for inbound TX notification chunks (raw bytes). Called once by - * the controller; the transport invokes it for every notification while connected. - */ - onFrame(handler: (bytes: Uint8Array) => void): void; - - /** - * Register the handler for a disconnect — whether a lost link or the result of - * {@link disconnect}. Called once by the controller. - */ - onDisconnect(handler: () => void): void; - - /** - * Register the handler for battery-level readings (percent, 0–100). Called once by - * the controller. Transports that can read the standard Battery service invoke it on - * connect and on each update; others may never call it. - */ - onBattery(handler: (percent: number) => void): void; - - /** - * Tear down the connection. Guarantees the {@link onDisconnect} handler fires once - * when a connection was active; a safe no-op otherwise. + * Tear down the connection. Idempotent and safe to call in any state. + * + * Contract: if a connection is active (`connect()`/`reconnect()` resolved and + * `onDisconnect` has not fired since), tear it down and fire + * `handlers.onDisconnect()` exactly once. If a `connect()` is pending, cancel it so + * the pending promise rejects (still without firing `onDisconnect`). Otherwise do + * nothing — NEVER fire `onDisconnect()` when no connect has succeeded. */ disconnect(): void; } diff --git a/packages/triki-controller/src/transports/web-bluetooth.ts b/packages/triki-controller/src/transports/web-bluetooth.ts index 634624d..f200f45 100644 --- a/packages/triki-controller/src/transports/web-bluetooth.ts +++ b/packages/triki-controller/src/transports/web-bluetooth.ts @@ -3,8 +3,16 @@ * touches `navigator.bluetooth`, and it is the default transport used by * {@link TrikiController} in the browser. */ -import { NUS_SERVICE, NUS_RX, NUS_TX, NUS_CTRL, BATTERY_SERVICE, BATTERY_LEVEL } from "../protocol"; -import type { TrikiTransport } from "../transport"; +import { + NUS_SERVICE, + NUS_RX, + NUS_TX, + NUS_CTRL, + BATTERY_SERVICE, + BATTERY_LEVEL, + DEVICE_NAME_PREFIX, +} from "../protocol"; +import type { TrikiTransport, TrikiTransportHandlers } from "../transport"; export class WebBluetoothTransport implements TrikiTransport { #device: BluetoothDevice | null = null; @@ -13,9 +21,13 @@ export class WebBluetoothTransport implements TrikiTransport { #txChar: BluetoothRemoteGATTCharacteristic | null = null; #ctrlChar: BluetoothRemoteGATTCharacteristic | null = null; #batteryChar: BluetoothRemoteGATTCharacteristic | null = null; - #frameHandler: ((bytes: Uint8Array) => void) | null = null; - #disconnectHandler: (() => void) | null = null; - #batteryHandler: ((percent: number) => void) | null = null; + #handlers: TrikiTransportHandlers | null = null; + /** True only between a successful connect()/reconnect() and the next teardown. */ + #connected = false; + /** True while a connect()/reconnect() is in flight. */ + #connecting = false; + /** Set by disconnect() during a pending connect; honored at the next checkpoint. */ + #cancelRequested = false; /** True when Web Bluetooth is available (safe to call during SSR). */ static isSupported(): boolean { @@ -26,16 +38,23 @@ export class WebBluetoothTransport implements TrikiTransport { return this.#ctrlChar !== null; } - onFrame(handler: (bytes: Uint8Array) => void): void { - this.#frameHandler = handler; + attach(handlers: TrikiTransportHandlers): void { + if (this.#handlers) throw new Error("WebBluetoothTransport is already attached."); + this.#handlers = handlers; } - onDisconnect(handler: () => void): void { - this.#disconnectHandler = handler; + detach(): void { + this.#handlers = null; } - onBattery(handler: (percent: number) => void): void { - this.#batteryHandler = handler; + /** Synchronous pre-flight: fail before any state change when the op cannot succeed. */ + preflight(op: "connect" | "reconnect"): void { + if (!WebBluetoothTransport.isSupported()) { + throw new Error("Web Bluetooth is not available in this environment."); + } + if (op === "reconnect" && !this.#device) { + throw new Error("No device to reconnect to; call connect() first."); + } } /** @@ -43,20 +62,37 @@ export class WebBluetoothTransport implements TrikiTransport { * gesture (e.g. a click handler). */ async connect(): Promise { - if (!WebBluetoothTransport.isSupported()) { - throw new Error("Web Bluetooth is not available in this environment."); + this.preflight("connect"); + this.#connecting = true; + this.#cancelRequested = false; + try { + // A pending requestDevice picker cannot be cancelled programmatically; + // disconnect() during it takes effect at the checkpoint below. + this.#device = await navigator.bluetooth.requestDevice({ + // Web Bluetooth prefix matching is case-sensitive, hence both shipped casings + // (noble's transport matches case-insensitively — inherently broader). + filters: [{ namePrefix: DEVICE_NAME_PREFIX }, { namePrefix: "Triki" }], + optionalServices: [NUS_SERVICE, BATTERY_SERVICE], + }); + this.#checkCancelled(); + await this.#openSession(); + } finally { + this.#connecting = false; + this.#cancelRequested = false; } - this.#device = await navigator.bluetooth.requestDevice({ - filters: [{ namePrefix: "TRIKI" }, { namePrefix: "Triki" }], - optionalServices: [NUS_SERVICE, BATTERY_SERVICE], - }); - await this.#openSession(); } /** Reconnect to the previously paired device without showing the picker. */ async reconnect(): Promise { - if (!this.#device) throw new Error("No device to reconnect to; call connect() first."); - await this.#openSession(); + this.preflight("reconnect"); + this.#connecting = true; + this.#cancelRequested = false; + try { + await this.#openSession(); + } finally { + this.#connecting = false; + this.#cancelRequested = false; + } } async writeRx(data: Uint8Array, withoutResponse = false): Promise { @@ -66,16 +102,28 @@ export class WebBluetoothTransport implements TrikiTransport { async writeCtrl(data: Uint8Array): Promise { if (!this.#ctrlChar) throw new Error("LED control characteristic is not available."); - await this.#ctrlChar.writeValue(data as BufferSource); + await this.#write(this.#ctrlChar, data, false); } disconnect(): void { - if (this.#gatt && this.#gatt.connected) this.#gatt.disconnect(); - else this.#onGattDisconnected(); + if (this.#connecting) { + // Cancel the pending connect: the in-flight session setup rejects at its next + // checkpoint (or its GATT await fails once the link drops). No onDisconnect — + // the connect() rejection is the signal. + this.#cancelRequested = true; + if (this.#gatt?.connected) this.#gatt.disconnect(); + return; + } + if (this.#gatt?.connected) this.#gatt.disconnect(); + // Never connected (or already torn down): idempotent no-op, no onDisconnect. } // --- internal ---------------------------------------------------------------- + #checkCancelled(): void { + if (this.#cancelRequested) throw new Error("Connection cancelled by disconnect()."); + } + async #openSession(): Promise { const device = this.#device; if (!device || !device.gatt) throw new Error("Device GATT is not available."); @@ -84,20 +132,34 @@ export class WebBluetoothTransport implements TrikiTransport { device.addEventListener("gattserverdisconnected", this.#onGattDisconnected); this.#gatt = await device.gatt.connect(); + this.#checkCancelled(); const svc = await this.#gatt.getPrimaryService(NUS_SERVICE); + this.#checkCancelled(); this.#rxChar = await svc.getCharacteristic(NUS_RX); + this.#checkCancelled(); this.#txChar = await svc.getCharacteristic(NUS_TX); + this.#checkCancelled(); try { this.#ctrlChar = await svc.getCharacteristic(NUS_CTRL); } catch { this.#ctrlChar = null; } + this.#checkCancelled(); await this.#txChar.startNotifications(); + this.#checkCancelled(); this.#txChar.addEventListener("characteristicvaluechanged", this.#onNotify); + this.#connected = true; void this.#startBattery(this.#gatt); // best-effort; never blocks streaming } catch (err) { + // Don't leave a half-open link behind a rejected connect. If the GATT drop + // fires #onGattDisconnected, #connected is still false, so no onDisconnect. + try { + if (this.#gatt?.connected) this.#gatt.disconnect(); + } catch { + /* best-effort */ + } this.#teardown(); throw err; } @@ -140,7 +202,7 @@ export class WebBluetoothTransport implements TrikiTransport { }; #emitBattery(view: DataView): void { - this.#batteryHandler?.(view.getUint8(0)); + this.#handlers?.onBattery(view.getUint8(0)); } #onNotify = (event: Event): void => { @@ -149,15 +211,19 @@ export class WebBluetoothTransport implements TrikiTransport { if (!view) return; // byteOffset/byteLength are load-bearing: the underlying buffer may be larger. const bytes = new Uint8Array(view.buffer, view.byteOffset, view.byteLength); - this.#frameHandler?.(bytes); + this.#handlers?.onFrame(bytes); }; #onGattDisconnected = (): void => { + // onDisconnect fires exactly once per successful connect — and never for a + // link that dropped before connect() resolved (the rejection is the signal). + const wasConnected = this.#connected; this.#teardown(); - this.#disconnectHandler?.(); + if (wasConnected) this.#handlers?.onDisconnect(); }; #teardown(): void { + this.#connected = false; if (this.#txChar) { try { this.#txChar.removeEventListener("characteristicvaluechanged", this.#onNotify); From 23024577dcb246eab54376e4c181864c87573aca Mon Sep 17 00:00:00 2001 From: Flop Butylkin Date: Sat, 4 Jul 2026 18:34:59 +0200 Subject: [PATCH 3/7] =?UTF-8?q?feat(triki-controller):=20production=20Nobl?= =?UTF-8?q?eTransport=20=E2=80=94=20cancellable=20connect,=20scan=20timeou?= =?UTF-8?q?t,=20true=20reconnect?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Node transport graduates from "working skeleton" to the full TrikiTransport contract: - Cancellable, time-bounded connect(): one budget (scanTimeoutMs, default 15 s, Infinity to disable) covers adapter power-on, the scan, and link setup; every exit path — match, timeout, cancel, startScanning failure — removes the discover listener and stops scanning, so retried connects never stack listeners on the module-level noble emitter. Rejections use named errors (ScanTimeoutError / ConnectAbortedError, exported from triki-controller/node). - disconnect() during a pending connect() cancels it (the promise rejects; a half-connected peripheral is dropped) — previously the abandoned scan could resurrect the session into "streaming" after an explicit disconnect. - Terminal adapter states ("unsupported"/"unauthorized") are checked synchronously: noble emits stateChange once per transition and never replays it, so waiting for an event that already happened hung connect() forever. - reconnect() re-links to the cached peripheral of the previous successful connect — no scan, so it can never latch onto a different token and silently mis-apply per-device calibration — and preflight("reconnect") throws the documented error (with zero events) when there is nothing to reconnect to. - NUS + Battery are discovered in ONE combined GATT round trip, and the battery read/subscribe is genuinely fire-and-forget with per-session staleness guards (the old code awaited it under a "never blocks streaming" comment). - onDisconnect fires exactly once per session, structurally: teardown removes the peripheral's disconnect listener before the handler runs. Fire-and-forget noble calls carry .catch(() => {}) so a rejection can't crash the process. - Battery UUIDs derive from protocol.ts via the exported nobleUuid() (which now shortens Bluetooth-base UUIDs to noble's 16-bit form) instead of hardcoded "180f"/"2a19"; scanServiceUuids lets callers opt into adapter-side filtering. New coverage: fakeNoble testkit (faithful to noble where it matters — one-shot stateChange, fresh Characteristic instances per discovery, async disconnect echo) + 27 lifecycle tests, all running with an injected mock so CI never needs the native module. Co-Authored-By: Claude Fable 5 --- packages/triki-controller/src/node.ts | 2 +- .../triki-controller/src/testkit/fakeNoble.ts | 266 +++++++++++ .../src/transports/noble.test.ts | 392 ++++++++++++++++ .../triki-controller/src/transports/noble.ts | 441 ++++++++++++++---- 4 files changed, 1017 insertions(+), 84 deletions(-) create mode 100644 packages/triki-controller/src/testkit/fakeNoble.ts create mode 100644 packages/triki-controller/src/transports/noble.test.ts diff --git a/packages/triki-controller/src/node.ts b/packages/triki-controller/src/node.ts index 5b541a5..bf36495 100644 --- a/packages/triki-controller/src/node.ts +++ b/packages/triki-controller/src/node.ts @@ -7,5 +7,5 @@ * never imports noble. */ export * from "./index"; -export { NobleTransport } from "./transports/noble"; +export { NobleTransport, ScanTimeoutError, ConnectAbortedError, nobleUuid } from "./transports/noble"; export type { NobleTransportOptions } from "./transports/noble"; diff --git a/packages/triki-controller/src/testkit/fakeNoble.ts b/packages/triki-controller/src/testkit/fakeNoble.ts new file mode 100644 index 0000000..a6efc0e --- /dev/null +++ b/packages/triki-controller/src/testkit/fakeNoble.ts @@ -0,0 +1,266 @@ +/** + * An in-memory mock of the @abandonware/noble surface {@link NobleTransport} uses, + * injected via its `noble` option. Faithful to the real module where it matters for + * the transport's correctness: + * - `stateChange` fires once per transition and is never replayed to late listeners; + * - every service discovery builds FRESH characteristic objects (so listeners on old + * instances die with them); + * - `disconnectAsync()` emits `disconnect` asynchronously (a microtask later). + */ +import { NUS_SERVICE, NUS_RX, NUS_TX, NUS_CTRL, BATTERY_SERVICE, BATTERY_LEVEL } from "../protocol"; +import { nobleUuid } from "../transports/noble"; + +const RX_ID = nobleUuid(NUS_RX); +const TX_ID = nobleUuid(NUS_TX); +const CTRL_ID = nobleUuid(NUS_CTRL); +const BATTERY_ID = nobleUuid(BATTERY_LEVEL); +const NUS_ID = nobleUuid(NUS_SERVICE); +const BATTERY_SVC_ID = nobleUuid(BATTERY_SERVICE); + +/** Microtask hop that survives vi.useFakeTimers (promises are never faked). */ +const microtask = (): Promise => Promise.resolve(); + +export class FakeNobleCharacteristic { + readonly uuid: string; + writes: { data: Uint8Array; withoutResponse: boolean }[] = []; + subscribeCount = 0; + readValue: Uint8Array; + /** Test hooks: gate/fail the next readAsync/subscribeAsync. */ + readGate: Promise | null = null; + readError: Error | null = null; + subscribeError: Error | null = null; + + #dataListeners = new Set<(data: Uint8Array, isNotification: boolean) => void>(); + + constructor(uuid: string, readValue: Uint8Array = new Uint8Array()) { + this.uuid = uuid; + this.readValue = readValue; + } + + async writeAsync(data: Uint8Array, withoutResponse: boolean): Promise { + this.writes.push({ data, withoutResponse }); + } + + async readAsync(): Promise { + if (this.readGate) await this.readGate; + if (this.readError) throw this.readError; + return this.readValue; + } + + async subscribeAsync(): Promise { + if (this.subscribeError) throw this.subscribeError; + this.subscribeCount++; + } + + on(event: "data", listener: (data: Uint8Array, isNotification: boolean) => void): this { + if (event === "data") this.#dataListeners.add(listener); + return this; + } + + // --- test drivers --- + emitData(bytes: Uint8Array): void { + for (const l of [...this.#dataListeners]) l(bytes, true); + } + + dataListenerCount(): number { + return this.#dataListeners.size; + } +} + +export interface FakePeripheralOptions { + id?: string; + name?: string; + address?: string; + led?: boolean; + /** Expose the standard Battery service. Default true. */ + battery?: boolean; + batteryLevel?: number; + /** Omit the NUS RX/TX characteristics entirely (a non-Triki device). */ + nus?: boolean; +} + +export class FakeNoblePeripheral { + readonly id: string; + readonly address: string; + readonly advertisement: { localName?: string }; + + connectCount = 0; + disconnectAsyncCount = 0; + discoverCalls: { serviceUuids: string[]; characteristicUuids: string[] }[] = []; + + /** Test hooks applied to the next operation / next discovery's fresh chars. */ + connectGate: Promise | null = null; + connectError: Error | null = null; + disconnectError: Error | null = null; + discoverError: Error | null = null; + txSubscribeError: Error | null = null; + batteryReadGate: Promise | null = null; + + /** Most recently discovered instances (fresh per discovery, like real noble). */ + rx: FakeNobleCharacteristic | null = null; + tx: FakeNobleCharacteristic | null = null; + ctrl: FakeNobleCharacteristic | null = null; + battery: FakeNobleCharacteristic | null = null; + + #led: boolean; + #hasBattery: boolean; + #batteryLevel: number; + #hasNus: boolean; + #disconnectListeners = new Set<() => void>(); + + constructor(options: FakePeripheralOptions = {}) { + this.id = options.id ?? "fake-id-1"; + this.address = options.address ?? "aa:bb:cc:dd:ee:ff"; + this.advertisement = { localName: options.name ?? "TRIKI 1234" }; + this.#led = options.led ?? true; + this.#hasBattery = options.battery ?? true; + this.#batteryLevel = options.batteryLevel ?? 87; + this.#hasNus = options.nus ?? true; + } + + async connectAsync(): Promise { + this.connectCount++; + if (this.connectGate) await this.connectGate; + if (this.connectError) throw this.connectError; + } + + async disconnectAsync(): Promise { + this.disconnectAsyncCount++; + if (this.disconnectError) throw this.disconnectError; + // Real noble delivers the 'disconnect' event asynchronously. + void microtask().then(() => this.emitDisconnect()); + } + + async discoverSomeServicesAndCharacteristicsAsync( + serviceUuids: string[], + characteristicUuids: string[], + ): Promise<{ characteristics: FakeNobleCharacteristic[] }> { + this.discoverCalls.push({ serviceUuids, characteristicUuids }); + if (this.discoverError) throw this.discoverError; + + // Fresh instances every round, mirroring noble's onCharacteristicsDiscover. + const found: FakeNobleCharacteristic[] = []; + const wants = (id: string): boolean => characteristicUuids.includes(id); + if (this.#hasNus && serviceUuids.includes(NUS_ID)) { + if (wants(RX_ID)) found.push((this.rx = new FakeNobleCharacteristic(RX_ID))); + if (wants(TX_ID)) { + this.tx = new FakeNobleCharacteristic(TX_ID); + this.tx.subscribeError = this.txSubscribeError; + found.push(this.tx); + } + if (this.#led && wants(CTRL_ID)) found.push((this.ctrl = new FakeNobleCharacteristic(CTRL_ID))); + } + if (this.#hasBattery && serviceUuids.includes(BATTERY_SVC_ID) && wants(BATTERY_ID)) { + this.battery = new FakeNobleCharacteristic(BATTERY_ID, Uint8Array.of(this.#batteryLevel)); + this.battery.readGate = this.batteryReadGate; + found.push(this.battery); + } + return { characteristics: found }; + } + + once(event: "disconnect", listener: () => void): this { + if (event === "disconnect") this.#disconnectListeners.add(listener); + return this; + } + + removeAllListeners(event?: string): this { + if (event === undefined || event === "disconnect") this.#disconnectListeners.clear(); + return this; + } + + // --- test drivers --- + emitDisconnect(): void { + const listeners = [...this.#disconnectListeners]; + this.#disconnectListeners.clear(); // once() semantics + for (const l of listeners) l(); + } + + disconnectListenerCount(): number { + return this.#disconnectListeners.size; + } +} + +export interface FakeNobleOptions extends FakePeripheralOptions { + /** Initial adapter state. Default "poweredOn". */ + state?: string; + /** Emit the peripheral as soon as scanning starts. Default true. */ + autoDiscover?: boolean; +} + +export class FakeNoble { + state: string; + scanning = false; + startScanCount = 0; + stopScanCount = 0; + startScanArgs: { serviceUuids: string[]; allowDuplicates: boolean }[] = []; + startScanError: Error | null = null; + stopScanError: Error | null = null; + /** When set, discovered as soon as scanning starts. */ + autoDiscoverPeripheral: FakeNoblePeripheral | null = null; + + #listeners = new Map void>>(); + + constructor(state = "poweredOn") { + this.state = state; + } + + on(event: string, listener: (...args: never[]) => void): this { + let set = this.#listeners.get(event); + if (!set) this.#listeners.set(event, (set = new Set())); + set.add(listener); + return this; + } + + removeListener(event: string, listener: (...args: never[]) => void): this { + this.#listeners.get(event)?.delete(listener); + return this; + } + + listenerCount(event: string): number { + return this.#listeners.get(event)?.size ?? 0; + } + + async startScanningAsync(serviceUuids: string[] = [], allowDuplicates = false): Promise { + if (this.startScanError) throw this.startScanError; + this.startScanCount++; + this.startScanArgs.push({ serviceUuids, allowDuplicates }); + this.scanning = true; + const peripheral = this.autoDiscoverPeripheral; + if (peripheral) void microtask().then(() => this.discover(peripheral)); + } + + async stopScanningAsync(): Promise { + this.stopScanCount++; + if (this.stopScanError) throw this.stopScanError; + this.scanning = false; + } + + // --- test drivers --- + + /** Emit a 'discover' event, like an advertisement arriving. */ + discover(peripheral: FakeNoblePeripheral): void { + const set = this.#listeners.get("discover"); + if (!set) return; + for (const l of [...set]) (l as (p: FakeNoblePeripheral) => void)(peripheral); + } + + /** Transition the adapter state, emitting stateChange ONCE (never replayed). */ + setState(state: string): void { + this.state = state; + const set = this.#listeners.get("stateChange"); + if (!set) return; + for (const l of [...set]) (l as (s: string) => void)(state); + } +} + +export interface FakeNobleHandles { + noble: FakeNoble; + peripheral: FakeNoblePeripheral; +} + +export function makeFakeNoble(options: FakeNobleOptions = {}): FakeNobleHandles { + const noble = new FakeNoble(options.state ?? "poweredOn"); + const peripheral = new FakeNoblePeripheral(options); + if (options.autoDiscover ?? true) noble.autoDiscoverPeripheral = peripheral; + return { noble, peripheral }; +} diff --git a/packages/triki-controller/src/transports/noble.test.ts b/packages/triki-controller/src/transports/noble.test.ts new file mode 100644 index 0000000..1244e24 --- /dev/null +++ b/packages/triki-controller/src/transports/noble.test.ts @@ -0,0 +1,392 @@ +/** + * NobleTransport lifecycle tests against an injected mock noble (no native module, + * no radio). The very fact these pass in CI without `@abandonware/noble` installed + * proves the injected module is used and no dynamic import is attempted. + */ +import { describe, test, expect, vi, afterEach } from "vitest"; +import { NobleTransport, ScanTimeoutError, ConnectAbortedError, nobleUuid } from "./noble"; +import { NUS_SERVICE, NUS_RX, BATTERY_SERVICE, BATTERY_LEVEL, startCmd } from "../protocol"; +import { makeFakeNoble, FakeNoblePeripheral } from "../testkit/fakeNoble"; +import type { FakeNobleOptions } from "../testkit/fakeNoble"; +import { deferred } from "../testkit/fakeTransport"; +import type { NobleTransportOptions } from "./noble"; + +/** Let pending microtasks and 0ms timers settle (real timers only). */ +const flush = (): Promise => new Promise((r) => setTimeout(r, 0)); + +interface Recorded { + frames: Uint8Array[]; + disconnects: number; + batteries: number[]; +} + +function rig( + fakeOptions?: FakeNobleOptions, + transportOptions?: Omit, +): { + transport: NobleTransport; + noble: ReturnType["noble"]; + peripheral: FakeNoblePeripheral; + events: Recorded; +} { + const { noble, peripheral } = makeFakeNoble(fakeOptions); + const transport = new NobleTransport({ noble, ...transportOptions }); + const events: Recorded = { frames: [], disconnects: 0, batteries: [] }; + transport.attach({ + onFrame: (b) => events.frames.push(b), + onDisconnect: () => events.disconnects++, + onBattery: (p) => events.batteries.push(p), + }); + return { transport, noble, peripheral, events }; +} + +afterEach(() => { + vi.useRealTimers(); +}); + +describe("nobleUuid", () => { + test("shortens Bluetooth-base UUIDs to 16-bit form, strips dashes otherwise", () => { + expect(nobleUuid(BATTERY_SERVICE)).toBe("180f"); + expect(nobleUuid(BATTERY_LEVEL)).toBe("2a19"); + expect(nobleUuid(NUS_SERVICE)).toBe("6e400001b5a3f393e0a9e50e24dcca9e"); + expect(nobleUuid("0000180F-0000-1000-8000-00805F9B34FB")).toBe("180f"); + }); +}); + +describe("happy path", () => { + test("scan -> connect -> one combined discovery -> subscribe -> frames flow", async () => { + const { transport, noble, peripheral, events } = rig(); + + await transport.connect(); + + expect(noble.startScanArgs).toEqual([{ serviceUuids: [], allowDuplicates: false }]); + expect(noble.stopScanCount).toBe(1); + expect(peripheral.connectCount).toBe(1); + // NUS + Battery discovered in ONE GATT round trip. + expect(peripheral.discoverCalls).toHaveLength(1); + expect(peripheral.discoverCalls[0]!.serviceUuids).toEqual([ + nobleUuid(NUS_SERVICE), + nobleUuid(BATTERY_SERVICE), + ]); + expect(peripheral.discoverCalls[0]!.characteristicUuids).toHaveLength(4); + expect(peripheral.tx!.subscribeCount).toBe(1); + expect(transport.hasLed).toBe(true); + + peripheral.tx!.emitData(Uint8Array.of(1, 2, 3)); + expect(events.frames.map((f) => Array.from(f))).toEqual([[1, 2, 3]]); + + await transport.writeRx(startCmd(104), true); + expect(peripheral.rx!.writes).toHaveLength(1); + expect(peripheral.rx!.writes[0]!.withoutResponse).toBe(true); + + await transport.writeCtrl(Uint8Array.of(0x01)); + expect(peripheral.ctrl!.writes.map((w) => Array.from(w.data))).toEqual([[0x01]]); + }); + + test("battery is read and subscribed after connect, without blocking it", async () => { + const { transport, peripheral, events } = rig({ batteryLevel: 61 }); + const gate = deferred(); + peripheral.batteryReadGate = gate.promise; + + await transport.connect(); // resolves while the battery read is still gated + expect(events.batteries).toEqual([]); + + gate.resolve(); + await flush(); + expect(events.batteries).toEqual([61]); + expect(peripheral.battery!.subscribeCount).toBe(1); + + peripheral.battery!.emitData(Uint8Array.of(42)); + expect(events.batteries).toEqual([61, 42]); + }); + + test("a stale battery read after disconnect never reaches the handler", async () => { + const { transport, peripheral, events } = rig(); + const gate = deferred(); + peripheral.batteryReadGate = gate.promise; + + await transport.connect(); + transport.disconnect(); + gate.resolve(); + await flush(); + + expect(events.batteries).toEqual([]); + expect(peripheral.battery!.subscribeCount).toBe(0); + }); + + test("scanServiceUuids are normalized and forwarded to startScanning", async () => { + const { transport, noble } = rig(undefined, { scanServiceUuids: [NUS_SERVICE] }); + await transport.connect(); + expect(noble.startScanArgs[0]!.serviceUuids).toEqual([nobleUuid(NUS_SERVICE)]); + }); +}); + +describe("adapter state", () => { + test.each(["unsupported", "unauthorized"])( + "an already-terminal state (%s) rejects immediately, no stateChange wait", + async (state) => { + const { transport, noble, peripheral } = rig({ state }); + await expect(transport.connect()).rejects.toThrow(`Bluetooth adapter is ${state}.`); + expect(noble.listenerCount("stateChange")).toBe(0); + expect(noble.startScanCount).toBe(0); + expect(peripheral.connectCount).toBe(0); + }, + ); + + test("poweredOff -> poweredOn proceeds to a normal connect", async () => { + const { transport, noble, peripheral } = rig({ state: "poweredOff" }); + const pending = transport.connect(); + await flush(); + expect(noble.startScanCount).toBe(0); // still waiting for the adapter + + noble.setState("poweredOn"); + await pending; + expect(peripheral.connectCount).toBe(1); + expect(noble.listenerCount("stateChange")).toBe(0); + }); + + test("an adapter stuck in poweredOff times out with the adapter message", async () => { + vi.useFakeTimers(); + const { transport, noble } = rig({ state: "poweredOff" }); + const pending = transport.connect(); + pending.catch(() => {}); // observed via expect below; avoid an unhandled gap + + await vi.advanceTimersByTimeAsync(15_000); + await expect(pending).rejects.toThrow(ScanTimeoutError); + await expect(pending).rejects.toThrow(/adapter did not power on within 15000ms/); + expect(noble.listenerCount("stateChange")).toBe(0); + }); +}); + +describe("scan lifecycle", () => { + test("scan timeout rejects, removes the listener, stops scanning", async () => { + vi.useFakeTimers(); + const { transport, noble, peripheral } = rig({ autoDiscover: false }); + const pending = transport.connect(); + pending.catch(() => {}); + await vi.advanceTimersByTimeAsync(1); // let the scan start + expect(noble.listenerCount("discover")).toBe(1); + + await vi.advanceTimersByTimeAsync(15_000); + await expect(pending).rejects.toThrow(ScanTimeoutError); + await expect(pending).rejects.toThrow(/name prefix "TRIKI".*15000ms/); + expect(noble.listenerCount("discover")).toBe(0); + expect(noble.stopScanCount).toBe(1); + + // A token appearing later must not resurrect the dead attempt. + noble.discover(peripheral); + expect(peripheral.connectCount).toBe(0); + }); + + test("a custom scanTimeoutMs is honored; invalid values fall back to the default", async () => { + vi.useFakeTimers(); + const { transport } = rig({ autoDiscover: false }, { scanTimeoutMs: 500 }); + const pending = transport.connect(); + pending.catch(() => {}); + await vi.advanceTimersByTimeAsync(500); + await expect(pending).rejects.toThrow(/500ms/); + + const bad = rig({ autoDiscover: false }, { scanTimeoutMs: -1 }); + const badPending = bad.transport.connect(); + badPending.catch(() => {}); + await vi.advanceTimersByTimeAsync(15_000); + await expect(badPending).rejects.toThrow(/15000ms/); + }); + + test("disconnect() during the scan cancels: no zombie session, no listener stacking", async () => { + const { transport, noble, peripheral, events } = rig({ autoDiscover: false }); + const pending = transport.connect(); + await flush(); // scan armed + + transport.disconnect(); + await expect(pending).rejects.toThrow(ConnectAbortedError); + expect(noble.listenerCount("discover")).toBe(0); + expect(noble.stopScanCount).toBe(1); + + // A token appearing later must not connect or flip anything to streaming. + noble.discover(peripheral); + await flush(); + expect(peripheral.connectCount).toBe(0); + expect(events.disconnects).toBe(0); // no connect succeeded -> no onDisconnect + + // A retried connect arms exactly one discover listener (no stacking). + const retry = transport.connect(); + await flush(); + expect(noble.listenerCount("discover")).toBe(1); + transport.disconnect(); + await expect(retry).rejects.toThrow(ConnectAbortedError); + expect(noble.listenerCount("discover")).toBe(0); + }); + + test("a startScanningAsync failure rejects and leaves no listener behind", async () => { + const { transport, noble } = rig({ autoDiscover: false }); + noble.startScanError = new Error("scan boom"); + + await expect(transport.connect()).rejects.toThrow("scan boom"); + expect(noble.listenerCount("discover")).toBe(0); + }); + + test("non-matching advertisements are ignored; address matching is case-insensitive", async () => { + const { transport, noble, peripheral } = rig( + { autoDiscover: false, address: "aa:bb:cc:dd:ee:01" }, + { address: "AA:BB:CC:DD:EE:01" }, + ); + const pending = transport.connect(); + await flush(); + + noble.discover(new FakeNoblePeripheral({ name: "SOMETHING ELSE", address: "11:22:33:44:55:66" })); + await flush(); + expect(peripheral.connectCount).toBe(0); + + noble.discover(peripheral); + await pending; + expect(peripheral.connectCount).toBe(1); + }); +}); + +describe("disconnect semantics", () => { + test("onDisconnect fires exactly once for a remote drop, even if it double-fires", async () => { + const { transport, peripheral, events } = rig(); + await transport.connect(); + + peripheral.emitDisconnect(); + peripheral.emitDisconnect(); + expect(events.disconnects).toBe(1); + expect(transport.hasLed).toBe(false); // session state was torn down + }); + + test("explicit disconnect() fires onDisconnect once, ignoring noble's async echo", async () => { + const { transport, peripheral, events } = rig(); + await transport.connect(); + + transport.disconnect(); + expect(events.disconnects).toBe(1); + expect(peripheral.disconnectAsyncCount).toBe(1); + + await flush(); // noble's own 'disconnect' event lands a microtask later + expect(events.disconnects).toBe(1); + }); + + test("disconnect() when never connected is a silent no-op", async () => { + const { transport, events } = rig(); + transport.disconnect(); + transport.disconnect(); + expect(events.disconnects).toBe(0); + }); + + test("rejections from noble's disconnect/stopScanning are swallowed, not crashes", async () => { + const { transport, noble, peripheral } = rig(); + noble.stopScanError = new Error("stop boom"); + peripheral.disconnectError = new Error("hci gone"); + + await transport.connect(); // stopScanning failure on the match path is absorbed + transport.disconnect(); // disconnectAsync rejection is absorbed + await flush(); // an unhandled rejection here would fail the test run + }); +}); + +describe("connect failure cleanup", () => { + test("a TX subscribe failure closes the half-open link without onDisconnect", async () => { + const { transport, peripheral, events } = rig(); + peripheral.txSubscribeError = new Error("subscribe boom"); + + await expect(transport.connect()).rejects.toThrow("subscribe boom"); + expect(peripheral.disconnectAsyncCount).toBe(1); + expect(events.disconnects).toBe(0); // rejection is the signal, not onDisconnect + }); + + test("a device without NUS RX/TX rejects with a clear error and closes the link", async () => { + const { transport, peripheral, events } = rig({ nus: false }); + + await expect(transport.connect()).rejects.toThrow("NUS RX/TX characteristics not found"); + expect(peripheral.disconnectAsyncCount).toBe(1); + expect(events.disconnects).toBe(0); + }); + + test("writeRx/writeCtrl reject when not connected", async () => { + const { transport } = rig(); + await expect(transport.writeRx(Uint8Array.of(1))).rejects.toThrow("Not connected."); + await expect(transport.writeCtrl(Uint8Array.of(1))).rejects.toThrow( + "LED control characteristic is not available.", + ); + }); + + test("hasLed reflects the control characteristic's presence", async () => { + const withLed = rig({ led: true }); + await withLed.transport.connect(); + expect(withLed.transport.hasLed).toBe(true); + + const withoutLed = rig({ led: false }); + await withoutLed.transport.connect(); + expect(withoutLed.transport.hasLed).toBe(false); + await expect(withoutLed.transport.writeCtrl(Uint8Array.of(1))).rejects.toThrow(); + }); +}); + +describe("reconnect", () => { + test("re-links to the cached peripheral with no scan, and frames arrive once", async () => { + const { transport, noble, peripheral, events } = rig(); + await transport.connect(); + peripheral.emitDisconnect(); + expect(events.disconnects).toBe(1); + + await transport.reconnect(); + expect(noble.startScanCount).toBe(1); // NO second scan + expect(peripheral.connectCount).toBe(2); + expect(peripheral.discoverCalls).toHaveLength(2); // fresh discovery per session + + // Exactly one frame per notification — old characteristic listeners are dead. + peripheral.tx!.emitData(Uint8Array.of(9)); + expect(events.frames).toHaveLength(1); + }); + + test("reconnect() before any connect() throws without touching noble", async () => { + const { transport, noble, peripheral } = rig(); + await expect(transport.reconnect()).rejects.toThrow( + "No device to reconnect to; call connect() first.", + ); + expect(() => transport.preflight("reconnect")).toThrow("No device to reconnect to"); + expect(noble.startScanCount).toBe(0); + expect(peripheral.connectCount).toBe(0); + }); + + test("the device stays reconnectable after a failed first session setup", async () => { + const { transport, peripheral } = rig(); + peripheral.txSubscribeError = new Error("subscribe boom"); + await expect(transport.connect()).rejects.toThrow("subscribe boom"); + + // The scan matched, so the peripheral is remembered; a repaired link reconnects. + peripheral.txSubscribeError = null; + await transport.reconnect(); + expect(peripheral.connectCount).toBe(2); + }); +}); + +describe("ownership and concurrency guards", () => { + test("attach() twice throws; detach() releases", () => { + const { noble } = makeFakeNoble(); + const transport = new NobleTransport({ noble }); + const handlers = { onFrame: () => {}, onDisconnect: () => {}, onBattery: () => {} }; + transport.attach(handlers); + expect(() => transport.attach(handlers)).toThrow("already attached"); + transport.detach(); + expect(() => transport.attach(handlers)).not.toThrow(); + }); + + test("overlapping connect() attempts are rejected", async () => { + const { transport } = rig({ autoDiscover: false }); + const first = transport.connect(); + first.catch(() => {}); + await flush(); + + await expect(transport.connect()).rejects.toThrow("already in progress"); + + transport.disconnect(); + await expect(first).rejects.toThrow(ConnectAbortedError); + + // And once streaming, another connect() is refused too. + const { transport: t2 } = rig(); + await t2.connect(); + await expect(t2.connect()).rejects.toThrow("Already connected."); + }); +}); diff --git a/packages/triki-controller/src/transports/noble.ts b/packages/triki-controller/src/transports/noble.ts index 79a988c..9923f31 100644 --- a/packages/triki-controller/src/transports/noble.ts +++ b/packages/triki-controller/src/transports/noble.ts @@ -1,10 +1,20 @@ /** * Node implementation of {@link TrikiTransport}, backed by `@abandonware/noble`. * - * This is a working skeleton: it scans for a TRIKI token by advertised name (or a - * fixed address), connects, discovers the NUS characteristics, and streams TX - * notifications back to the {@link TrikiController}. It is intentionally minimal — see - * the `TODO`s for scan timeouts and richer error handling before production use. + * Lifecycle guarantees (the {@link TrikiTransport} contract): + * - `connect()` scans for a TRIKI token by advertised name (or a fixed address), + * connects, discovers the NUS + Battery characteristics in one GATT round trip, and + * subscribes to TX notifications. The whole attempt shares one time budget + * ({@link NobleTransportOptions.scanTimeoutMs}, default 15 s) and rejects with + * {@link ScanTimeoutError} when it runs out. + * - `disconnect()` is idempotent: it cancels a pending `connect()` (which then rejects + * with {@link ConnectAbortedError}), tears down an active session (firing + * `onDisconnect` exactly once), and is a no-op otherwise. + * - `reconnect()` re-links to the exact peripheral of the previous successful + * `connect()` — no scan, so it can never latch onto a different token — and throws + * when there is nothing to reconnect to. + * - The battery read/subscribe is fire-and-forget: it never delays `connect()` or the + * controller's START command. * * `@abandonware/noble` is an **optional peer dependency**: it is imported lazily and * only required when {@link NobleTransport.connect} runs, so importing @@ -15,8 +25,8 @@ * npm install @abandonware/noble * ``` */ -import { NUS_SERVICE, NUS_RX, NUS_TX, NUS_CTRL } from "../protocol"; -import type { TrikiTransport } from "../transport"; +import { NUS_SERVICE, NUS_RX, NUS_TX, NUS_CTRL, BATTERY_SERVICE, BATTERY_LEVEL, DEVICE_NAME_PREFIX } from "../protocol"; +import type { TrikiTransport, TrikiTransportHandlers } from "../transport"; // Node's Buffer, declared locally so this file needs no `@types/node` (noble's writes // want a Buffer; it is a Uint8Array subclass at runtime). @@ -24,11 +34,31 @@ declare const Buffer: { from(buffer: ArrayBufferLike, byteOffset?: number, length?: number): Uint8Array; }; +/** The connect()/scan budget ran out before a device was streaming. */ +export class ScanTimeoutError extends Error { + constructor(message: string) { + super(message); + this.name = "ScanTimeoutError"; + } +} + +/** disconnect() was called while a connect()/reconnect() was still pending. */ +export class ConnectAbortedError extends Error { + constructor(message: string) { + super(message); + this.name = "ConnectAbortedError"; + } +} + +/** Default budget for adapter power-on + scan + link setup, in milliseconds. */ +const DEFAULT_SCAN_TIMEOUT_MS = 15_000; + /** Options for {@link NobleTransport}. */ export interface NobleTransportOptions { /** - * Match devices whose advertised name starts with this prefix. Default `"TRIKI"` - * (the token advertises as `TRIKI `). Ignored when {@link address} is set. + * Match devices whose advertised name starts with this prefix (case-insensitive). + * Default `"TRIKI"` (the token advertises as `TRIKI `). Ignored when + * {@link address} is set. */ namePrefix?: string; /** Match a specific device by BLE address (case-insensitive) instead of by name. */ @@ -39,6 +69,20 @@ export interface NobleTransportOptions { * default fork won't build on your Node version, or a mock for tests. */ noble?: unknown; + /** + * Time budget in milliseconds for the whole connect attempt: adapter power-on, the + * scan, and link setup (reconnect shares the same budget). Default 15 000. Pass + * `Infinity` to wait forever (cancel via `disconnect()`). + */ + scanTimeoutMs?: number; + /** + * Advertised-service filter passed to noble's `startScanningAsync`. Default `[]` + * (scan everything): it is unverified whether Triki tokens include the 128-bit NUS + * UUID in their advertisement (the `TRIKI ` name may crowd it out of the + * ADV packet), so filtering by default could find nothing. Pass `[NUS_SERVICE]` to + * opt in; UUIDs are normalized via {@link nobleUuid}. + */ + scanServiceUuids?: string[]; } // --- minimal slice of the @abandonware/noble surface we use ---------------------- @@ -54,6 +98,9 @@ interface NobleCharacteristic { } interface NoblePeripheral { + /** Stable device id. noble caches Peripheral objects by id, and a cached peripheral + * supports `connectAsync()` again after `disconnectAsync()` — reconnect relies on it. */ + readonly id: string; readonly address: string; readonly advertisement: { localName?: string }; connectAsync(): Promise; @@ -67,6 +114,9 @@ interface NoblePeripheral { } interface Noble { + /** May already be terminal ('unsupported'/'unauthorized') before any listener + * attaches; 'stateChange' fires once per transition and never replays, so always + * check `state` synchronously first. */ readonly state: string; on(event: "stateChange", listener: (state: string) => void): this; on(event: "discover", listener: (peripheral: NoblePeripheral) => void): this; @@ -75,15 +125,24 @@ interface Noble { stopScanningAsync(): Promise; } -/** noble matches UUIDs as lowercase hex without dashes. */ -const nobleUuid = (uuid: string): string => uuid.replace(/-/g, "").toLowerCase(); +const BLUETOOTH_BASE_SUFFIX = "-0000-1000-8000-00805f9b34fb"; + +/** + * Convert a canonical UUID to noble's representation: 16-bit-assigned UUIDs on the + * Bluetooth base are reported short ("180f"), everything else lowercase and dashless. + */ +export const nobleUuid = (uuid: string): string => { + const u = uuid.toLowerCase(); + if (u.startsWith("0000") && u.endsWith(BLUETOOTH_BASE_SUFFIX)) return u.slice(4, 8); + return u.replace(/-/g, ""); +}; + const NUS_SERVICE_ID = nobleUuid(NUS_SERVICE); const NUS_RX_ID = nobleUuid(NUS_RX); const NUS_TX_ID = nobleUuid(NUS_TX); const NUS_CTRL_ID = nobleUuid(NUS_CTRL); -/** Standard Battery Service (0x180F) and Battery Level characteristic (0x2A19). */ -const BATTERY_SERVICE_ID = "180f"; -const BATTERY_LEVEL_ID = "2a19"; +const BATTERY_SERVICE_ID = nobleUuid(BATTERY_SERVICE); +const BATTERY_LEVEL_ID = nobleUuid(BATTERY_LEVEL); async function loadNoble(): Promise { try { @@ -103,95 +162,100 @@ async function loadNoble(): Promise { } } +/** Bookkeeping for one in-flight connect()/reconnect(). */ +interface ConnectAttempt { + cancelled: boolean; + /** Rejecter of whichever cancellable wait is pending; disconnect() invokes it. */ + fail: ((err: Error) => void) | null; +} + export class NobleTransport implements TrikiTransport { - #namePrefix: string; + #namePrefixLower: string; #address: string | null; #injectedNoble: Noble | null; + #scanTimeoutMs: number; + #scanServiceUuids: string[]; + + #noble: Noble | null = null; // lazily-loaded module, cached across sessions + #handlers: TrikiTransportHandlers | null = null; + #attempt: ConnectAttempt | null = null; + /** True only between connect()/reconnect() resolving and the next teardown. */ + #sessionActive = false; #peripheral: NoblePeripheral | null = null; #rxChar: NobleCharacteristic | null = null; #txChar: NobleCharacteristic | null = null; #ctrlChar: NobleCharacteristic | null = null; - #frameHandler: ((bytes: Uint8Array) => void) | null = null; - #disconnectHandler: (() => void) | null = null; - #batteryHandler: ((percent: number) => void) | null = null; + /** Retained across teardown so reconnect() can re-link without a scan. */ + #lastPeripheral: NoblePeripheral | null = null; constructor(options: NobleTransportOptions = {}) { - this.#namePrefix = options.namePrefix ?? "TRIKI"; + this.#namePrefixLower = (options.namePrefix ?? DEVICE_NAME_PREFIX).toLowerCase(); this.#address = options.address ? options.address.toLowerCase() : null; this.#injectedNoble = (options.noble as Noble | undefined) ?? null; + const t = options.scanTimeoutMs; + this.#scanTimeoutMs = + typeof t === "number" && ((Number.isFinite(t) && t > 0) || t === Infinity) + ? t + : DEFAULT_SCAN_TIMEOUT_MS; + this.#scanServiceUuids = (options.scanServiceUuids ?? []).map(nobleUuid); } get hasLed(): boolean { return this.#ctrlChar !== null; } - onFrame(handler: (bytes: Uint8Array) => void): void { - this.#frameHandler = handler; + attach(handlers: TrikiTransportHandlers): void { + if (this.#handlers) throw new Error("NobleTransport is already attached."); + this.#handlers = handlers; } - onDisconnect(handler: () => void): void { - this.#disconnectHandler = handler; + detach(): void { + this.#handlers = null; } - onBattery(handler: (percent: number) => void): void { - this.#batteryHandler = handler; + /** Synchronous pre-flight: reconnect needs a remembered device. */ + preflight(op: "connect" | "reconnect"): void { + if (op === "reconnect" && !this.#lastPeripheral) { + throw new Error("No device to reconnect to; call connect() first."); + } } async connect(): Promise { - const noble = this.#injectedNoble ?? (await loadNoble()); + const { noble, attempt, deadline } = await this.#beginAttempt(); try { - await this.#waitPoweredOn(noble); - const peripheral = await this.#scan(noble); - this.#peripheral = peripheral; - peripheral.once("disconnect", this.#onDisconnected); - await peripheral.connectAsync(); - - const { characteristics } = await peripheral.discoverSomeServicesAndCharacteristicsAsync( - [NUS_SERVICE_ID], - [NUS_RX_ID, NUS_TX_ID, NUS_CTRL_ID], - ); - this.#rxChar = characteristics.find((c) => c.uuid === NUS_RX_ID) ?? null; - this.#txChar = characteristics.find((c) => c.uuid === NUS_TX_ID) ?? null; - this.#ctrlChar = characteristics.find((c) => c.uuid === NUS_CTRL_ID) ?? null; - if (!this.#rxChar || !this.#txChar) { - throw new Error("Triki NUS RX/TX characteristics not found on the device."); - } - - this.#txChar.on("data", (data) => this.#frameHandler?.(data)); - await this.#txChar.subscribeAsync(); - - await this.#startBattery(peripheral); // best-effort; never blocks streaming + await this.#waitPoweredOn(noble, attempt, deadline); + this.#checkCancelled(attempt); + const peripheral = await this.#scan(noble, attempt, deadline); + this.#checkCancelled(attempt); + // Cached as soon as the scan matches, so reconnect() works even when the rest + // of this attempt fails (mirrors the web transport retaining its #device). + this.#lastPeripheral = peripheral; + await this.#openSession(peripheral, attempt, deadline); } catch (err) { - this.#teardown(); + this.#abortAttempt(); throw err; + } finally { + this.#attempt = null; } } - /** Read the Battery service once and subscribe to updates, if the device exposes it. */ - async #startBattery(peripheral: NoblePeripheral): Promise { + /** Re-link to the peripheral of the previous successful connect() — no scan. */ + async reconnect(): Promise { + this.preflight("reconnect"); + const peripheral = this.#lastPeripheral!; + const { noble, attempt, deadline } = await this.#beginAttempt(); try { - const { characteristics } = await peripheral.discoverSomeServicesAndCharacteristicsAsync( - [BATTERY_SERVICE_ID], - [BATTERY_LEVEL_ID], - ); - const batt = characteristics.find((c) => c.uuid === BATTERY_LEVEL_ID); - if (!batt) return; - const value = await batt.readAsync(); - if (value.length) this.#batteryHandler?.(value[0]!); - batt.on("data", (data) => { - if (data.length) this.#batteryHandler?.(data[0]!); - }); - await batt.subscribeAsync(); - } catch { - /* no Battery service, or notifications unsupported — the read (if any) stands */ + await this.#waitPoweredOn(noble, attempt, deadline); + this.#checkCancelled(attempt); + await this.#openSession(peripheral, attempt, deadline); + } catch (err) { + this.#abortAttempt(); + throw err; + } finally { + this.#attempt = null; } } - /** Re-scan and connect again (skeleton: same as a fresh connect). */ - async reconnect(): Promise { - await this.connect(); - } - async writeRx(data: Uint8Array, withoutResponse = false): Promise { if (!this.#rxChar) throw new Error("Not connected."); await this.#rxChar.writeAsync(toBuffer(data), withoutResponse); @@ -203,60 +267,271 @@ export class NobleTransport implements TrikiTransport { } disconnect(): void { - const peripheral = this.#peripheral; - if (peripheral) void peripheral.disconnectAsync(); - else this.#onDisconnected(); + const attempt = this.#attempt; + if (attempt) { + // Cancel the pending connect: reject the cancellable wait (scan / power-on) or + // let the next checkpoint throw; drop a half-connected peripheral so pending + // GATT awaits settle. No onDisconnect — the connect() rejection is the signal. + attempt.cancelled = true; + attempt.fail?.(new ConnectAbortedError("Disconnected while connecting.")); + const peripheral = this.#peripheral; + if (peripheral) void peripheral.disconnectAsync().catch(() => {}); + return; + } + if (!this.#sessionActive) return; // idempotent; never connected -> no onDisconnect + const peripheral = this.#peripheral!; + this.#finishSession(); + void peripheral.disconnectAsync().catch(() => {}); } // --- internal ---------------------------------------------------------------- - #waitPoweredOn(noble: Noble): Promise { + /** Shared connect/reconnect entry: guards, module load, attempt bookkeeping. */ + async #beginAttempt(): Promise<{ noble: Noble; attempt: ConnectAttempt; deadline: number }> { + if (this.#attempt) throw new Error("connect() is already in progress."); + if (this.#sessionActive) throw new Error("Already connected."); + const attempt: ConnectAttempt = { cancelled: false, fail: null }; + this.#attempt = attempt; + const deadline = Number.isFinite(this.#scanTimeoutMs) + ? Date.now() + this.#scanTimeoutMs + : Infinity; + try { + const noble = this.#injectedNoble ?? (this.#noble ??= await loadNoble()); + this.#checkCancelled(attempt); + return { noble, attempt, deadline }; + } catch (err) { + this.#attempt = null; + throw err; + } + } + + #checkCancelled(attempt: ConnectAttempt): void { + if (attempt.cancelled) throw new ConnectAbortedError("Disconnected while connecting."); + } + + /** Failure path of an attempt: drop any half-open link, reset session state. */ + #abortAttempt(): void { + const peripheral = this.#peripheral; + if (peripheral) void peripheral.disconnectAsync().catch(() => {}); + this.#teardown(); + } + + /** + * Resolve when the adapter is powered on. Terminal states are checked + * synchronously — noble emits `stateChange` once per transition and never replays + * it, so waiting for an event that already happened would hang forever. + */ + #waitPoweredOn(noble: Noble, attempt: ConnectAttempt, deadline: number): Promise { if (noble.state === "poweredOn") return Promise.resolve(); + if (noble.state === "unsupported" || noble.state === "unauthorized") { + return Promise.reject(new Error(`Bluetooth adapter is ${noble.state}.`)); + } return new Promise((resolve, reject) => { + let timer: ReturnType | undefined; + const cleanup = (): void => { + noble.removeListener("stateChange", onState); + if (timer !== undefined) clearTimeout(timer); + attempt.fail = null; + }; const onState = (state: string): void => { if (state === "poweredOn") { - noble.removeListener("stateChange", onState); + cleanup(); resolve(); } else if (state === "unsupported" || state === "unauthorized") { - noble.removeListener("stateChange", onState); + cleanup(); reject(new Error(`Bluetooth adapter is ${state}.`)); } }; + attempt.fail = (err) => { + cleanup(); + reject(err); + }; + const remaining = deadline - Date.now(); + if (Number.isFinite(remaining)) { + timer = setTimeout(() => { + cleanup(); + reject( + new ScanTimeoutError( + `Bluetooth adapter did not power on within ${this.#scanTimeoutMs}ms ` + + `(state: ${noble.state}).`, + ), + ); + }, Math.max(0, remaining)); + } noble.on("stateChange", onState); }); } - #scan(noble: Noble): Promise { + /** + * Scan until a matching peripheral appears. Every exit — match, timeout, cancel, + * or a startScanning failure — removes the `discover` listener and stops scanning, + * so retried connects never stack listeners on the (module-level) noble emitter. + */ + #scan(noble: Noble, attempt: ConnectAttempt, deadline: number): Promise { return new Promise((resolve, reject) => { + let timer: ReturnType | undefined; + let settled = false; + const cleanup = (): void => { + settled = true; + noble.removeListener("discover", onDiscover); + if (timer !== undefined) clearTimeout(timer); + attempt.fail = null; + void noble.stopScanningAsync().catch(() => {}); + }; const onDiscover = (peripheral: NoblePeripheral): void => { if (!this.#matches(peripheral)) return; - noble.removeListener("discover", onDiscover); - void noble.stopScanningAsync(); + cleanup(); resolve(peripheral); }; + attempt.fail = (err) => { + cleanup(); + reject(err); + }; + const remaining = deadline - Date.now(); + if (Number.isFinite(remaining)) { + timer = setTimeout(() => { + cleanup(); + const target = this.#address + ? `address "${this.#address}"` + : `name prefix "${this.#namePrefixLower.toUpperCase()}"`; + reject( + new ScanTimeoutError( + `No device matching ${target} found within ${this.#scanTimeoutMs}ms of scanning.`, + ), + ); + }, Math.max(0, remaining)); + } noble.on("discover", onDiscover); - // TODO: add a scan timeout that removes the listener and rejects. - noble.startScanningAsync([], false).catch(reject); + noble.startScanningAsync(this.#scanServiceUuids, false).catch((err) => { + if (!settled) { + cleanup(); + reject(err); + } + }); + }); + } + + /** Connect + discover + subscribe on a known peripheral. Shared by (re)connect. */ + async #openSession( + peripheral: NoblePeripheral, + attempt: ConnectAttempt, + deadline: number, + ): Promise { + this.#peripheral = peripheral; + peripheral.once("disconnect", this.#onPeripheralDisconnect); + + await this.#race(peripheral.connectAsync(), attempt, deadline, "Connecting to the device"); + this.#checkCancelled(attempt); + + // One combined GATT discovery for NUS + Battery: a second discovery procedure + // would double the connect-time round trips. Battery absence just means find() + // comes back empty. + const { characteristics } = await this.#race( + peripheral.discoverSomeServicesAndCharacteristicsAsync( + [NUS_SERVICE_ID, BATTERY_SERVICE_ID], + [NUS_RX_ID, NUS_TX_ID, NUS_CTRL_ID, BATTERY_LEVEL_ID], + ), + attempt, + deadline, + "Service discovery", + ); + this.#checkCancelled(attempt); + this.#rxChar = characteristics.find((c) => c.uuid === NUS_RX_ID) ?? null; + this.#txChar = characteristics.find((c) => c.uuid === NUS_TX_ID) ?? null; + this.#ctrlChar = characteristics.find((c) => c.uuid === NUS_CTRL_ID) ?? null; + const battChar = characteristics.find((c) => c.uuid === BATTERY_LEVEL_ID) ?? null; + if (!this.#rxChar || !this.#txChar) { + throw new Error("Triki NUS RX/TX characteristics not found on the device."); + } + + this.#txChar.on("data", (data) => this.#handlers?.onFrame(data)); + await this.#race(this.#txChar.subscribeAsync(), attempt, deadline, "TX subscription"); + this.#checkCancelled(attempt); + + this.#sessionActive = true; + // Fire-and-forget: battery must never delay connect() or the START command. + void this.#startBattery(battChar, peripheral); + } + + /** + * Read the battery level once and subscribe to updates. Best-effort and detached + * from connect(); the per-session guards keep a stale read from leaking into a + * torn-down or replaced session. + */ + async #startBattery(batt: NobleCharacteristic | null, peripheral: NoblePeripheral): Promise { + if (!batt) return; + try { + const value = await batt.readAsync(); + if (this.#peripheral !== peripheral || !this.#sessionActive) return; + if (value.length) this.#handlers?.onBattery(value[0]!); + batt.on("data", (data) => { + if (data.length && this.#peripheral === peripheral && this.#sessionActive) { + this.#handlers?.onBattery(data[0]!); + } + }); + await batt.subscribeAsync(); + } catch { + /* no Battery service, or notifications unsupported — the read (if any) stands */ + } + } + + /** Race a GATT promise against the attempt's cancel hook and the shared deadline. */ + #race(work: Promise, attempt: ConnectAttempt, deadline: number, what: string): Promise { + return new Promise((resolve, reject) => { + let timer: ReturnType | undefined; + let done = false; + const settle = (fn: () => void): void => { + if (done) return; + done = true; + if (timer !== undefined) clearTimeout(timer); + attempt.fail = null; + fn(); + }; + attempt.fail = (err) => settle(() => reject(err)); + const remaining = deadline - Date.now(); + if (Number.isFinite(remaining)) { + timer = setTimeout( + () => settle(() => reject(new ScanTimeoutError(`${what} timed out after ${this.#scanTimeoutMs}ms.`))), + Math.max(0, remaining), + ); + } + work.then( + (value) => settle(() => resolve(value)), + (err) => settle(() => reject(err)), + ); }); } #matches(peripheral: NoblePeripheral): boolean { if (this.#address) return peripheral.address?.toLowerCase() === this.#address; const name = peripheral.advertisement?.localName ?? ""; - return name.toLowerCase().startsWith(this.#namePrefix.toLowerCase()); + return name.toLowerCase().startsWith(this.#namePrefixLower); } - #onDisconnected = (): void => { - this.#teardown(); - this.#disconnectHandler?.(); + /** Remote link drop. Explicit disconnect() runs #finishSession itself first. */ + #onPeripheralDisconnect = (): void => { + if (this.#sessionActive) this.#finishSession(); }; + /** Tear the active session down and fire onDisconnect — structurally exactly once: + * #teardown removes the peripheral's disconnect listener before the handler runs. */ + #finishSession(): void { + this.#sessionActive = false; + this.#teardown(); + this.#handlers?.onDisconnect(); + } + #teardown(): void { + this.#sessionActive = false; this.#peripheral?.removeAllListeners("disconnect"); + // Characteristic 'data' listeners are intentionally not removed: noble builds + // fresh Characteristic instances on every discovery, so old listeners die with + // the old objects. this.#rxChar = null; this.#txChar = null; this.#ctrlChar = null; this.#peripheral = null; + // #lastPeripheral is retained so reconnect() can re-link without a scan. } } From 19f01fb4efffb3d4de2b7bc72dbad85868a6d5c6 Mon Sep 17 00:00:00 2001 From: Flop Butylkin Date: Sat, 4 Jul 2026 18:37:15 +0200 Subject: [PATCH 4/7] docs(triki-controller): transport contract v2, Node options, README; clean example shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - library.md: the Custom Transports section now documents the real surface — attach(handlers)/detach, optional reconnect/preflight, the exactly-once onDisconnect + reject-on-cancel + idempotent-disconnect contract (the old list omitted onBattery entirely, so a transport written from the docs crashed the controller constructor) — plus a minimal replay-transport example. The Node section gains scanTimeoutMs / scanServiceUuids, the ScanTimeoutError / ConnectAbortedError semantics, and the reconnect guarantee. The API section states the event guarantees (precondition failures reject with zero connectionchange events). - README: was never updated for transports — broaden the Web-Bluetooth-only framing, document the transport option, add a Node.js & custom transports section pointing at the guide. - overview.md: the "native BLE transport" half of the bridge roadmap item is done; reflect that. - example/node.mjs: exit through connectionchange after disconnect() (with an unref'd 1 s fallback) instead of process.exit right after firing the disconnect — the old handler killed Node before the HCI disconnect was sent, leaving the token connected until its supervision timeout. Wrap the top-level connect() in try/catch: Ctrl+C mid-scan now rejects with ConnectAbortedError. Co-Authored-By: Claude Fable 5 --- docs/guide/library.md | 88 ++++++++++++++++++---- docs/guide/overview.md | 7 +- packages/triki-controller/README.md | 38 ++++++++-- packages/triki-controller/example/node.mjs | 15 +++- 4 files changed, 125 insertions(+), 23 deletions(-) diff --git a/docs/guide/library.md b/docs/guide/library.md index 11d2b79..d7967be 100644 --- a/docs/guide/library.md +++ b/docs/guide/library.md @@ -129,7 +129,7 @@ interface OrientationEvent { | `static isSupported()` | `true` when `navigator.bluetooth` exists (SSR-safe). | | `connect()` | Show the picker, connect, start streaming. User gesture required. | | `reconnect()` | Reconnect to the last paired device without the picker. | -| `disconnect()` | Disconnect. | +| `disconnect()` | Disconnect. Idempotent; safe in any state — cancels a pending `connect()`. | | `setLed(on)` | Green LED on/off (throws if the token has no LED characteristic). | | `setRate(hz)` | Set the sample rate; applied live when streaming. | | `resetHeading()` | Re-zero yaw (no-op when fusion is off). | @@ -140,9 +140,15 @@ interface OrientationEvent { | `setGyroBias(v)` / `setAccelBias(v)` | Set the per-axis correction vectors live. | | `isConnected` / `state` / `rateHz` / `hasLed` / `battery` / `fusion` / `fusionAlgorithm` | Getters. | -`connect()` rejects (after cleaning up) if pairing or the handshake fails, so wrap it in -`try/catch` to surface picker errors. `reconnect()` reuses the cached device and throws -if `connect()` was never called. +`connect()` rejects (after cleaning up, transport included) if pairing or the handshake +fails, so wrap it in `try/catch` to surface picker errors. `reconnect()` reuses the +cached device and throws if `connect()` was never called. + +Event guarantees: precondition failures (no Web Bluetooth, nothing to reconnect to) +reject **before** any state change — zero `connectionchange` events. Once a real +attempt starts you get `pairing`, and on failure a single `disconnected` before the +rejection. Calling `disconnect()` while `connect()` is pending cancels it (the pending +promise rejects). ## Node.js (receive outside the browser) @@ -177,9 +183,18 @@ prompts the terminal (or your IDE) for Bluetooth access. | Option | Default | Meaning | |---|---|---| -| `namePrefix` | `"TRIKI"` | match devices whose advertised name starts with this prefix | +| `namePrefix` | `"TRIKI"` | match devices whose advertised name starts with this prefix (case-insensitive) | | `address` | — | match a specific BLE address instead of by name | | `noble` | `@abandonware/noble` | a noble-API-compatible module to use instead | +| `scanTimeoutMs` | `15000` | budget for adapter power-on + scan + link setup; `Infinity` to wait forever | +| `scanServiceUuids` | `[]` | advertised-service filter for the scan (e.g. `[NUS_SERVICE]`); empty scans everything | + +The whole connect attempt shares the `scanTimeoutMs` budget and rejects with a +**`ScanTimeoutError`** when it runs out; `disconnect()` during a pending `connect()` +rejects it with a **`ConnectAbortedError`** (both exported from +`triki-controller/node`). `reconnect()` re-links to the exact peripheral of the +previous successful `connect()` — no scan, so with several tokens in range it can +never silently attach to a different one. ::: warning Native build `@abandonware/noble` compiles a native addon, and its bundled `node-gyp` is old enough @@ -194,12 +209,55 @@ const triki = new TrikiController({ transport: new NobleTransport({ noble }) }); ### Custom transports -`NobleTransport` and the browser `WebBluetoothTransport` both implement the small -**`TrikiTransport`** interface (`connect`, `writeRx`, `writeCtrl`, `onFrame`, -`onDisconnect`, `disconnect`, `hasLed`). Implement it yourself to drive the controller -over any link — a WebSocket bridge, a serial dongle, or a replay of recorded frames — -while reusing all of the parsing and fusion. The transport is a dumb NUS pipe; the -controller owns the Triki protocol, so the protocol logic lives in exactly one place. +`NobleTransport` and the browser `WebBluetoothTransport` both implement the +**`TrikiTransport`** interface. Implement it yourself to drive the controller over any +link — a WebSocket bridge, a serial dongle, or a replay of recorded frames — while +reusing all of the parsing and fusion. The transport is a dumb NUS pipe; the controller +owns the Triki protocol, so the protocol logic lives in exactly one place. + +Required members: `attach(handlers)`, `detach()`, `connect()`, `writeRx(data, +withoutResponse?)`, `writeCtrl(data)`, `hasLed`, `disconnect()`. Optional: +`reconnect()` (omit it when the transport can't remember a device — the controller +then rejects `reconnect()` with a clear error) and `preflight(op)` (synchronous +precondition check, called before any state change). + +The controller claims the transport with `attach(handlers)` in its constructor — one +controller per transport (`attach` throws if already attached). The handlers object +carries `onFrame(bytes)`, `onDisconnect()`, and `onBattery(percent)`; call only the +ones you have data for. + +The contract, in short: + +- `onDisconnect()` fires **exactly once** per successful `connect()`/`reconnect()`, + and **never** when no connect succeeded — a failed connect signals through its + rejection. +- `disconnect()` is idempotent and safe in any state: it tears down an active session + (firing `onDisconnect` once), cancels a pending `connect()` (which then rejects), + and is a no-op otherwise. +- `connect()` cleans up its own resources before rejecting. + +A minimal replay transport: + +```ts +import type { TrikiTransport, TrikiTransportHandlers } from "triki-controller"; + +class ReplayTransport implements TrikiTransport { + #handlers: TrikiTransportHandlers | null = null; + readonly hasLed = false; + + attach(handlers: TrikiTransportHandlers): void { + if (this.#handlers) throw new Error("Already attached."); + this.#handlers = handlers; + } + detach(): void { this.#handlers = null; } + async connect(): Promise { /* open your source */ } + async writeRx(): Promise { /* swallow START — it's a replay */ } + async writeCtrl(): Promise { throw new Error("No LED on a replay."); } + disconnect(): void { this.#handlers?.onDisconnect(); /* if connected */ } + + feed(bytes: Uint8Array): void { this.#handlers?.onFrame(bytes); } +} +``` ## Standalone primitives @@ -240,9 +298,11 @@ Exported alongside the controller: `reset()`. `decodeCounts(frame)` decodes a single 14-byte frame. - `startCmd(hz)` / `ledCmd(on)` — command builders. - Quaternion helpers `quatMul` / `quatAboutZ` / `yawRadOf` / `eulerOf`. -- Constants: `NUS_SERVICE` / `NUS_RX` / `NUS_TX` / `NUS_CTRL`, `START_BASE`, - `FRAME_LEN`, `SUPPORTED_RATES_HZ` (`[26, 52, 104, 208, 416]`), `DEFAULT_RATE_HZ`, - `DEFAULT_GYRO_SCALE`, `DEFAULT_ACCEL_SCALE`, `DEFAULT_BETA`, `DEFAULT_TAU_ACC`. +- Constants: `NUS_SERVICE` / `NUS_RX` / `NUS_TX` / `NUS_CTRL`, + `BATTERY_SERVICE` / `BATTERY_LEVEL` (canonical 128-bit UUIDs), + `DEVICE_NAME_PREFIX`, `START_BASE`, `FRAME_LEN`, `SUPPORTED_RATES_HZ` + (`[26, 52, 104, 208, 416]`), `DEFAULT_RATE_HZ`, `DEFAULT_GYRO_SCALE`, + `DEFAULT_ACCEL_SCALE`, `DEFAULT_BETA`, `DEFAULT_TAU_ACC`. ## Caveats diff --git a/docs/guide/overview.md b/docs/guide/overview.md index cf6baf1..68625b5 100644 --- a/docs/guide/overview.md +++ b/docs/guide/overview.md @@ -51,7 +51,10 @@ tools/ BLE tooling + scripts (tracked; .venv ignored) into a reusable, dependency-free [`triki-controller`](https://www.npmjs.com/package/triki-controller) library (browser BLE client + framework-agnostic fusion math) -- [ ] Build a native controller bridge (joystick / OSC / HID) — the parse + fusion - half above is done and reusable; what's left is a native BLE transport and the +- [x] Receive the token **outside the browser**: `triki-controller` now has a + pluggable transport layer with a Node `NobleTransport` + (see [Node.js](./library#nodejs-receive-outside-the-browser)) +- [ ] Build a native controller bridge (joystick / OSC / HID) — the parse, fusion + and Node BLE input halves above are done and reusable; what's left is the OS-level output, since a browser can't emit a real joystick / HID (see [Web Bluetooth controller](./controller)) diff --git a/packages/triki-controller/README.md b/packages/triki-controller/README.md index 0f43b11..c9e9aa7 100644 --- a/packages/triki-controller/README.md +++ b/packages/triki-controller/README.md @@ -1,16 +1,19 @@ # triki-controller -A dependency-free, strongly-typed **Web Bluetooth** client for the [Żabka **Triki**](https://github.com/Flopsstuff/triki) +A strongly-typed driver for the [Żabka **Triki**](https://github.com/Flopsstuff/triki) BLE token (an nRF52810 BLE SoC + LSM6DSL accelerometer/gyroscope, shaped like a bottle cap). It connects over the Nordic UART Service, starts the IMU stream, parses the 14-byte motion frames, and optionally fuses orientation with a selectable 6-axis filter — **Madgwick** or -**VQF** — so you can reuse the token as a motion controller from any web app. +**VQF** — so you can reuse the token as a motion controller. **Web Bluetooth** in the +browser (zero runtime dependencies), or **Node** via the optional +[`@abandonware/noble`](https://github.com/abandonware/noble) transport. ![Triki motion controller demo](https://flopsstuff.github.io/triki/img/controller-demo.gif) ## Browser support -Web Bluetooth is required: +In the browser, Web Bluetooth is required (see [Node.js & custom transports](#nodejs--custom-transports) +for everything else): - ✅ Desktop **Chrome / Edge / Opera** (Chromium-based). - ❌ **Safari and Firefox** do not implement Web Bluetooth. @@ -71,14 +74,15 @@ standalone (the filters share an `OrientationFilter` interface). `new TrikiController(options?)` — `options`: `fusion?` (`"madgwick"`/`"vqf"`/`"accel"`/`"none"`, default `"madgwick"`), `rateHz?` (default `104`), `beta?` (Madgwick gain, default `0.08`), `tauAcc?` (VQF accel low-pass seconds, default `2.0`), `gyroScale?` (default `14.286`), -`gyroBias?` / `accelBias?` (per-axis correction vectors, default zeros). +`gyroBias?` / `accelBias?` (per-axis correction vectors, default zeros), `transport?` +(a `TrikiTransport`; defaults to the browser's `WebBluetoothTransport`). | Member | Description | | --- | --- | | `static isSupported()` | `true` when `navigator.bluetooth` exists (SSR-safe). | | `connect()` | Show the picker, connect, start streaming. User gesture required. | | `reconnect()` | Reconnect to the last paired device without the picker. | -| `disconnect()` | Disconnect. | +| `disconnect()` | Disconnect. Idempotent; safe in any state — cancels a pending `connect()`. | | `setLed(on)` | Green LED on/off (throws if the token has no LED characteristic). | | `setRate(hz)` | Set sample rate; applied live when streaming. | | `resetHeading()` | Re-zero yaw (no-op when fusion is off). | @@ -92,6 +96,30 @@ default `"madgwick"`), `rateHz?` (default `104`), `beta?` (Madgwick gain, defaul Events: `frame`, `orientation` (fusion only), `connectionchange` (payload is the state string), `rate` (measured Hz), `battery` (level in percent, 0–100). +## Node.js & custom transports + +The BLE link is pluggable: pass any `TrikiTransport` via the `transport` option. To +receive the token from a plain Node process, use the **`NobleTransport`** from the +`triki-controller/node` entry (backed by the **optional** peer dependency +`@abandonware/noble` — installed separately, imported lazily): + +```sh +npm install triki-controller @abandonware/noble +``` + +```ts +import { TrikiController, NobleTransport } from "triki-controller/node"; + +const triki = new TrikiController({ transport: new NobleTransport() }); +triki.on("orientation", (o) => console.log(o.euler)); +await triki.connect(); // scans for a "TRIKI" token (15 s budget), connects, streams +``` + +Options, timeout/cancellation semantics, the full `TrikiTransport` contract, and a +runnable example live in the +[library guide](https://flopsstuff.github.io/triki/guide/library#nodejs-receive-outside-the-browser) +and [`example/node.mjs`](./example/node.mjs). + ## Caveats - **Yaw drift.** Fusion is 6-axis (gyro + accel, no magnetometer), so absolute heading drifts diff --git a/packages/triki-controller/example/node.mjs b/packages/triki-controller/example/node.mjs index 7790584..2c0e19b 100644 --- a/packages/triki-controller/example/node.mjs +++ b/packages/triki-controller/example/node.mjs @@ -23,9 +23,20 @@ triki.on("orientation", ({ euler }) => { }); process.on("SIGINT", () => { + if (triki.state === "disconnected") process.exit(0); + // Exit once the BLE teardown lands, so the token isn't left connected. + triki.on("connectionchange", (state) => { + if (state === "disconnected") process.exit(0); + }); triki.disconnect(); - process.exit(0); + // Fallback: force-exit if the teardown stalls; unref'd so it never holds the loop. + setTimeout(() => process.exit(0), 1000).unref?.(); }); console.log("Scanning for a TRIKI token… (Ctrl+C to quit)"); -await triki.connect(); +try { + await triki.connect(); // 15 s scan budget by default; Ctrl+C mid-scan rejects +} catch (err) { + console.error("[connect]", err instanceof Error ? err.message : err); + process.exit(1); +} From b87f8cb59ac09bc5de6264959b0d544916facb17 Mon Sep 17 00:00:00 2001 From: Flop Butylkin Date: Sat, 4 Jul 2026 18:50:41 +0200 Subject: [PATCH 5/7] =?UTF-8?q?chore(triki-controller):=20bump=20to=200.3.?= =?UTF-8?q?0=20=E2=80=94=200.2.0=20is=20already=20published?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0.2.0 is live on npm without the ./node subpath or the v2 transport interface; this branch adds new public API (triki-controller/node, NobleTransport, attach/detach transport contract, DEVICE_NAME_PREFIX) and breaks the just-shipped TrikiTransport shape, so the next release must be 0.3.0 — publish.yml also refuses a tag that does not match package.json, and npm rejects a republish. Co-Authored-By: Claude Fable 5 --- packages/triki-controller/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/triki-controller/package.json b/packages/triki-controller/package.json index c92e1d9..9de234c 100644 --- a/packages/triki-controller/package.json +++ b/packages/triki-controller/package.json @@ -1,6 +1,6 @@ { "name": "triki-controller", - "version": "0.2.0", + "version": "0.3.0", "description": "Driver for the Żabka Triki token (nRF52810 + LSM6DSL) — connect, stream the IMU, and read fused orientation (selectable Madgwick or VQF) to reuse the token as a motion controller. Web Bluetooth in the browser, or @abandonware/noble in Node.", "type": "module", "license": "MIT", From e5ff2a540929ed6ac0a120da0c3de331fcac0f80 Mon Sep 17 00:00:00 2001 From: Flop Butylkin Date: Sat, 4 Jul 2026 19:03:07 +0200 Subject: [PATCH 6/7] docs(triki-controller): make the ReplayTransport example honor the transport contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The example's disconnect() fired onDisconnect unconditionally (even when never connected) and feed() kept delivering frames after disconnect — both violations of the contract the surrounding section documents. Track a #connected flag: disconnect() is now idempotent and never fires onDisconnect unconnected, and feed() only forwards frames while active. Deliberately NOT clearing #handlers in disconnect(): attach/detach is ownership (the controller stays attached across sessions), not connection state. Co-Authored-By: Claude Fable 5 --- docs/guide/library.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/guide/library.md b/docs/guide/library.md index d7967be..d6c600e 100644 --- a/docs/guide/library.md +++ b/docs/guide/library.md @@ -243,6 +243,7 @@ import type { TrikiTransport, TrikiTransportHandlers } from "triki-controller"; class ReplayTransport implements TrikiTransport { #handlers: TrikiTransportHandlers | null = null; + #connected = false; // connection state — separate from attach/detach ownership readonly hasLed = false; attach(handlers: TrikiTransportHandlers): void { @@ -250,12 +251,18 @@ class ReplayTransport implements TrikiTransport { this.#handlers = handlers; } detach(): void { this.#handlers = null; } - async connect(): Promise { /* open your source */ } + async connect(): Promise { this.#connected = true; /* open your source */ } async writeRx(): Promise { /* swallow START — it's a replay */ } async writeCtrl(): Promise { throw new Error("No LED on a replay."); } - disconnect(): void { this.#handlers?.onDisconnect(); /* if connected */ } + disconnect(): void { + if (!this.#connected) return; // idempotent; never fires onDisconnect unconnected + this.#connected = false; + this.#handlers?.onDisconnect(); + } - feed(bytes: Uint8Array): void { this.#handlers?.onFrame(bytes); } + feed(bytes: Uint8Array): void { + if (this.#connected) this.#handlers?.onFrame(bytes); // frames only while active + } } ``` From 5dbb6e7c0126caf80ba10049e259c99f6cb7746a Mon Sep 17 00:00:00 2001 From: Flop Butylkin Date: Sat, 4 Jul 2026 19:21:48 +0200 Subject: [PATCH 7/7] =?UTF-8?q?fix(triki-controller):=20harden=20the=20con?= =?UTF-8?q?nect=20lifecycle=20=E2=80=94=20second-review=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A recall-mode multi-agent review of the finished branch surfaced a wave of lifecycle races and platform gaps; all confirmed findings are fixed here, each with a regression test. Controller: - #open gains an epoch token and closes the transport BEFORE emitting 'disconnected': an auto-retry started synchronously from a connectionchange listener no longer gets cancelled by the failed attempt's own cleanup (it used to spiral into a self-sustaining cancel loop), and a superseded attempt's late rejection can no longer reset a newer attempt's state. - disconnect()'s local fallback is epoch-guarded too, so a connect() started from the 'disconnected' listener survives the stack unwind. - New dispose(): disconnect + detach + drop listeners — the missing release path for long-lived transports (a second controller can then attach). - assertTransport rejects primitives with the friendly member-list TypeError instead of crashing on the `in` operator. - ConnectAbortedError/ScanTimeoutError now live in transport.ts (contract level, browser-safe) and are exported from the main entry as well. WebBluetoothTransport: - Per-attempt cancellation object + in-progress/already-connected guards replace the instance-wide #connecting/#cancelRequested pair: a retry connect() can no longer un-cancel and resurrect an earlier attempt, and two attempts can no longer interleave on shared session fields. - Session state is built in locals and committed only after every setup step succeeds — a failed/cancelled attempt never leaves half-built state. - #emitBattery guards byteLength: a 0-byte ATT value (protocol-legal) used to throw an uncaught RangeError per notification, or silently kill battery support for the session via the outer catch. NobleTransport: - disconnect() clears #attempt synchronously, so a same-tick disconnect()-then-connect() retry is no longer refused with a spurious "connect() is already in progress." - Session commit moved to the end of #openSession (locals until then): the abort path only drops the attempt's own peripheral, never touches instance state, and — because our 'disconnect' listener is attached only at commit — no longer strips noble's own promisify resolver (which leaked one forever-pending disconnectAsync promise per failed attempt) nor issues a duplicate HCI disconnect. - disconnect() checks the active session before the pending attempt, closing the microtask window where a just-committed session made connect() fulfil after a cancelling disconnect(). - #teardown removes ONLY the transport's own peripheral 'disconnect' listener (removeListener by reference): noble caches Peripheral objects module-wide, and removeAllListeners was silently clobbering third-party listeners. - The TX 'data' listener is session-guarded like the battery one: buffered notifications arriving after onDisconnect no longer violate the handlers contract for custom transport owners. - Scanning uses allowDuplicates=true — with duplicate filtering noble emits 'discover' once per peripheral per scan, so a first report without localName (name-in-scan-response, common on macOS) made the token permanently unmatchable; the scan is also now STOPPED (awaited) before connectAsync, which some stacks reject while scanning (BlueZ "Command Disallowed"). - Writes are bounded: noble never settles pending GATT callbacks on a dropped link, so writeRx/writeCtrl race the session teardown signal and the scanTimeoutMs budget instead of hanging await connect()/setRate()/setLed() forever. - address matching also accepts the noble peripheral id (macOS hides MAC addresses — address:"" there made the option unmatchable), documented. Packaging / docs / example: - peerDependencies range ">=1.9.2-26 <2.0.0": the old ^1.9.2-26 prerelease comparator only matched prereleases of exactly 1.9.2, guaranteeing ERESOLVE the moment noble ships its next (always prerelease-tagged) version. - tsup splitting:true — dist/node.js embedded a full second copy of the core (instanceof TrikiController was false across entries; 57 KB -> 18 KB + one shared chunk). - example/node.mjs: process.exit ran synchronously inside the 'disconnected' emit — before the transport even issued disconnectAsync — leaving the token connected; now exits after a short delay, and Ctrl+C mid-scan exits 0 via ConnectAbortedError. - Docs: reconnect() re-links to "the peripheral matched by the previous connect() scan" (the cache is armed at scan match, deliberately), dispose() documented, isSupported() scoped to the default transport, absolute example link for npm, coverage excludes the node re-export barrel. Refuted by verification (no change needed): stale-battery leak across reconnect (session guards + noble's fresh-characteristic dispatch close every window), gattserverdisconnected async race (the spec fires it synchronously inside gatt.disconnect()). 123 tests (9 new regression cases), typecheck and build clean. Co-Authored-By: Claude Fable 5 --- docs/guide/library.md | 13 +- packages/triki-controller/README.md | 5 +- packages/triki-controller/example/node.mjs | 15 +- packages/triki-controller/package.json | 2 +- .../triki-controller/src/controller.test.ts | 11 + .../src/controller.transport.test.ts | 54 +++++ packages/triki-controller/src/controller.ts | 59 +++-- packages/triki-controller/src/index.ts | 1 + packages/triki-controller/src/node.ts | 4 +- .../triki-controller/src/testkit/fakeNoble.ts | 18 +- .../src/testkit/fakeTransport.ts | 6 + packages/triki-controller/src/transport.ts | 20 +- .../src/transports/noble.test.ts | 63 ++++- .../triki-controller/src/transports/noble.ts | 229 ++++++++++++------ .../src/transports/web-bluetooth.ts | 135 +++++++---- packages/triki-controller/tsup.config.ts | 5 +- packages/triki-controller/vitest.config.ts | 2 +- 17 files changed, 475 insertions(+), 167 deletions(-) diff --git a/docs/guide/library.md b/docs/guide/library.md index d6c600e..878d48d 100644 --- a/docs/guide/library.md +++ b/docs/guide/library.md @@ -126,10 +126,11 @@ interface OrientationEvent { | Member | Description | |---|---| -| `static isSupported()` | `true` when `navigator.bluetooth` exists (SSR-safe). | +| `static isSupported()` | `true` when `navigator.bluetooth` exists (SSR-safe). Only about the **default** Web Bluetooth transport — in Node with `NobleTransport` just call `connect()`. | | `connect()` | Show the picker, connect, start streaming. User gesture required. | | `reconnect()` | Reconnect to the last paired device without the picker. | | `disconnect()` | Disconnect. Idempotent; safe in any state — cancels a pending `connect()`. | +| `dispose()` | Disconnect, release the transport (another controller may then attach), drop all listeners. | | `setLed(on)` | Green LED on/off (throws if the token has no LED characteristic). | | `setRate(hz)` | Set the sample rate; applied live when streaming. | | `resetHeading()` | Re-zero yaw (no-op when fusion is off). | @@ -184,16 +185,16 @@ prompts the terminal (or your IDE) for Bluetooth access. | Option | Default | Meaning | |---|---|---| | `namePrefix` | `"TRIKI"` | match devices whose advertised name starts with this prefix (case-insensitive) | -| `address` | — | match a specific BLE address instead of by name | +| `address` | — | match a specific BLE address **or noble peripheral id** instead of by name (macOS hides MAC addresses — use the id there) | | `noble` | `@abandonware/noble` | a noble-API-compatible module to use instead | -| `scanTimeoutMs` | `15000` | budget for adapter power-on + scan + link setup; `Infinity` to wait forever | +| `scanTimeoutMs` | `15000` | budget for adapter power-on + scan + link setup (writes share it too); `Infinity` to wait forever | | `scanServiceUuids` | `[]` | advertised-service filter for the scan (e.g. `[NUS_SERVICE]`); empty scans everything | The whole connect attempt shares the `scanTimeoutMs` budget and rejects with a **`ScanTimeoutError`** when it runs out; `disconnect()` during a pending `connect()` -rejects it with a **`ConnectAbortedError`** (both exported from -`triki-controller/node`). `reconnect()` re-links to the exact peripheral of the -previous successful `connect()` — no scan, so with several tokens in range it can +rejects it with a **`ConnectAbortedError`** (both exported from the main entry and +`triki-controller/node`). `reconnect()` re-links to the exact peripheral matched by +the previous `connect()` scan — no new scan, so with several tokens in range it can never silently attach to a different one. ::: warning Native build diff --git a/packages/triki-controller/README.md b/packages/triki-controller/README.md index c9e9aa7..46e1c82 100644 --- a/packages/triki-controller/README.md +++ b/packages/triki-controller/README.md @@ -79,10 +79,11 @@ default `"madgwick"`), `rateHz?` (default `104`), `beta?` (Madgwick gain, defaul | Member | Description | | --- | --- | -| `static isSupported()` | `true` when `navigator.bluetooth` exists (SSR-safe). | +| `static isSupported()` | `true` when `navigator.bluetooth` exists (SSR-safe). Only about the **default** Web Bluetooth transport — in Node with `NobleTransport` just call `connect()`. | | `connect()` | Show the picker, connect, start streaming. User gesture required. | | `reconnect()` | Reconnect to the last paired device without the picker. | | `disconnect()` | Disconnect. Idempotent; safe in any state — cancels a pending `connect()`. | +| `dispose()` | Disconnect, release the transport (another controller may then attach), drop all listeners. | | `setLed(on)` | Green LED on/off (throws if the token has no LED characteristic). | | `setRate(hz)` | Set sample rate; applied live when streaming. | | `resetHeading()` | Re-zero yaw (no-op when fusion is off). | @@ -118,7 +119,7 @@ await triki.connect(); // scans for a "TRIKI" token (15 s budget), connects, str Options, timeout/cancellation semantics, the full `TrikiTransport` contract, and a runnable example live in the [library guide](https://flopsstuff.github.io/triki/guide/library#nodejs-receive-outside-the-browser) -and [`example/node.mjs`](./example/node.mjs). +and [`example/node.mjs`](https://github.com/Flopsstuff/triki/blob/main/packages/triki-controller/example/node.mjs). ## Caveats diff --git a/packages/triki-controller/example/node.mjs b/packages/triki-controller/example/node.mjs index 2c0e19b..8e9e3e7 100644 --- a/packages/triki-controller/example/node.mjs +++ b/packages/triki-controller/example/node.mjs @@ -11,7 +11,7 @@ * macOS grants Bluetooth access per-app, so the first run prompts the terminal * (or your IDE) for permission. */ -import { TrikiController, NobleTransport } from "../dist/node.js"; +import { TrikiController, NobleTransport, ConnectAbortedError } from "../dist/node.js"; const triki = new TrikiController({ transport: new NobleTransport() }); @@ -23,20 +23,17 @@ triki.on("orientation", ({ euler }) => { }); process.on("SIGINT", () => { - if (triki.state === "disconnected") process.exit(0); - // Exit once the BLE teardown lands, so the token isn't left connected. - triki.on("connectionchange", (state) => { - if (state === "disconnected") process.exit(0); - }); - triki.disconnect(); - // Fallback: force-exit if the teardown stalls; unref'd so it never holds the loop. - setTimeout(() => process.exit(0), 1000).unref?.(); + triki.disconnect(); // kicks off the BLE teardown (fire-and-forget inside the transport) + // The 'disconnected' event fires synchronously, BEFORE the HCI disconnect is sent — + // so give the radio a moment instead of exiting from the event. + setTimeout(() => process.exit(0), 300); }); console.log("Scanning for a TRIKI token… (Ctrl+C to quit)"); try { await triki.connect(); // 15 s scan budget by default; Ctrl+C mid-scan rejects } catch (err) { + if (err instanceof ConnectAbortedError) process.exit(0); // user quit during the scan console.error("[connect]", err instanceof Error ? err.message : err); process.exit(1); } diff --git a/packages/triki-controller/package.json b/packages/triki-controller/package.json index 9de234c..7791cfa 100644 --- a/packages/triki-controller/package.json +++ b/packages/triki-controller/package.json @@ -68,7 +68,7 @@ "vitest": "^4.1.9" }, "peerDependencies": { - "@abandonware/noble": "^1.9.2-26" + "@abandonware/noble": ">=1.9.2-26 <2.0.0" }, "peerDependenciesMeta": { "@abandonware/noble": { diff --git a/packages/triki-controller/src/controller.test.ts b/packages/triki-controller/src/controller.test.ts index 55e7e11..55940bb 100644 --- a/packages/triki-controller/src/controller.test.ts +++ b/packages/triki-controller/src/controller.test.ts @@ -211,6 +211,17 @@ describe("battery", () => { expect(ctrl.battery).toBe(42); }); + test("ignores an empty battery payload instead of throwing", async () => { + const h = install({ batteryLevel: 87 }); + const ctrl = make({ fusion: "none" }); + await ctrl.connect(); + await flush(); // battery read + subscription settle + + // A 0-byte ATT value is protocol-legal; getUint8(0) on it would RangeError. + h.battery!.notify(new Uint8Array(0)); + expect(ctrl.battery).toBe(87); // the initial read stands, the empty update is dropped + }); + test("tolerates a token without the Battery service", async () => { install({ battery: false }); const ctrl = make({ fusion: "none" }); diff --git a/packages/triki-controller/src/controller.transport.test.ts b/packages/triki-controller/src/controller.transport.test.ts index b3fc2bc..31844c8 100644 --- a/packages/triki-controller/src/controller.transport.test.ts +++ b/packages/triki-controller/src/controller.transport.test.ts @@ -234,6 +234,60 @@ describe("reconnect through the transport", () => { }); }); +describe("re-entrant listeners (review regressions)", () => { + const flush = (): Promise => new Promise((r) => setTimeout(r, 0)); + + test("an auto-retry from the 'disconnected' listener is not cancelled by the failed attempt", async () => { + const { ctrl, t } = rig(); + t.writeRxError = new Error("START failed"); + let retried = false; + ctrl.on("connectionchange", (s) => { + if (s === "disconnected" && !retried) { + retried = true; + t.writeRxError = null; + void ctrl.reconnect(); // classic auto-reconnect, synchronously in the handler + } + }); + + await expect(ctrl.connect()).rejects.toThrow("START failed"); + await flush(); + expect(ctrl.state).toBe("streaming"); // the retry survived the original catch + }); + + test("disconnect()'s fallback does not clobber a connect() started from the listener", async () => { + const { ctrl, t } = rig(); + await ctrl.connect(); + let retried = false; + ctrl.on("connectionchange", (s) => { + if (s === "disconnected" && !retried) { + retried = true; + void ctrl.connect(); // re-enter synchronously during disconnect()'s emit + } + }); + + ctrl.disconnect(); + await flush(); + expect(ctrl.state).toBe("streaming"); + expect(t.connectCount).toBe(2); + }); +}); + +describe("dispose", () => { + test("releases the transport so another controller can attach", async () => { + const t = new FakeTransport(); + const first = make({ fusion: "none", transport: t }); + await first.connect(); + + first.dispose(); + expect(first.state).toBe("disconnected"); + expect(t.handlers).toBeNull(); + + const second = make({ fusion: "none", transport: t }); + await second.connect(); + expect(second.state).toBe("streaming"); + }); +}); + describe("transport ownership and validation", () => { test("the constructor rejects a malformed transport with a member list", () => { expect(() => make({ transport: {} as unknown as TrikiTransport })).toThrow(TypeError); diff --git a/packages/triki-controller/src/controller.ts b/packages/triki-controller/src/controller.ts index d5ea46a..2f6b05d 100644 --- a/packages/triki-controller/src/controller.ts +++ b/packages/triki-controller/src/controller.ts @@ -21,6 +21,7 @@ import { ledCmd, } from "./protocol"; import { WebBluetoothTransport } from "./transports/web-bluetooth"; +import { ConnectAbortedError } from "./transport"; import type { TrikiTransport } from "./transport"; import type { ConnectionState, @@ -59,6 +60,9 @@ function resolveFusion(opt: boolean | FusionAlgorithm | undefined): FusionAlgori * get no compiler help). */ function assertTransport(t: TrikiTransport): void { + if (t === null || (typeof t !== "object" && typeof t !== "function")) { + throw new TypeError(`Invalid TrikiTransport: expected an object, got ${typeof t}.`); + } const isFn = (name: string): boolean => typeof (t as unknown as Record)[name] === "function"; const problems: string[] = []; @@ -89,6 +93,8 @@ export class TrikiController extends TypedEmitter { #beta: number; #tauAcc: number; #state: ConnectionState = "disconnected"; + /** Bumped by every #open; failure paths compare it to ignore superseded attempts. */ + #epoch = 0; #rateHz: number; #gyroScale: number; #gyroBias: Vec3; @@ -193,14 +199,28 @@ export class TrikiController extends TypedEmitter { * `disconnected` when leaving `pairing`/`streaming`. */ disconnect(): void { + const epoch = this.#epoch; try { this.#transport.disconnect(); } catch { /* still reset local state below */ } // The transport's onDisconnect normally drives this; the fallback keeps the - // controller recoverable even against a buggy transport. - if (this.#state !== "disconnected") this.#cleanup(); + // controller recoverable even against a buggy transport. The epoch guard skips it + // when a listener already started a NEW attempt during the synchronous + // onDisconnect cascade — its 'pairing' state is not ours to reset. + if (this.#epoch === epoch && this.#state !== "disconnected") this.#cleanup(); + } + + /** + * Release this controller: disconnect, detach from the transport (so another + * controller may attach), and drop all event listeners. The controller must not be + * used afterwards. + */ + dispose(): void { + this.disconnect(); + this.#transport.detach(); + this.removeAllListeners(); } /** Turn the green LED on or off. Throws when the LED characteristic is unavailable. */ @@ -272,34 +292,45 @@ export class TrikiController extends TypedEmitter { /** * Shared connect/reconnect flow: sync pre-flight (zero events on failure), then * `pairing` → establish → START → `streaming`; any failure (including a - * {@link disconnect} racing the awaits) cleans up locally, closes the transport - * best-effort, and rethrows. + * {@link disconnect} racing the awaits) closes the transport, cleans up locally, + * and rethrows. The epoch token makes the failure path attempt-aware: a superseded + * attempt's late rejection must neither touch the transport nor reset the state a + * NEWER attempt owns. */ async #open(op: "connect" | "reconnect", establish: () => Promise): Promise { if (this.#state !== "disconnected") return; this.#transport.preflight?.(op); // throw → rejection with zero connectionchange events + const epoch = ++this.#epoch; this.#setState("pairing"); try { await establish(); - this.#assertPairing(); + this.#assertCurrent(epoch); await this.#transport.writeRx(startCmd(this.#rateHz), true); - this.#assertPairing(); + this.#assertCurrent(epoch); this.#startRateTimer(); this.#setState("streaming"); } catch (err) { - this.#cleanup(); - try { - this.#transport.disconnect(); // close a half-open link; #setState dedupes - } catch { - /* best-effort */ + if (this.#epoch === epoch) { + // Close the transport BEFORE emitting 'disconnected', so a retry started + // synchronously from a connectionchange listener runs against a fully + // torn-down transport instead of being cancelled by us a moment later. + try { + this.#transport.disconnect(); + } catch { + /* best-effort */ + } + // A listener may have started a new attempt during the emit above. + if (this.#epoch === epoch && this.#state !== "disconnected") this.#cleanup(); } throw err; } } - /** A disconnect (or lost link) intervened while connecting. */ - #assertPairing(): void { - if (this.#state !== "pairing") throw new Error("Connection cancelled while connecting."); + /** A disconnect (or a newer attempt) intervened while connecting. */ + #assertCurrent(epoch: number): void { + if (this.#epoch !== epoch || this.#state !== "pairing") { + throw new ConnectAbortedError("Connection cancelled while connecting."); + } } /** Transport callback: decode inbound notification bytes into frames. */ diff --git a/packages/triki-controller/src/index.ts b/packages/triki-controller/src/index.ts index 2766d7c..ae3b56c 100644 --- a/packages/triki-controller/src/index.ts +++ b/packages/triki-controller/src/index.ts @@ -26,6 +26,7 @@ export { SUPPORTED_RATES_HZ, } from "./protocol"; +export { ScanTimeoutError, ConnectAbortedError } from "./transport"; export type { TrikiTransport, TrikiTransportHandlers } from "./transport"; export type { Quaternion, EulerAngles, MadgwickOptions, OrientationFilter } from "./fusion"; export type { VqfOptions } from "./vqf"; diff --git a/packages/triki-controller/src/node.ts b/packages/triki-controller/src/node.ts index bf36495..1ce8880 100644 --- a/packages/triki-controller/src/node.ts +++ b/packages/triki-controller/src/node.ts @@ -6,6 +6,6 @@ * outside the browser. Keeping it on a separate entry means the default browser bundle * never imports noble. */ -export * from "./index"; -export { NobleTransport, ScanTimeoutError, ConnectAbortedError, nobleUuid } from "./transports/noble"; +export * from "./index"; // includes ScanTimeoutError / ConnectAbortedError +export { NobleTransport, nobleUuid } from "./transports/noble"; export type { NobleTransportOptions } from "./transports/noble"; diff --git a/packages/triki-controller/src/testkit/fakeNoble.ts b/packages/triki-controller/src/testkit/fakeNoble.ts index a6efc0e..77c6979 100644 --- a/packages/triki-controller/src/testkit/fakeNoble.ts +++ b/packages/triki-controller/src/testkit/fakeNoble.ts @@ -25,10 +25,11 @@ export class FakeNobleCharacteristic { writes: { data: Uint8Array; withoutResponse: boolean }[] = []; subscribeCount = 0; readValue: Uint8Array; - /** Test hooks: gate/fail the next readAsync/subscribeAsync. */ + /** Test hooks: gate/fail the next readAsync/subscribeAsync/writeAsync. */ readGate: Promise | null = null; readError: Error | null = null; subscribeError: Error | null = null; + writeGate: Promise | null = null; #dataListeners = new Set<(data: Uint8Array, isNotification: boolean) => void>(); @@ -38,6 +39,7 @@ export class FakeNobleCharacteristic { } async writeAsync(data: Uint8Array, withoutResponse: boolean): Promise { + if (this.writeGate) await this.writeGate; this.writes.push({ data, withoutResponse }); } @@ -87,6 +89,8 @@ export class FakeNoblePeripheral { connectCount = 0; disconnectAsyncCount = 0; discoverCalls: { serviceUuids: string[]; characteristicUuids: string[] }[] = []; + /** Shared operation-order log (wired to FakeNoble.log by makeFakeNoble). */ + log: string[] = []; /** Test hooks applied to the next operation / next discovery's fresh chars. */ connectGate: Promise | null = null; @@ -120,6 +124,7 @@ export class FakeNoblePeripheral { async connectAsync(): Promise { this.connectCount++; + this.log.push("connect"); if (this.connectGate) await this.connectGate; if (this.connectError) throw this.connectError; } @@ -163,6 +168,12 @@ export class FakeNoblePeripheral { return this; } + /** Remove one listener by reference (like Node's EventEmitter with once-wrapping). */ + removeListener(event: "disconnect", listener: () => void): this { + if (event === "disconnect") this.#disconnectListeners.delete(listener); + return this; + } + removeAllListeners(event?: string): this { if (event === undefined || event === "disconnect") this.#disconnectListeners.clear(); return this; @@ -229,8 +240,12 @@ export class FakeNoble { if (peripheral) void microtask().then(() => this.discover(peripheral)); } + /** Shared operation-order log (peripherals push "connect", scans push "stopScan"). */ + readonly log: string[] = []; + async stopScanningAsync(): Promise { this.stopScanCount++; + this.log.push("stopScan"); if (this.stopScanError) throw this.stopScanError; this.scanning = false; } @@ -261,6 +276,7 @@ export interface FakeNobleHandles { export function makeFakeNoble(options: FakeNobleOptions = {}): FakeNobleHandles { const noble = new FakeNoble(options.state ?? "poweredOn"); const peripheral = new FakeNoblePeripheral(options); + peripheral.log = noble.log; // one shared operation-order log if (options.autoDiscover ?? true) noble.autoDiscoverPeripheral = peripheral; return { noble, peripheral }; } diff --git a/packages/triki-controller/src/testkit/fakeTransport.ts b/packages/triki-controller/src/testkit/fakeTransport.ts index ab0d801..b353f7c 100644 --- a/packages/triki-controller/src/testkit/fakeTransport.ts +++ b/packages/triki-controller/src/testkit/fakeTransport.ts @@ -41,6 +41,9 @@ export class FakeTransport implements TrikiTransport { disconnectCount = 0; #cancelPending: ((err: Error) => void) | null = null; + /** Set by disconnect() while a gated connect is pending — checked after the race, + * so a cancel that lost the race (gate resolved first, same tick) still lands. */ + #pendingCancelled = false; attach(handlers: TrikiTransportHandlers): void { if (this.handlers) throw new Error("FakeTransport is already attached."); @@ -64,6 +67,7 @@ export class FakeTransport implements TrikiTransport { async #establish(): Promise { if (this.connectError) throw this.connectError; if (this.connectGate) { + this.#pendingCancelled = false; await Promise.race([ this.connectGate, new Promise((_, reject) => { @@ -71,6 +75,7 @@ export class FakeTransport implements TrikiTransport { }), ]); this.#cancelPending = null; + if (this.#pendingCancelled) throw new Error("Connection cancelled by disconnect()."); } this.connected = true; } @@ -87,6 +92,7 @@ export class FakeTransport implements TrikiTransport { disconnect(): void { this.disconnectCount++; if (this.#cancelPending) { + this.#pendingCancelled = true; const cancel = this.#cancelPending; this.#cancelPending = null; cancel(new Error("Connection cancelled by disconnect().")); diff --git a/packages/triki-controller/src/transport.ts b/packages/triki-controller/src/transport.ts index 0387e11..9142da5 100644 --- a/packages/triki-controller/src/transport.ts +++ b/packages/triki-controller/src/transport.ts @@ -4,7 +4,7 @@ * A transport is a dumb Nordic UART Service (NUS) pipe: it connects to the device, * writes command bytes to RX, exposes the optional control (LED) characteristic, and * streams TX notification bytes back. It knows nothing about the Triki protocol — the - * {@link TrikiController} builds the START/LED commands and parses the motion frames — + * `TrikiController` builds the START/LED commands and parses the motion frames — * so protocol logic lives in exactly one place and new transports stay tiny. * * Ownership is explicit: the controller claims a transport with {@link @@ -16,6 +16,22 @@ * (Node, via `@abandonware/noble`) is available from the `triki-controller/node` entry. */ +/** A connect attempt ran out of its time budget before the device was streaming. */ +export class ScanTimeoutError extends Error { + constructor(message: string) { + super(message); + this.name = "ScanTimeoutError"; + } +} + +/** `disconnect()` was called while a `connect()`/`reconnect()` was still pending. */ +export class ConnectAbortedError extends Error { + constructor(message: string) { + super(message); + this.name = "ConnectAbortedError"; + } +} + /** Callbacks a {@link TrikiTransport} owner registers via {@link TrikiTransport.attach}. */ export interface TrikiTransportHandlers { /** Inbound TX notification bytes (raw). Only called while a connection is active. */ @@ -72,7 +88,7 @@ export interface TrikiTransport { /** * Reconnect to the previously selected device without re-prompting. Optional — * transports that cannot remember a device may omit it, in which case - * {@link TrikiController.reconnect} rejects with a clear error. Same contract as + * `TrikiController.reconnect` rejects with a clear error. Same contract as * {@link connect}. */ reconnect?(): Promise; diff --git a/packages/triki-controller/src/transports/noble.test.ts b/packages/triki-controller/src/transports/noble.test.ts index 1244e24..7c0bdff 100644 --- a/packages/triki-controller/src/transports/noble.test.ts +++ b/packages/triki-controller/src/transports/noble.test.ts @@ -59,7 +59,8 @@ describe("happy path", () => { await transport.connect(); - expect(noble.startScanArgs).toEqual([{ serviceUuids: [], allowDuplicates: false }]); + // allowDuplicates=true: the advertised name may only arrive in a scan response. + expect(noble.startScanArgs).toEqual([{ serviceUuids: [], allowDuplicates: true }]); expect(noble.stopScanCount).toBe(1); expect(peripheral.connectCount).toBe(1); // NUS + Battery discovered in ONE GATT round trip. @@ -362,6 +363,66 @@ describe("reconnect", () => { }); }); +describe("review regressions", () => { + test("same-tick disconnect() then connect() retries cleanly", async () => { + const { transport, noble, peripheral } = rig({ autoDiscover: false }); + const first = transport.connect(); + first.catch(() => {}); + await flush(); // scanning + + transport.disconnect(); + const second = transport.connect(); // same tick as the cancel — must not be refused + await expect(first).rejects.toThrow(ConnectAbortedError); + + await flush(); // second scan armed + noble.discover(peripheral); + await second; + expect(peripheral.connectCount).toBe(1); // only the retry connected + }); + + test("the scan is stopped before connectAsync is issued", async () => { + const { transport, noble } = rig(); + await transport.connect(); + expect(noble.log).toEqual(["stopScan", "connect"]); + }); + + test("a write pending on a dead link rejects on disconnect instead of hanging", async () => { + const { transport, peripheral } = rig(); + await transport.connect(); + + const gate = deferred(); + peripheral.rx!.writeGate = gate.promise; + const write = transport.writeRx(startCmd(104), true); + transport.disconnect(); // noble would never settle the write callback + await expect(write).rejects.toThrow("Disconnected."); + gate.resolve(); // late settle must be inert + await flush(); + }); + + test("buffered TX notifications after disconnect never reach onFrame", async () => { + const { transport, peripheral, events } = rig(); + await transport.connect(); + const tx = peripheral.tx!; + + transport.disconnect(); + tx.emitData(Uint8Array.of(1, 2, 3)); // still buffered in the stack + expect(events.frames).toHaveLength(0); + }); + + test("teardown removes only the transport's own disconnect listener", async () => { + const { transport, peripheral } = rig(); + await transport.connect(); + + // Third-party code sharing noble's cached peripheral object. + const thirdParty = vi.fn(); + peripheral.once("disconnect", thirdParty); + + transport.disconnect(); // teardown must not clobber the third-party listener + await flush(); // noble's own async 'disconnect' echo lands + expect(thirdParty).toHaveBeenCalledTimes(1); + }); +}); + describe("ownership and concurrency guards", () => { test("attach() twice throws; detach() releases", () => { const { noble } = makeFakeNoble(); diff --git a/packages/triki-controller/src/transports/noble.ts b/packages/triki-controller/src/transports/noble.ts index 9923f31..1295b6c 100644 --- a/packages/triki-controller/src/transports/noble.ts +++ b/packages/triki-controller/src/transports/noble.ts @@ -6,15 +6,20 @@ * connects, discovers the NUS + Battery characteristics in one GATT round trip, and * subscribes to TX notifications. The whole attempt shares one time budget * ({@link NobleTransportOptions.scanTimeoutMs}, default 15 s) and rejects with - * {@link ScanTimeoutError} when it runs out. + * {@link ScanTimeoutError} when it runs out. Session state is committed only after + * every step succeeds, so a failed or cancelled attempt never leaks half-built state. * - `disconnect()` is idempotent: it cancels a pending `connect()` (which then rejects * with {@link ConnectAbortedError}), tears down an active session (firing - * `onDisconnect` exactly once), and is a no-op otherwise. - * - `reconnect()` re-links to the exact peripheral of the previous successful - * `connect()` — no scan, so it can never latch onto a different token — and throws + * `onDisconnect` exactly once), and is a no-op otherwise. A retry `connect()` may be + * issued immediately after the cancelling `disconnect()` — same tick is fine. + * - `reconnect()` re-links to the exact peripheral matched by the previous `connect()` + * scan — no new scan, so it can never latch onto a different token — and throws * when there is nothing to reconnect to. * - The battery read/subscribe is fire-and-forget: it never delays `connect()` or the * controller's START command. + * - Writes are bounded: noble never settles pending GATT callbacks on a dropped link, + * so `writeRx`/`writeCtrl` race the session teardown and the `scanTimeoutMs` budget + * instead of hanging forever. * * `@abandonware/noble` is an **optional peer dependency**: it is imported lazily and * only required when {@link NobleTransport.connect} runs, so importing @@ -26,30 +31,17 @@ * ``` */ import { NUS_SERVICE, NUS_RX, NUS_TX, NUS_CTRL, BATTERY_SERVICE, BATTERY_LEVEL, DEVICE_NAME_PREFIX } from "../protocol"; +import { ScanTimeoutError, ConnectAbortedError } from "../transport"; import type { TrikiTransport, TrikiTransportHandlers } from "../transport"; +export { ScanTimeoutError, ConnectAbortedError }; + // Node's Buffer, declared locally so this file needs no `@types/node` (noble's writes // want a Buffer; it is a Uint8Array subclass at runtime). declare const Buffer: { from(buffer: ArrayBufferLike, byteOffset?: number, length?: number): Uint8Array; }; -/** The connect()/scan budget ran out before a device was streaming. */ -export class ScanTimeoutError extends Error { - constructor(message: string) { - super(message); - this.name = "ScanTimeoutError"; - } -} - -/** disconnect() was called while a connect()/reconnect() was still pending. */ -export class ConnectAbortedError extends Error { - constructor(message: string) { - super(message); - this.name = "ConnectAbortedError"; - } -} - /** Default budget for adapter power-on + scan + link setup, in milliseconds. */ const DEFAULT_SCAN_TIMEOUT_MS = 15_000; @@ -61,7 +53,11 @@ export interface NobleTransportOptions { * {@link address} is set. */ namePrefix?: string; - /** Match a specific device by BLE address (case-insensitive) instead of by name. */ + /** + * Match a specific device by BLE address or noble peripheral id (case-insensitive) + * instead of by name. Note: on macOS CoreBluetooth hides MAC addresses (noble + * reports an empty `address`), so use the peripheral **id** there. + */ address?: string; /** * A noble-API-compatible module to use instead of lazily importing @@ -71,8 +67,9 @@ export interface NobleTransportOptions { noble?: unknown; /** * Time budget in milliseconds for the whole connect attempt: adapter power-on, the - * scan, and link setup (reconnect shares the same budget). Default 15 000. Pass - * `Infinity` to wait forever (cancel via `disconnect()`). + * scan, and link setup (reconnect and writes share the same budget). Default + * 15 000. Pass `Infinity` to wait forever (cancel via `disconnect()`). Zero, + * negative, or non-numeric values fall back to the default. */ scanTimeoutMs?: number; /** @@ -99,7 +96,8 @@ interface NobleCharacteristic { interface NoblePeripheral { /** Stable device id. noble caches Peripheral objects by id, and a cached peripheral - * supports `connectAsync()` again after `disconnectAsync()` — reconnect relies on it. */ + * supports `connectAsync()` again after `disconnectAsync()` — reconnect relies on it. + * On macOS this is the only stable identifier (CoreBluetooth hides MAC addresses). */ readonly id: string; readonly address: string; readonly advertisement: { localName?: string }; @@ -110,7 +108,8 @@ interface NoblePeripheral { characteristicUuids: string[], ): Promise<{ characteristics: NobleCharacteristic[] }>; once(event: "disconnect", listener: () => void): this; - removeAllListeners(event?: string): this; + /** Peripheral objects are shared module-globally; remove ONLY our own listener. */ + removeListener(event: "disconnect", listener: () => void): this; } interface Noble { @@ -167,6 +166,8 @@ interface ConnectAttempt { cancelled: boolean; /** Rejecter of whichever cancellable wait is pending; disconnect() invokes it. */ fail: ((err: Error) => void) | null; + /** The peripheral this attempt is linking to, once known — the abort path drops it. */ + peripheral: NoblePeripheral | null; } export class NobleTransport implements TrikiTransport { @@ -179,12 +180,15 @@ export class NobleTransport implements TrikiTransport { #noble: Noble | null = null; // lazily-loaded module, cached across sessions #handlers: TrikiTransportHandlers | null = null; #attempt: ConnectAttempt | null = null; - /** True only between connect()/reconnect() resolving and the next teardown. */ + /** True only between connect()/reconnect() committing a session and its teardown. */ #sessionActive = false; #peripheral: NoblePeripheral | null = null; #rxChar: NobleCharacteristic | null = null; #txChar: NobleCharacteristic | null = null; #ctrlChar: NobleCharacteristic | null = null; + /** Rejects when the session tears down — bounds writes that noble would never settle. */ + #dropped: Promise | null = null; + #rejectDropped: ((err: Error) => void) | null = null; /** Retained across teardown so reconnect() can re-link without a scan. */ #lastPeripheral: NoblePeripheral | null = null; @@ -227,19 +231,19 @@ export class NobleTransport implements TrikiTransport { this.#checkCancelled(attempt); const peripheral = await this.#scan(noble, attempt, deadline); this.#checkCancelled(attempt); - // Cached as soon as the scan matches, so reconnect() works even when the rest - // of this attempt fails (mirrors the web transport retaining its #device). + // Cached as soon as the scan matches, so reconnect() can re-link even when the + // rest of this attempt fails (mirrors the web transport retaining its #device). this.#lastPeripheral = peripheral; await this.#openSession(peripheral, attempt, deadline); } catch (err) { - this.#abortAttempt(); + this.#abortAttempt(attempt); throw err; } finally { - this.#attempt = null; + if (this.#attempt === attempt) this.#attempt = null; } } - /** Re-link to the peripheral of the previous successful connect() — no scan. */ + /** Re-link to the peripheral matched by the previous connect() scan — no new scan. */ async reconnect(): Promise { this.preflight("reconnect"); const peripheral = this.#lastPeripheral!; @@ -249,39 +253,46 @@ export class NobleTransport implements TrikiTransport { this.#checkCancelled(attempt); await this.#openSession(peripheral, attempt, deadline); } catch (err) { - this.#abortAttempt(); + this.#abortAttempt(attempt); throw err; } finally { - this.#attempt = null; + if (this.#attempt === attempt) this.#attempt = null; } } async writeRx(data: Uint8Array, withoutResponse = false): Promise { - if (!this.#rxChar) throw new Error("Not connected."); - await this.#rxChar.writeAsync(toBuffer(data), withoutResponse); + const rx = this.#rxChar; + if (!rx) throw new Error("Not connected."); + await this.#boundedWrite(rx.writeAsync(toBuffer(data), withoutResponse)); } async writeCtrl(data: Uint8Array): Promise { - if (!this.#ctrlChar) throw new Error("LED control characteristic is not available."); - await this.#ctrlChar.writeAsync(toBuffer(data), false); + const ctrl = this.#ctrlChar; + if (!ctrl) throw new Error("LED control characteristic is not available."); + await this.#boundedWrite(ctrl.writeAsync(toBuffer(data), false)); } disconnect(): void { + // Active session first: in the microtask window where a just-committed session + // still has its attempt pending in connect()'s finally, this is a real disconnect, + // not a cancellation — connect() has effectively succeeded. + if (this.#sessionActive) { + const peripheral = this.#peripheral!; + this.#finishSession(); + void peripheral.disconnectAsync().catch(() => {}); + return; + } const attempt = this.#attempt; if (attempt) { - // Cancel the pending connect: reject the cancellable wait (scan / power-on) or - // let the next checkpoint throw; drop a half-connected peripheral so pending - // GATT awaits settle. No onDisconnect — the connect() rejection is the signal. + // Cancel the pending connect: reject the cancellable wait (or let the next + // checkpoint throw). The attempt's own abort path drops any half-open link. + // #attempt is cleared HERE so a same-tick retry connect() is not refused. attempt.cancelled = true; + this.#attempt = null; attempt.fail?.(new ConnectAbortedError("Disconnected while connecting.")); - const peripheral = this.#peripheral; - if (peripheral) void peripheral.disconnectAsync().catch(() => {}); return; } - if (!this.#sessionActive) return; // idempotent; never connected -> no onDisconnect - const peripheral = this.#peripheral!; - this.#finishSession(); - void peripheral.disconnectAsync().catch(() => {}); + // Never connected (or already torn down): idempotent no-op, no onDisconnect. } // --- internal ---------------------------------------------------------------- @@ -290,7 +301,7 @@ export class NobleTransport implements TrikiTransport { async #beginAttempt(): Promise<{ noble: Noble; attempt: ConnectAttempt; deadline: number }> { if (this.#attempt) throw new Error("connect() is already in progress."); if (this.#sessionActive) throw new Error("Already connected."); - const attempt: ConnectAttempt = { cancelled: false, fail: null }; + const attempt: ConnectAttempt = { cancelled: false, fail: null, peripheral: null }; this.#attempt = attempt; const deadline = Number.isFinite(this.#scanTimeoutMs) ? Date.now() + this.#scanTimeoutMs @@ -300,7 +311,7 @@ export class NobleTransport implements TrikiTransport { this.#checkCancelled(attempt); return { noble, attempt, deadline }; } catch (err) { - this.#attempt = null; + if (this.#attempt === attempt) this.#attempt = null; throw err; } } @@ -309,11 +320,15 @@ export class NobleTransport implements TrikiTransport { if (attempt.cancelled) throw new ConnectAbortedError("Disconnected while connecting."); } - /** Failure path of an attempt: drop any half-open link, reset session state. */ - #abortAttempt(): void { - const peripheral = this.#peripheral; + /** + * Failure path of an attempt: drop the half-open link it created (session state was + * never committed, so no instance teardown — and our `disconnect` listener was never + * attached, so noble's own `disconnectAsync` resolver survives untouched). + */ + #abortAttempt(attempt: ConnectAttempt): void { + const peripheral = attempt.peripheral; + attempt.peripheral = null; if (peripheral) void peripheral.disconnectAsync().catch(() => {}); - this.#teardown(); } /** @@ -366,31 +381,33 @@ export class NobleTransport implements TrikiTransport { * Scan until a matching peripheral appears. Every exit — match, timeout, cancel, * or a startScanning failure — removes the `discover` listener and stops scanning, * so retried connects never stack listeners on the (module-level) noble emitter. + * The match path additionally AWAITS the scan stop before resolving: initiating an + * LE connection while the scan is still active is rejected by some stacks + * (Linux/BlueZ "Command Disallowed"). */ #scan(noble: Noble, attempt: ConnectAttempt, deadline: number): Promise { return new Promise((resolve, reject) => { let timer: ReturnType | undefined; let settled = false; - const cleanup = (): void => { + const cleanup = (): Promise => { settled = true; noble.removeListener("discover", onDiscover); if (timer !== undefined) clearTimeout(timer); attempt.fail = null; - void noble.stopScanningAsync().catch(() => {}); + return noble.stopScanningAsync().catch(() => {}); }; const onDiscover = (peripheral: NoblePeripheral): void => { - if (!this.#matches(peripheral)) return; - cleanup(); - resolve(peripheral); + if (settled || !this.#matches(peripheral)) return; + void cleanup().then(() => resolve(peripheral)); }; attempt.fail = (err) => { - cleanup(); + void cleanup(); reject(err); }; const remaining = deadline - Date.now(); if (Number.isFinite(remaining)) { timer = setTimeout(() => { - cleanup(); + void cleanup(); const target = this.#address ? `address "${this.#address}"` : `name prefix "${this.#namePrefixLower.toUpperCase()}"`; @@ -402,23 +419,31 @@ export class NobleTransport implements TrikiTransport { }, Math.max(0, remaining)); } noble.on("discover", onDiscover); - noble.startScanningAsync(this.#scanServiceUuids, false).catch((err) => { + // allowDuplicates=true: the advertised name often arrives only in the scan + // response, and with duplicate filtering noble emits 'discover' once per + // peripheral per scan — a first report without localName would make the token + // unmatchable for the rest of the scan. + noble.startScanningAsync(this.#scanServiceUuids, true).catch((err) => { if (!settled) { - cleanup(); + void cleanup(); reject(err); } }); }); } - /** Connect + discover + subscribe on a known peripheral. Shared by (re)connect. */ + /** + * Connect + discover + subscribe on a known peripheral. Shared by (re)connect. + * Session state is built in locals and committed to the instance only once every + * step has succeeded — a failed or cancelled attempt never leaves half-built state + * behind, and cannot clobber a newer attempt's session. + */ async #openSession( peripheral: NoblePeripheral, attempt: ConnectAttempt, deadline: number, ): Promise { - this.#peripheral = peripheral; - peripheral.once("disconnect", this.#onPeripheralDisconnect); + attempt.peripheral = peripheral; await this.#race(peripheral.connectAsync(), attempt, deadline, "Connecting to the device"); this.#checkCancelled(attempt); @@ -436,18 +461,35 @@ export class NobleTransport implements TrikiTransport { "Service discovery", ); this.#checkCancelled(attempt); - this.#rxChar = characteristics.find((c) => c.uuid === NUS_RX_ID) ?? null; - this.#txChar = characteristics.find((c) => c.uuid === NUS_TX_ID) ?? null; - this.#ctrlChar = characteristics.find((c) => c.uuid === NUS_CTRL_ID) ?? null; + const rx = characteristics.find((c) => c.uuid === NUS_RX_ID) ?? null; + const tx = characteristics.find((c) => c.uuid === NUS_TX_ID) ?? null; + const ctrl = characteristics.find((c) => c.uuid === NUS_CTRL_ID) ?? null; const battChar = characteristics.find((c) => c.uuid === BATTERY_LEVEL_ID) ?? null; - if (!this.#rxChar || !this.#txChar) { + if (!rx || !tx) { throw new Error("Triki NUS RX/TX characteristics not found on the device."); } - this.#txChar.on("data", (data) => this.#handlers?.onFrame(data)); - await this.#race(this.#txChar.subscribeAsync(), attempt, deadline, "TX subscription"); + // Session-guarded: buffered notifications can still arrive after teardown, and + // the handlers contract promises no onFrame after onDisconnect. + tx.on("data", (data) => { + if (this.#txChar === tx && this.#sessionActive) this.#handlers?.onFrame(data); + }); + await this.#race(tx.subscribeAsync(), attempt, deadline, "TX subscription"); this.#checkCancelled(attempt); + // Commit: from here on this is the live session. + this.#peripheral = peripheral; + this.#rxChar = rx; + this.#txChar = tx; + this.#ctrlChar = ctrl; + peripheral.once("disconnect", this.#onPeripheralDisconnect); + let rejectDropped!: (err: Error) => void; + const dropped = new Promise((_, rej) => { + rejectDropped = rej; + }); + dropped.catch(() => {}); // pre-handled so an idle session's teardown never surfaces it + this.#dropped = dropped; + this.#rejectDropped = rejectDropped; this.#sessionActive = true; // Fire-and-forget: battery must never delay connect() or the START command. void this.#startBattery(battChar, peripheral); @@ -502,8 +544,38 @@ export class NobleTransport implements TrikiTransport { }); } + /** + * Bound a write: noble never settles pending GATT callbacks once the link drops, + * so race the session's teardown signal and the shared time budget. + */ + #boundedWrite(work: Promise): Promise { + work.catch(() => {}); // injected noble modules may reject after we settle + const dropped = this.#dropped ?? Promise.reject(new Error("Not connected.")); + if (!this.#dropped) dropped.catch(() => {}); + let timer: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + if (Number.isFinite(this.#scanTimeoutMs)) { + timer = setTimeout( + () => reject(new ScanTimeoutError(`GATT write timed out after ${this.#scanTimeoutMs}ms.`)), + this.#scanTimeoutMs, + ); + } + }); + timeout.catch(() => {}); + return Promise.race([work, dropped, timeout]).finally(() => { + if (timer !== undefined) clearTimeout(timer); + }); + } + #matches(peripheral: NoblePeripheral): boolean { - if (this.#address) return peripheral.address?.toLowerCase() === this.#address; + if (this.#address) { + // macOS/CoreBluetooth hides MAC addresses (noble reports ""), so match the + // stable peripheral id too. + return ( + peripheral.address?.toLowerCase() === this.#address || + peripheral.id?.toLowerCase() === this.#address + ); + } const name = peripheral.advertisement?.localName ?? ""; return name.toLowerCase().startsWith(this.#namePrefixLower); } @@ -516,17 +588,22 @@ export class NobleTransport implements TrikiTransport { /** Tear the active session down and fire onDisconnect — structurally exactly once: * #teardown removes the peripheral's disconnect listener before the handler runs. */ #finishSession(): void { - this.#sessionActive = false; this.#teardown(); this.#handlers?.onDisconnect(); } #teardown(): void { this.#sessionActive = false; - this.#peripheral?.removeAllListeners("disconnect"); - // Characteristic 'data' listeners are intentionally not removed: noble builds - // fresh Characteristic instances on every discovery, so old listeners die with - // the old objects. + // Remove ONLY our own listener: the peripheral object is cached module-globally + // by noble and may carry listeners owned by other code (removeListener works on + // once()-wrapped listeners by original reference). + this.#peripheral?.removeListener("disconnect", this.#onPeripheralDisconnect); + // Characteristic 'data' listeners are left attached but session-guarded: noble + // builds fresh Characteristic instances on every discovery, and the guards drop + // anything that arrives after this point. + this.#rejectDropped?.(new Error("Disconnected.")); + this.#rejectDropped = null; + this.#dropped = null; this.#rxChar = null; this.#txChar = null; this.#ctrlChar = null; diff --git a/packages/triki-controller/src/transports/web-bluetooth.ts b/packages/triki-controller/src/transports/web-bluetooth.ts index f200f45..c92750e 100644 --- a/packages/triki-controller/src/transports/web-bluetooth.ts +++ b/packages/triki-controller/src/transports/web-bluetooth.ts @@ -2,6 +2,10 @@ * Web Bluetooth implementation of {@link TrikiTransport}. This is the only module that * touches `navigator.bluetooth`, and it is the default transport used by * {@link TrikiController} in the browser. + * + * Session state is built in locals and committed to the instance only after every + * setup step succeeds, so a failed or cancelled attempt never leaves half-built state + * behind and can never clobber a newer attempt's session. */ import { NUS_SERVICE, @@ -12,8 +16,14 @@ import { BATTERY_LEVEL, DEVICE_NAME_PREFIX, } from "../protocol"; +import { ConnectAbortedError } from "../transport"; import type { TrikiTransport, TrikiTransportHandlers } from "../transport"; +/** Bookkeeping for one in-flight connect()/reconnect(). */ +interface ConnectAttempt { + cancelled: boolean; +} + export class WebBluetoothTransport implements TrikiTransport { #device: BluetoothDevice | null = null; #gatt: BluetoothRemoteGATTServer | null = null; @@ -22,12 +32,10 @@ export class WebBluetoothTransport implements TrikiTransport { #ctrlChar: BluetoothRemoteGATTCharacteristic | null = null; #batteryChar: BluetoothRemoteGATTCharacteristic | null = null; #handlers: TrikiTransportHandlers | null = null; - /** True only between a successful connect()/reconnect() and the next teardown. */ + /** True only between a committed connect()/reconnect() and the next teardown. */ #connected = false; - /** True while a connect()/reconnect() is in flight. */ - #connecting = false; - /** Set by disconnect() during a pending connect; honored at the next checkpoint. */ - #cancelRequested = false; + /** The one in-flight connect/reconnect attempt, if any. */ + #attempt: ConnectAttempt | null = null; /** True when Web Bluetooth is available (safe to call during SSR). */ static isSupported(): boolean { @@ -63,35 +71,32 @@ export class WebBluetoothTransport implements TrikiTransport { */ async connect(): Promise { this.preflight("connect"); - this.#connecting = true; - this.#cancelRequested = false; + const attempt = this.#beginAttempt(); try { // A pending requestDevice picker cannot be cancelled programmatically; // disconnect() during it takes effect at the checkpoint below. - this.#device = await navigator.bluetooth.requestDevice({ + const device = await navigator.bluetooth.requestDevice({ // Web Bluetooth prefix matching is case-sensitive, hence both shipped casings // (noble's transport matches case-insensitively — inherently broader). filters: [{ namePrefix: DEVICE_NAME_PREFIX }, { namePrefix: "Triki" }], optionalServices: [NUS_SERVICE, BATTERY_SERVICE], }); - this.#checkCancelled(); - await this.#openSession(); + this.#checkCancelled(attempt); + this.#device = device; + await this.#openSession(attempt); } finally { - this.#connecting = false; - this.#cancelRequested = false; + if (this.#attempt === attempt) this.#attempt = null; } } /** Reconnect to the previously paired device without showing the picker. */ async reconnect(): Promise { this.preflight("reconnect"); - this.#connecting = true; - this.#cancelRequested = false; + const attempt = this.#beginAttempt(); try { - await this.#openSession(); + await this.#openSession(attempt); } finally { - this.#connecting = false; - this.#cancelRequested = false; + if (this.#attempt === attempt) this.#attempt = null; } } @@ -106,61 +111,88 @@ export class WebBluetoothTransport implements TrikiTransport { } disconnect(): void { - if (this.#connecting) { - // Cancel the pending connect: the in-flight session setup rejects at its next - // checkpoint (or its GATT await fails once the link drops). No onDisconnect — - // the connect() rejection is the signal. - this.#cancelRequested = true; + if (this.#connected) { + // Active session: the browser fires gattserverdisconnected synchronously inside + // gatt.disconnect() (per spec), driving teardown + onDisconnect exactly once. if (this.#gatt?.connected) this.#gatt.disconnect(); + else this.#onGattDisconnected(); + return; + } + const attempt = this.#attempt; + if (attempt) { + // Cancel the pending connect: the attempt rejects at its next checkpoint (a + // pending picker cannot be interrupted). #attempt is cleared HERE so a retry + // connect() is not refused; the cancelled attempt only ever touches its own + // locals, so the retry cannot be clobbered. No onDisconnect — the connect() + // rejection is the signal. + attempt.cancelled = true; + this.#attempt = null; return; } - if (this.#gatt?.connected) this.#gatt.disconnect(); // Never connected (or already torn down): idempotent no-op, no onDisconnect. } // --- internal ---------------------------------------------------------------- - #checkCancelled(): void { - if (this.#cancelRequested) throw new Error("Connection cancelled by disconnect()."); + #beginAttempt(): ConnectAttempt { + if (this.#attempt) throw new Error("connect() is already in progress."); + if (this.#connected) throw new Error("Already connected."); + const attempt: ConnectAttempt = { cancelled: false }; + this.#attempt = attempt; + return attempt; } - async #openSession(): Promise { + #checkCancelled(attempt: ConnectAttempt): void { + if (attempt.cancelled) throw new ConnectAbortedError("Connection cancelled by disconnect()."); + } + + /** + * Open a GATT session on #device. Built entirely in locals; instance state is + * committed only after the last step succeeds. On failure the local link is + * dropped and no instance field has changed. + */ + async #openSession(attempt: ConnectAttempt): Promise { const device = this.#device; if (!device || !device.gatt) throw new Error("Device GATT is not available."); + let gatt: BluetoothRemoteGATTServer | null = null; try { - // Same listener reference, so re-adding on reconnect is a no-op. - device.addEventListener("gattserverdisconnected", this.#onGattDisconnected); - - this.#gatt = await device.gatt.connect(); - this.#checkCancelled(); - const svc = await this.#gatt.getPrimaryService(NUS_SERVICE); - this.#checkCancelled(); - this.#rxChar = await svc.getCharacteristic(NUS_RX); - this.#checkCancelled(); - this.#txChar = await svc.getCharacteristic(NUS_TX); - this.#checkCancelled(); + gatt = await device.gatt.connect(); + this.#checkCancelled(attempt); + const svc = await gatt.getPrimaryService(NUS_SERVICE); + this.#checkCancelled(attempt); + const rx = await svc.getCharacteristic(NUS_RX); + this.#checkCancelled(attempt); + const tx = await svc.getCharacteristic(NUS_TX); + this.#checkCancelled(attempt); + let ctrl: BluetoothRemoteGATTCharacteristic | null; try { - this.#ctrlChar = await svc.getCharacteristic(NUS_CTRL); + ctrl = await svc.getCharacteristic(NUS_CTRL); } catch { - this.#ctrlChar = null; + ctrl = null; } - this.#checkCancelled(); + this.#checkCancelled(attempt); - await this.#txChar.startNotifications(); - this.#checkCancelled(); - this.#txChar.addEventListener("characteristicvaluechanged", this.#onNotify); + await tx.startNotifications(); + this.#checkCancelled(attempt); + // Commit: from here on this is the live session. + this.#gatt = gatt; + this.#rxChar = rx; + this.#txChar = tx; + this.#ctrlChar = ctrl; + tx.addEventListener("characteristicvaluechanged", this.#onNotify); + // Same listener reference, so re-adding on reconnect is a no-op. + device.addEventListener("gattserverdisconnected", this.#onGattDisconnected); this.#connected = true; - void this.#startBattery(this.#gatt); // best-effort; never blocks streaming + void this.#startBattery(gatt); // best-effort; never blocks streaming } catch (err) { - // Don't leave a half-open link behind a rejected connect. If the GATT drop - // fires #onGattDisconnected, #connected is still false, so no onDisconnect. + // Don't leave a half-open link behind a rejected connect. Only locals were + // touched, so a newer attempt's session (if any) is unaffected. try { - if (this.#gatt?.connected) this.#gatt.disconnect(); + if (gatt?.connected) gatt.disconnect(); } catch { /* best-effort */ } - this.#teardown(); throw err; } } @@ -202,7 +234,8 @@ export class WebBluetoothTransport implements TrikiTransport { }; #emitBattery(view: DataView): void { - this.#handlers?.onBattery(view.getUint8(0)); + // A 0-byte ATT value is protocol-legal (flaky firmware); getUint8(0) would throw. + if (view.byteLength) this.#handlers?.onBattery(view.getUint8(0)); } #onNotify = (event: Event): void => { @@ -215,8 +248,8 @@ export class WebBluetoothTransport implements TrikiTransport { }; #onGattDisconnected = (): void => { - // onDisconnect fires exactly once per successful connect — and never for a - // link that dropped before connect() resolved (the rejection is the signal). + // onDisconnect fires exactly once per committed session — and never for a link + // that dropped before connect() resolved (the rejection is the signal). const wasConnected = this.#connected; this.#teardown(); if (wasConnected) this.#handlers?.onDisconnect(); diff --git a/packages/triki-controller/tsup.config.ts b/packages/triki-controller/tsup.config.ts index 6b3b8eb..5042918 100644 --- a/packages/triki-controller/tsup.config.ts +++ b/packages/triki-controller/tsup.config.ts @@ -7,7 +7,10 @@ export default defineConfig({ dts: true, sourcemap: true, treeshake: true, - splitting: false, + // Splitting keeps the core in ONE shared chunk: with two entries and no splitting, + // dist/node.js would inline a full second copy of TrikiController & co., breaking + // `instanceof` across entry points and doubling the shipped code. + splitting: true, clean: true, outDir: "dist", // Optional native dependency for the Node transport — resolved at runtime, never bundled. diff --git a/packages/triki-controller/vitest.config.ts b/packages/triki-controller/vitest.config.ts index 9f4855d..54dde06 100644 --- a/packages/triki-controller/vitest.config.ts +++ b/packages/triki-controller/vitest.config.ts @@ -12,7 +12,7 @@ export default defineConfig({ coverage: { provider: "v8", include: ["src/**/*.ts"], - exclude: ["src/**/*.test.ts", "src/testkit/**", "src/index.ts"], + exclude: ["src/**/*.test.ts", "src/testkit/**", "src/index.ts", "src/node.ts"], }, }, });