Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 103 additions & 10 deletions src/transport/ReactNativeBleTransport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ describe('ReactNativeBleTransport', () => {
expect(found).not.toHaveBeenCalled();
});

it('deduplicates rotating addresses for the same named PC while a probe is in flight', async () => {
it('probes a distinct same-name address while another probe is in flight', async () => {
let scanCallback!: (error: Error | null, value: Device | null) => void;
let releaseConnect!: (value: Device) => void;
const connected = device();
Expand All @@ -602,15 +602,15 @@ describe('ReactNativeBleTransport', () => {
scanCallback(null, first);
await waitFor(() => (first.connect as jest.Mock).mock.calls.length === 1);
scanCallback(null, rotated);
expect(rotated.isConnected).not.toHaveBeenCalled();
expect(rotated.isConnected).toHaveBeenCalledTimes(1);

stop();
releaseConnect(connected);
await waitFor(() => (connected.cancelConnection as jest.Mock).mock.calls.length === 1);
expect(found).not.toHaveBeenCalled();
});

it('deduplicates a rotated Windows address after the first probe completes', async () => {
it('deduplicates a known Windows address but probes a new same-name address', async () => {
let scanCallback!: (error: Error | null, value: Device | null) => void;
const firstConnected = device({
readCharacteristicForService: jest.fn(async () => ({ value: fromByteArray(new TextEncoder().encode('{"protocolVersion":1,"desktopId":"pc-1","displayName":"A9_MAX","platform":"windows"}')) } as Characteristic)),
Expand All @@ -624,14 +624,16 @@ describe('ReactNativeBleTransport', () => {

scanCallback(null, first);
await waitFor(() => found.mock.calls.some(([desktop]) => desktop.desktopId === 'pc-1'));
scanCallback(null, first);
scanCallback(null, rotated);
await Promise.resolve();

expect(rotated.isConnected).not.toHaveBeenCalled();
expect(first.connect).toHaveBeenCalledTimes(1);
expect(rotated.isConnected).toHaveBeenCalledTimes(1);
stop();
});

it('discovers multiple Macs that share the Switchify PC Bluetooth name', async () => {
it.each(['android', 'ios'] as const)('discovers overlapping same-name PCs on %s', async (platform) => {
let scanCallback!: (error: Error | null, value: Device | null) => void;
const makeMac = (id: string, desktopId: string, displayName: string) => {
const connected = device({
Expand All @@ -642,21 +644,38 @@ describe('ReactNativeBleTransport', () => {
const first = makeMac('mac-1', 'pc-1', 'First Mac');
const second = makeMac('mac-2', 'pc-2', 'Second Mac');
const native = manager({ startDeviceScan: jest.fn((_uuids, _options, callback) => { scanCallback = callback; }) });
const transport = new ReactNativeBleTransport(native, 'ios');
const transport = new ReactNativeBleTransport(native, platform);
const found = jest.fn();
const stop = transport.scan(found, jest.fn());

scanCallback(null, first);
await waitFor(() => found.mock.calls.some(([desktop]) => desktop.desktopId === 'pc-1'));
scanCallback(null, second);
await waitFor(() => found.mock.calls.some(([desktop]) => desktop.desktopId === 'pc-1'));
await waitFor(() => found.mock.calls.some(([desktop]) => desktop.desktopId === 'pc-2'));

expect(first.connect).toHaveBeenCalledTimes(1);
expect(second.connect).toHaveBeenCalledTimes(1);
stop();
});

it('can resolve the second of two same-name PCs', async () => {
it('limits concurrent same-name probes to four and cancels them on stop', async () => {
let callback!: (error: Error | null, value: Device | null) => void;
const native = manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) });
const transport = new ReactNativeBleTransport(native, 'android');
const peers = Array.from({ length: 5 }, (_, index) => device({
id: `peer-${index}`, name: 'Switchify PC',
isConnected: jest.fn(() => new Promise<boolean>(() => undefined)),
}));
const stop = transport.scan(jest.fn(), jest.fn());
peers.forEach((peer) => callback(null, peer));
peers.slice(0, 4).forEach((peer) => expect(peer.isConnected).toHaveBeenCalledTimes(1));
expect(peers[4]!.isConnected).not.toHaveBeenCalled();
stop();
peers.slice(0, 4).forEach((peer) => expect(peer.cancelConnection).toHaveBeenCalled());
await transport.disconnect();
});

it.each(['android', 'ios'] as const)('resolves overlapping same-name PCs on %s without starving the target', async (platform) => {
let scanCallback!: (error: Error | null, value: Device | null) => void;
const firstConnected = device();
const first = device({ id: 'mac-1', name: 'Switchify PC', isConnected: jest.fn(async () => false), connect: jest.fn(async () => firstConnected) });
Expand All @@ -668,12 +687,13 @@ describe('ReactNativeBleTransport', () => {
});
const second = device({ id: 'mac-2', name: 'Switchify PC', isConnected: jest.fn(async () => false), connect: jest.fn(async () => secondConnected) });
const native = manager({ startDeviceScan: jest.fn((_uuids, _options, callback) => { scanCallback = callback; }) });
const transport = new ReactNativeBleTransport(native, 'ios');
const transport = new ReactNativeBleTransport(native, platform);

const resolving = transport.resolveAndConnect('pc-2');
await waitFor(() => typeof scanCallback === 'function');
scanCallback(null, first);
await waitFor(() => (firstConnected.cancelConnection as jest.Mock).mock.calls.length === 1);
scanCallback(null, second);
scanCallback(null, first);
scanCallback(null, second);
await expect(resolving).resolves.toMatchObject({ desktopId: 'pc-2', peripheralId: 'mac-2' });

Expand All @@ -682,6 +702,79 @@ describe('ReactNativeBleTransport', () => {
expect(secondConnected.cancelConnection).not.toHaveBeenCalled();
});

it.each(['android', 'ios'] as const)('queues a target behind four active probes on %s', async (platform) => {
let callback!: (error: Error | null, value: Device | null) => void;
const releases: (() => void)[] = [];
const blockers = Array.from({ length: 4 }, (_, index) => device({
id: `other-${index}`, name: 'Switchify PC',
discoverAllServicesAndCharacteristics: jest.fn(() => new Promise<Device>((resolve) => {
releases.push(() => resolve(device()));
})),
}));
const target = device({ id: 'target', name: 'Switchify PC', readCharacteristicForService: jest.fn(async () => ({
value: fromByteArray(new TextEncoder().encode('{"protocolVersion":1,"desktopId":"wanted"}')),
} as Characteristic)) });
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), platform);
const resolving = transport.resolveAndConnect('wanted');
await waitFor(() => !!callback);
blockers.forEach((peer) => callback(null, peer));
await waitFor(() => releases.length === 4);
callback(null, target);
expect(target.isConnected).not.toHaveBeenCalled();
releases.forEach((release) => release());
await expect(resolving).resolves.toMatchObject({ desktopId: 'wanted' });
expect(target.isConnected).toHaveBeenCalledTimes(1);
await transport.disconnect();
});

it('retains the newest Windows address by evicting the oldest at capacity', async () => {
let callback!: (error: Error | null, value: Device | null) => void;
const found = jest.fn();
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), 'android');
const stop = transport.scan(found, jest.fn());
const peers = Array.from({ length: 257 }, (_, index) => device({ id: `windows-${index}` }));
for (const [index, peer] of peers.entries()) {
callback(null, peer);
await waitFor(() => found.mock.calls.length === index + 1);
}
callback(null, peers[256]!);
expect(peers[256]!.isConnected).toHaveBeenCalledTimes(1);
callback(null, peers[0]!);
expect(peers[0]!.isConnected).toHaveBeenCalledTimes(2);
stop();
await transport.disconnect();
});

it('does not drain waiting probes while preparing a claimed target', async () => {
let callback!: (error: Error | null, value: Device | null) => void;
let releaseMtu!: () => void;
const releases: (() => void)[] = [];
const target = device({ id: 'target', requestMTU: jest.fn(() => new Promise<Device>((resolve) => {
releaseMtu = () => resolve(target);
})) });
const others = Array.from({ length: 3 }, (_, index) => device({
id: `other-${index}`, discoverAllServicesAndCharacteristics: jest.fn(() => new Promise<Device>((resolve) => {
releases.push(() => resolve(device()));
})),
}));
const queued = device({ id: 'queued' });
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), 'android');
const result = transport.resolveAndConnect('pc-1');
await waitFor(() => !!callback);
others.forEach((peer) => callback(null, peer));
callback(null, target);
callback(null, queued);
await waitFor(() => !!releaseMtu && releases.length === 3);
releases.forEach((release) => release());
await waitFor(() => others.every((peer) => (peer.readCharacteristicForService as jest.Mock).mock.calls.length === 1));
callback(null, queued);
expect(queued.isConnected).not.toHaveBeenCalled();
releaseMtu();
await result;
expect(queued.isConnected).not.toHaveBeenCalled();
await transport.disconnect();
});

it('waits for cancelled discovery probe cleanup before a real connection', async () => {
let scanCallback!: (error: Error | null, value: Device | null) => void;
let releaseDiscovery!: (value: Device) => void;
Expand Down
50 changes: 36 additions & 14 deletions src/transport/ReactNativeBleTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { BLE_DESCRIPTORS, BLE_UUIDS } from '@/domain/protocol/constants';
import { parseStatus } from '@/domain/protocol/responses';
import type { ConnectionStage, ConnectionStageOutcome, DiagnosticLog } from '@/diagnostics/DiagnosticLog';
import type { BleAvailability, BleTransport, DiscoveredDesktop, Unsubscribe } from './BleTransport';
import { bluetoothDeviceDisplayName, desktopDisplayName } from './desktopDisplayName';
import { desktopDisplayName } from './desktopDisplayName';
import { ReadResponsePoller } from './ReadResponsePoller';

export class ReactNativeBleTransport implements BleTransport {
Expand Down Expand Up @@ -46,34 +46,46 @@ export class ReactNativeBleTransport implements BleTransport {
scan(onDesktop: (desktop: DiscoveredDesktop) => void, onError: (error: Error) => void): Unsubscribe {
const operation = ++this.#operation;
let active = true;
this.#managerOrCreate().startDeviceScan([BLE_UUIDS.service], null, (error, device) => {
const waiting = new Map<string, Device>();
const onAdvertisement = (error: Error | null, device: Device | null) => {
if (!active || operation !== this.#operation) return;
if (error) { onError(error); return; }
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device)) || this.#scanDevices.size >= 4) return;
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device))) return;
if (this.#scanDevices.size >= 4) {
if (waiting.size < 32) waiting.set(device.id, device);
return;
}
waiting.delete(device.id);
const scanKey = this.#scanKey(device);
let retainCompletedKey = false;
this.#scanDevices.set(device.id, device);
this.#scanKeys.add(scanKey);
const task = this.#readStatus(device).then((desktop) => {
// Windows commonly rotates its private BLE address while retaining the
// computer name. Keep that name claimed for this scan after a
// successful probe so one PC cannot repeatedly open GATT connections.
// macOS uses the shared name "Switchify PC", so its key must be
// released after each probe to allow multiple Macs to be discovered.
retainCompletedKey = desktop?.platform === 'windows' && scanKey.startsWith('name:');
// Suppress repeats only for this peripheral. A rotated address must be
// probed again: neither a shared nor a cached name establishes identity.
// Cap retained keys so long scans cannot accumulate unbounded state.
retainCompletedKey = desktop?.platform === 'windows';
if (active && operation === this.#operation && retainCompletedKey && this.#scanKeys.size > 256) {
const oldest = [...this.#scanKeys].find((key) => ![...this.#scanDevices.values()].some((peer) => this.#scanKey(peer) === key));
if (oldest) this.#scanKeys.delete(oldest);
}
if (active && operation === this.#operation && desktop) onDesktop(desktop);
}).catch(() => undefined).finally(() => {
if (operation === this.#operation) {
this.#scanDevices.delete(device.id);
if (!retainCompletedKey) this.#scanKeys.delete(scanKey);
const next = waiting.values().next().value;
if (next) onAdvertisement(null, next);
}
});
this.#scanTasks.add(task);
void task.finally(() => this.#scanTasks.delete(task));
});
};
this.#managerOrCreate().startDeviceScan([BLE_UUIDS.service], null, onAdvertisement);
return () => {
if (!active) return;
active = false;
waiting.clear();
if (operation === this.#operation) this.#operation += 1;
this.#managerOrCreate().stopDeviceScan();
this.#cancelNativeOperations();
Expand All @@ -100,10 +112,12 @@ export class ReactNativeBleTransport implements BleTransport {
let active = true;
let claimedDeviceId: string | null = null;
let cancellation: Promise<void> | null = null;
const waiting = new Map<string, Device>();
const succeed = (desktop: DiscoveredDesktop) => {
if (!active) return;
this.#recordStage('resolution', 'succeeded', operation);
active = false;
waiting.clear();
clearTimeout(timer);
this.#managerOrCreate().stopDeviceScan();
this.#scanKeys.clear();
Expand All @@ -114,6 +128,7 @@ export class ReactNativeBleTransport implements BleTransport {
if (!active) return cancellation ?? Promise.resolve();
this.#recordStage('resolution', outcome, operation);
active = false;
waiting.clear();
clearTimeout(timer);
this.#managerOrCreate().stopDeviceScan();
const probes = [...this.#scanDevices.values()];
Expand All @@ -135,16 +150,22 @@ export class ReactNativeBleTransport implements BleTransport {
this.#resolutionCancel = cancel;
const timer = setTimeout(() => { void cancel(new Error('Saved PC discovery timed out.'), 'timed_out'); }, this.nativeTimeoutMs);
const onAdvertisement = (error: Error | null, device: Device | null) => {
if (!active || operation !== this.#operation) return;
if (!active || operation !== this.#operation || claimedDeviceId !== null) return;
if (error) { void cancel(new Error('Saved PC discovery failed.')); return; }
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device)) || this.#scanDevices.size >= 4) return;
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device))) return;
if (this.#scanDevices.size >= 4) {
if (waiting.size < 32) waiting.set(device.id, device);
return;
Comment on lines +157 to +158

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Full Queue Drops Targets

When four status probes are active and all 32 waiting slots are occupied, this branch discards the next advertisement instead of retaining it. A saved PC advertised only once in that state is never status-probed, so resolveAndConnect times out even though it received the PC's advertisement.

Knowledge Base Used: BLE transport adapter

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/transport/ReactNativeBleTransport.ts
Line: 157-158

Comment:
**Full Queue Drops Targets**

When four status probes are active and all 32 waiting slots are occupied, this branch discards the next advertisement instead of retaining it. A saved PC advertised only once in that state is never status-probed, so `resolveAndConnect` times out even though it received the PC's advertisement.

**Knowledge Base Used:** [BLE transport adapter](https://app.greptile.com/owen-mcgirr/-/custom-context/knowledge-base/switchifyapp/switchify-remote/-/docs/ble-transport.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

}
waiting.delete(device.id);
this.#scanDevices.set(device.id, device);
this.#scanKeys.add(this.#scanKey(device));
const task = this.#readStatus(device, (desktop) => {
if (!active || operation !== this.#operation) return false;
this.#recordStage('selected_match', desktop.desktopId === desktopId ? 'succeeded' : 'not_matched', operation);
if (desktop.desktopId !== desktopId || claimedDeviceId !== null) return false;
claimedDeviceId = device.id;
waiting.clear();
return true;
}).then(async (desktop) => {
if (!active || operation !== this.#operation || desktop?.desktopId !== desktopId || claimedDeviceId !== device.id) return;
Expand Down Expand Up @@ -176,6 +197,8 @@ export class ReactNativeBleTransport implements BleTransport {
if (operation === this.#operation) {
this.#scanDevices.delete(device.id);
this.#scanKeys.delete(this.#scanKey(device));
const next = waiting.values().next().value;
if (next) onAdvertisement(null, next);
}
});
this.#scanTasks.add(task);
Expand Down Expand Up @@ -474,8 +497,7 @@ export class ReactNativeBleTransport implements BleTransport {
}

#scanKey(device: Device): string {
const name = bluetoothDeviceDisplayName({ name: device.name, localName: device.localName }, this.platform);
return name ? `name:${name}` : `id:${device.id}`;
return `id:${device.id}`;
}

#bounded<T>(operation: Promise<T>, timeoutMs = this.nativeTimeoutMs): Promise<T> {
Expand Down