diff --git a/app/peripherals/PeripheralManager.js b/app/peripherals/PeripheralManager.js index ca29deae15..a5e67c1de3 100644 --- a/app/peripherals/PeripheralManager.js +++ b/app/peripherals/PeripheralManager.js @@ -109,7 +109,8 @@ export function createPeripheralManager (config) { setupPeripherals() async function setupPeripherals () { - // The order is important, starting with the BLEs causes EBUSY error on the HCI socket on switching. I was not able to find the cause - its probably the order within the async initialization of the BleManager, but cannot find a proper fix + // The order is important, starting with the BLEs causes EBUSY error on the HCI socket on switching. + // @ToDo: I was not able to find the cause - its probably the order within the async initialization of the BleManager, but cannot find a proper fix await createAntPeripheral(config.antPlusMode) await createHrmPeripheral(config.heartRateMode) await createBlePeripheral(config.bluetoothMode) @@ -123,7 +124,7 @@ export function createPeripheralManager (config) { * @param {unknown} data for executing the command * * @see {@link https://github.com/JaapvanEkris/openrowingmonitor/blob/main/docs/Architecture.md#command-flow|The command flow documentation} - */ + */ /* eslint-disable-next-line no-unused-vars -- data is irrelevant here, but it is a standardised interface */ async function handleCommand (commandName, data) { switch (commandName) { @@ -390,6 +391,8 @@ export function createPeripheralManager (config) { } if (hrmPeripheral && hrmMode.toLocaleLowerCase() !== 'OFF'.toLocaleLowerCase()) { + // Remove any existing heartRateMeasurement listeners before adding a new one to prevent memory leaks + hrmPeripheral.removeAllListeners('heartRateMeasurement') hrmPeripheral.on('heartRateMeasurement', (heartRateMeasurement) => { // Clear the HRM watchdog as new HRM data has been received clearTimeout(hrmWatchdogTimer) diff --git a/app/peripherals/ble/CpsPeripheral.js b/app/peripherals/ble/CpsPeripheral.js index bcc1574ffd..9b23ea57b6 100644 --- a/app/peripherals/ble/CpsPeripheral.js +++ b/app/peripherals/ble/CpsPeripheral.js @@ -90,7 +90,7 @@ export function createCpsPeripheral (bleManager, config) { log.debug('CPS pairing request rejected') }) - _connection.on('disconnect', async () => { + _connection.once('disconnect', async () => { log.debug(`CPS client disconnected (address: ${_connection?.peerAddress}), restarting advertising`) _connection = undefined await triggerAdvertising() diff --git a/app/peripherals/ble/Pm5Peripheral.js b/app/peripherals/ble/Pm5Peripheral.js index 4e7a8b970d..e2ff5aabad 100644 --- a/app/peripherals/ble/Pm5Peripheral.js +++ b/app/peripherals/ble/Pm5Peripheral.js @@ -106,6 +106,7 @@ export function createPm5Peripheral (bleManager, config, controlCallback) { function destroy () { log.debug('Shutting down PM5 peripheral') + rowingService.stop() if (_manager !== undefined) { gattServices.forEach((service) => { diff --git a/app/peripherals/ble/hrm/HrmService.js b/app/peripherals/ble/hrm/HrmService.js index 47a64b19b4..6ba1890bb0 100644 --- a/app/peripherals/ble/hrm/HrmService.js +++ b/app/peripherals/ble/hrm/HrmService.js @@ -69,17 +69,30 @@ export class HrmService extends EventEmitter { * @type {number | string | undefined} */ #serialNumber + /** + * Set to true when stop() is called so that any in-flight start() invocation + * exits cleanly and does not restart scanning. + */ + #stopped = false + /** + * Reject function for the pending scan Promise, allowing stop() to cancel + * a start() that is blocked waiting for a connectable advertisement. + * @type {((reason?: unknown) => void) | undefined} + */ + #scanReject /** * @param {import('../ble-host.interface.js').BleManager} manager */ - constructor (manager) { + constructor(manager) { super() this.#manager = manager } /* eslint-disable max-statements -- This initialises the BLE HRM handler */ - async start () { + async start() { + if (this.#stopped) { return } + this.#scanner = this.#manager.startScan({ scanFilters: [new BleManager.ServiceUUIDScanFilter(heartRateServiceUUID)] }) @@ -87,29 +100,47 @@ export class HrmService extends EventEmitter { this.#heartRateMeasurementCharacteristic?.removeAllListeners() this.#batteryLevelCharacteristic?.removeAllListeners() - const device = await new Promise((resolve) => { - /** @type {Scanner} */(this.#scanner).on('report', (eventData) => { - if (eventData.connectable) { - resolve(eventData) - } + let device + try { + device = await new Promise((resolve, reject) => { + this.#scanReject = reject + const activeScanner = /** @type {Scanner} */(this.#scanner) + activeScanner.on('report', (eventData) => { + if (eventData.connectable) { + resolve(eventData) + } + }) }) - }) + } catch { + return // stop() was called while scanning + } finally { + this.#scanReject = undefined + } log.info(`Found device (${device.parsedDataItems.localName || 'no name'})`) this.#scanner.removeAllListeners() this.#scanner.stopScan() + if (this.#stopped) { return } + this.#connection = await new Promise((/** @type {(value: Connection) => void} */resolve) => { this.#manager.connect(device.addressType, device.address, {}, (connection) => { resolve(connection) }) }) + if (this.#stopped) { + this.#connection.disconnect() + this.#connection = undefined + return + } + this.#connection.once('disconnect', () => { log.debug(`Disconnected from ${this.#connection?.peerAddress}, restart scanning`) - - this.start() + if (!this.#stopped) { + this.start() + } }) log.debug('Connected to ' + this.#connection.peerAddress) @@ -128,13 +159,15 @@ export class HrmService extends EventEmitter { }) }) - const deviceInformationService = primaryServices.find(service => service.uuid === deviceInformationServiceUUID) + if (this.#stopped) { return } + + const deviceInformationService = primaryServices.find((service) => service.uuid === deviceInformationServiceUUID) if (deviceInformationService !== undefined) { log.debug('HR device information service was discovered') const characteristics = await new Promise((/** @type {(value: { serialNumber?: GattClientCharacteristic, manufacturerId?: GattClientCharacteristic}) => void} */resolve) => { deviceInformationService.discoverCharacteristics((characteristics) => { resolve({ - serialNumber: characteristics.find(characteristic => characteristic.uuid === serialNumberUUID), manufacturerId: characteristics.find(characteristic => characteristic.uuid === manufacturerIdUUID) + serialNumber: characteristics.find((characteristic) => characteristic.uuid === serialNumberUUID), manufacturerId: characteristics.find((characteristic) => characteristic.uuid === manufacturerIdUUID) }) }) }) @@ -164,25 +197,25 @@ export class HrmService extends EventEmitter { }) } - const heartRateService = primaryServices.find(service => service.uuid === heartRateServiceUUID) + const heartRateService = primaryServices.find((service) => service.uuid === heartRateServiceUUID) if (heartRateService === undefined) { log.error(`Heart rate service not found in ${device.localName}`) - - this.start() + // Disconnect cleanly – the registered once('disconnect') handler will call this.start() + this.#connection?.disconnect() return } this.#heartRateMeasurementCharacteristic = await new Promise((resolve) => { heartRateService.discoverCharacteristics((characteristics) => { - resolve(characteristics.find(characteristic => characteristic.uuid === heartRateMeasurementUUID)) + resolve(characteristics.find((characteristic) => characteristic.uuid === heartRateMeasurementUUID)) }) }) if (this.#heartRateMeasurementCharacteristic === undefined) { log.error(`Heart rate measurement characteristic not found in ${device.localName}`) - - this.start() + // Disconnect cleanly – the registered once('disconnect') handler will call this.start() + this.#connection?.disconnect() return } @@ -193,7 +226,7 @@ export class HrmService extends EventEmitter { this.#onHeartRateNotify(value) }) - const batteryService = primaryServices.find(service => service.uuid === batteryLevelServiceUUID) + const batteryService = primaryServices.find((service) => service.uuid === batteryLevelServiceUUID) if (batteryService === undefined) { log.info(`Battery service not found in ${device.localName}`) @@ -202,7 +235,7 @@ export class HrmService extends EventEmitter { this.#batteryLevelCharacteristic = await new Promise((resolve) => { batteryService.discoverCharacteristics((characteristics) => { - resolve(characteristics.find(characteristic => characteristic.uuid === batteryLevelMeasurementUUID)) + resolve(characteristics.find((characteristic) => characteristic.uuid === batteryLevelMeasurementUUID)) }) }) @@ -228,9 +261,12 @@ export class HrmService extends EventEmitter { }) } - stop () { + stop() { + this.#stopped = true + this.#scanReject?.(new Error('HrmService stopped')) this.#batteryLevelCharacteristic?.removeAllListeners() this.#heartRateMeasurementCharacteristic?.removeAllListeners() + this.#scanner?.removeAllListeners() this.#scanner?.stopScan() return new Promise((/** @type {(value: void) => void} */resolve) => { log.debug('Shutting down HRM peripheral') @@ -249,7 +285,7 @@ export class HrmService extends EventEmitter { /** * @param {Buffer} data */ - #onHeartRateNotify (data) { + #onHeartRateNotify(data) { if (!Buffer.isBuffer(data) || data.length === 0) { log.error('Received invalid heart rate data, ignoring') @@ -305,7 +341,7 @@ export class HrmService extends EventEmitter { /** * @param {Buffer} data */ - #onBatteryNotify (data) { + #onBatteryNotify(data) { if (Buffer.isBuffer(data) && data.length > 0) { this.#batteryLevel = data.readUInt8(0) } diff --git a/app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js b/app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js index 4062ad68aa..381e413f8b 100644 --- a/app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js +++ b/app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js @@ -312,6 +312,10 @@ export class Pm5RowingService extends GattService { this.#genericStatusDataNotifies(this.#lastKnownMetrics, this.#previousSplitMetrics) } + stop () { + clearTimeout(this.#timer) + } + /** * @param {Metrics} metrics * @param {SplitTimeDistanceData} previousSplitMetrics