From 7a8fa447bd2802f1564bef52e31eb1b2e89e2fd5 Mon Sep 17 00:00:00 2001 From: Heiko Rothe Date: Mon, 26 Oct 2020 23:06:57 +0100 Subject: [PATCH] fix(bluetooth): improve adapter state handling The async code is waited for where needed and the locks prevent an adapter from being re-locked. Closes #316, closes #302 --- package-lock.json | 9 ---- package.json | 1 - .../bluetooth/bluetooth.service.spec.ts | 52 +++++++++++-------- .../bluetooth/bluetooth.service.ts | 25 +++++---- tsconfig.json | 1 - 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/package-lock.json b/package-lock.json index ed4224f..d42598d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3020,15 +3020,6 @@ "integrity": "sha1-aaI6OtKcrwCX8G7aWbNh7i8GOfY=", "dev": true }, - "@types/noble": { - "version": "0.0.40", - "resolved": "https://registry.npmjs.org/@types/noble/-/noble-0.0.40.tgz", - "integrity": "sha512-14o/gk86uiJCCqQhcAeZsB228pHj1kVA47W6EaqIpQPt7y5IFzYSns3/rxC/nP9OqZFGOsgPwu4FTo9tlgnGTA==", - "dev": true, - "requires": { - "@types/node": "*" - } - }, "@types/node": { "version": "12.12.68", "resolved": "https://registry.npmjs.org/@types/node/-/node-12.12.68.tgz", diff --git a/package.json b/package.json index fb723e7..6a6839f 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,6 @@ "@types/lodash": "^4.14.162", "@types/mathjs": "^6.0.5", "@types/mdns": "0.0.33", - "@types/noble": "0.0.40", "@types/node": "^12.12.68", "@types/supertest": "^2.0.10", "@typescript-eslint/eslint-plugin": "^4.4.1", diff --git a/src/integrations/bluetooth/bluetooth.service.spec.ts b/src/integrations/bluetooth/bluetooth.service.spec.ts index 4c3185a..f84d178 100644 --- a/src/integrations/bluetooth/bluetooth.service.spec.ts +++ b/src/integrations/bluetooth/bluetooth.service.spec.ts @@ -2,7 +2,7 @@ const mockExec = jest.fn(); const mockNoble = { state: 'poweredOn', on: jest.fn(), - startScanning: jest.fn(), + startScanningAsync: jest.fn(), stopScanning: jest.fn(), }; jest.mock( @@ -46,35 +46,43 @@ describe('BluetoothService', () => { }); describe('Bluetooth Classic', () => { - it('should return measured RSSI value from command output', () => { + it('should return measured RSSI value from command output', async () => { mockExec.mockResolvedValue({ stdout: 'RSSI return value: -4' }); const address = '77:50:fb:4d:ab:70'; - expect(service.inquireClassicRssi(1, address)).resolves.toBe(-4); + expect(await service.inquireClassicRssi(1, address)).toBe(-4); expect(mockExec).toHaveBeenCalledWith( `hcitool -i hci1 cc \"${address}\" && hcitool -i hci1 rssi \"${address}\"`, expect.anything() ); }); - it('should return undefined if no RSSI could be determined', () => { + it('should return undefined if no RSSI could be determined', async () => { mockExec.mockResolvedValue({ stdout: "Can't create connection: Input/output error", stderr: 'Not connected.', }); expect( - service.inquireClassicRssi(0, '08:05:90:ed:3b:60') - ).resolves.toBeUndefined(); + await service.inquireClassicRssi(0, '08:05:90:ed:3b:60') + ).toBeUndefined(); }); - it('should return undefined if the command failed', () => { + it('should return undefined if the command failed', async () => { mockExec.mockRejectedValue({ message: 'Command failed' }); expect( - service.inquireClassicRssi(0, '08:05:90:ed:3b:60') - ).resolves.toBeUndefined(); + await service.inquireClassicRssi(0, '08:05:90:ed:3b:60') + ).toBeUndefined(); + }); + + it('should throw an exception if an inquiry is requested for a locked adapter', async () => { + await service.lockAdapter(1); + + await expect( + service.inquireClassicRssi(1, '77:50:fb:4d:ab:71') + ).rejects.toThrow(); }); it('should reset the HCI device if the query took too long', async () => { @@ -85,18 +93,18 @@ describe('BluetoothService', () => { expect(mockExec).toHaveBeenCalledWith('hciconfig hci1 reset'); }); - it('should stop scanning on an adapter while performing an inquiry', () => { + it('should stop scanning on an adapter while performing an inquiry', async () => { service.onLowEnergyDiscovery(() => undefined); const stateChangeHandler = mockNoble.on.mock.calls[0][1]; - stateChangeHandler('poweredOn'); + await stateChangeHandler('poweredOn'); - expect(mockNoble.startScanning).toHaveBeenCalledTimes(1); + expect(mockNoble.startScanningAsync).toHaveBeenCalledTimes(1); let execResolve; const execPromise = new Promise((r) => (execResolve = r)); mockExec.mockReturnValue(execPromise); const inquirePromise = service.inquireClassicRssi(0, 'x').then(() => { - expect(mockNoble.startScanning).toHaveBeenCalledTimes(2); + expect(mockNoble.startScanningAsync).toHaveBeenCalledTimes(2); }); expect(mockNoble.stopScanning).toHaveBeenCalledTimes(1); @@ -114,15 +122,15 @@ describe('BluetoothService', () => { mockExec.mockRejectedValue({ stderr: 'error' }); await service.inquireClassicRssi(0, 'x'); - expect(mockNoble.startScanning).toHaveBeenCalledTimes(2); + expect(mockNoble.startScanningAsync).toHaveBeenCalledTimes(2); }); - it('should stop scanning on an adapter while getting Classic device info', () => { + it('should stop scanning on an adapter while getting Classic device info', async () => { service.onLowEnergyDiscovery(() => undefined); const stateChangeHandler = mockNoble.on.mock.calls[0][1]; - stateChangeHandler('poweredOn'); + await stateChangeHandler('poweredOn'); - expect(mockNoble.startScanning).toHaveBeenCalledTimes(1); + expect(mockNoble.startScanningAsync).toHaveBeenCalledTimes(1); let execResolve; const execPromise = new Promise((r) => (execResolve = r)); @@ -130,7 +138,7 @@ describe('BluetoothService', () => { const inquirePromise = service .inquireClassicDeviceInfo(0, 'x') .then(() => { - expect(mockNoble.startScanning).toHaveBeenCalledTimes(2); + expect(mockNoble.startScanningAsync).toHaveBeenCalledTimes(2); }); expect(mockNoble.stopScanning).toHaveBeenCalledTimes(1); @@ -247,7 +255,7 @@ Requesting information ... stateChangeHandler('poweredOn'); - expect(mockNoble.startScanning).toHaveBeenCalledTimes(1); + expect(mockNoble.startScanningAsync).toHaveBeenCalledTimes(1); }); it('should not enable scanning when the adapter is performing a Classic inquiry', () => { @@ -258,12 +266,12 @@ Requesting information ... const execPromise = new Promise((r) => (execResolve = r)); mockExec.mockReturnValue(execPromise); const inquirePromise = service.inquireClassicRssi(0, 'x').then(() => { - expect(mockNoble.startScanning).toHaveBeenCalledTimes(1); + expect(mockNoble.startScanningAsync).toHaveBeenCalledTimes(1); }); stateChangeHandler('poweredOn'); - expect(mockNoble.startScanning).not.toHaveBeenCalled(); + expect(mockNoble.startScanningAsync).not.toHaveBeenCalled(); execResolve({ stdout: '-1' }); @@ -279,7 +287,7 @@ Requesting information ... mockExec.mockReturnValue(execPromise); await service.inquireClassicRssi(1, 'x'); - expect(mockNoble.startScanning).toHaveBeenCalledTimes(1); + expect(mockNoble.startScanningAsync).toHaveBeenCalledTimes(1); expect(mockNoble.stopScanning).not.toHaveBeenCalled(); }); }); diff --git a/src/integrations/bluetooth/bluetooth.service.ts b/src/integrations/bluetooth/bluetooth.service.ts index a61cbf7..be2a1e6 100644 --- a/src/integrations/bluetooth/bluetooth.service.ts +++ b/src/integrations/bluetooth/bluetooth.service.ts @@ -51,7 +51,7 @@ export class BluetoothService { adapterId: number, address: string ): Promise { - this.lockAdapter(adapterId); + await this.lockAdapter(adapterId); this.logger.debug(`Querying for RSSI of ${address} using hcitool`); try { @@ -105,7 +105,7 @@ export class BluetoothService { adapterId: number, address: string ): Promise { - this.lockAdapter(adapterId); + await this.lockAdapter(adapterId); try { const output = await execPromise( @@ -149,9 +149,14 @@ export class BluetoothService { * * @param adapterId - HCI Device ID of the adapter to lock */ - protected lockAdapter(adapterId: number): void { - if (this.adapterStates.get(adapterId) == 'scan') { - noble.stopScanning(); + async lockAdapter(adapterId: number): Promise { + switch (this.adapterStates.get(adapterId)) { + case 'inquiry': + throw new Error( + `Trying to lock adapter ${adapterId} even though it is already locked` + ); + case 'scan': + noble.stopScanning(); } this.adapterStates.set(adapterId, 'inquiry'); @@ -162,11 +167,11 @@ export class BluetoothService { * * @param adapterId - HCI Device ID of the adapter to unlock */ - protected unlockAdapter(adapterId: number): void { + async unlockAdapter(adapterId: number): Promise { this.adapterStates.set(adapterId, 'inactive'); if (adapterId == this.lowEnergyAdapterId) { - this.handleAdapterStateChange(noble.state); + await this.handleAdapterStateChange(noble.state); } } @@ -191,13 +196,13 @@ export class BluetoothService { * * @param state - State of the HCI adapter */ - private handleAdapterStateChange(state: string): void { + private async handleAdapterStateChange(state: string): Promise { if (this.adapterStates.get(this.lowEnergyAdapterId) != 'inquiry') { if (state === 'poweredOn') { - noble.startScanning([], true); + await noble.startScanningAsync([], true); this.adapterStates.set(this.lowEnergyAdapterId, 'scan'); } else { - noble.stopScanning(); + await noble.stopScanning(); this.adapterStates.set(this.lowEnergyAdapterId, 'inactive'); } } diff --git a/tsconfig.json b/tsconfig.json index 1f68ce8..0ae751b 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,7 +12,6 @@ "outDir": "./dist", "baseUrl": "./", "paths": { - "@abandonware/noble": ["node_modules/@types/noble"], "democracy": ["typings/democracy"] }, "incremental": true