Refactoring: Merge observer into device provider

src/device/knownDeviceRegistry.ts

@@ -0,0 +1,63 @@
+import Settings from '../settings/settings.js';
+import KnownDevice from '../settings/knownDevice.js';
+import DeviceNameGenerator from './deviceNameGenerator.js';
+import Logger from '../logging/Logger.js';
+import { DeviceId } from './deviceId.js';
+
+/**
+ * Looks up and registers the persisted `KnownDevice` identity for a newly detected raw device
+ * (serial port, BLE peripheral, buttplug.io device, ...).
+ *
+ * Centralizes identity lookup/creation logic that used to be duplicated across several device
+ * providers/factories. Deliberately has no opinion on *when* a newly created identity should be
+ * persisted - `resolve()` never has side effects, so callers stay in control of only calling
+ * `persist()` once they've actually finished building the Device successfully.
+ */
+export default class KnownDeviceRegistry
+{
+    private readonly settings: Settings;
+
+    private readonly nameGenerator: DeviceNameGenerator;
+
+    private readonly logger: Logger;
+
+    public constructor(settings: Settings, nameGenerator: DeviceNameGenerator, logger: Logger) {
+        this.settings = settings;
+        this.nameGenerator = nameGenerator;
+        this.logger = logger.child({ name: KnownDeviceRegistry.name });
+    }
+
+    /**
+     * Looks up the already-known identity for `deviceId`, or builds a new (not yet persisted)
+     * one if none exists.
+     */
+    public resolve(deviceId: DeviceId, type: string, provider: string, name?: string): KnownDevice {
+        const knownDevice = this.settings.getKnownDeviceById(deviceId);
+
+        if (undefined !== knownDevice) {
+            // Already known (previously detected serial number)
+            this.logger.debug(`Device is already known: ${knownDevice.id}`);
+            return knownDevice;
+        }
+
+        return new KnownDevice(deviceId, name ?? this.nameGenerator.generateName(), type, provider);
+    }
+
+    /**
+     * Persists a resolved identity. Safe to call unconditionally after successfully building a
+     * Device, even for an already-known identity - a no-op in that case, since KnownDevice is
+     * immutable and `resolve()` returns the exact same instance for an already-known device.
+     *
+     * This matters beyond just avoiding pointless work: Settings is wrapped with `on-change` to
+     * auto-save to disk, so an unconditional `settings.addKnownDevice()` call here would trigger a
+     * settings.json write and a settings-changed WebSocket broadcast on *every* device connect,
+     * even for a device that has been known and unchanged for months.
+     */
+    public persist(knownDevice: KnownDevice): void {
+        if (this.settings.getKnownDeviceById(knownDevice.id) === knownDevice) {
+            return;
+        }
+
+        this.settings.addKnownDevice(knownDevice);
+    }
+}

src/device/protocol/airotic/airoticDeviceProvider.ts

@@ -10,27 +10,32 @@ import AiroticProtocol from './airtonicProtocol.js';
 import MessageResponseHandler from '../messageResponseHandler.js';
 import StrDeviceAttribute from '../../attribute/strDeviceAttribute.js';
 import { DeviceAttributeModifier } from '../../attribute/deviceAttribute.js';
-import Settings from '../../../settings/settings.js';
-import KnownDevice from '../../../settings/knownDevice.js';
-import { DeviceId } from '../../deviceId.js';
+import KnownDeviceRegistry from '../../knownDeviceRegistry.js';
 import BoolDeviceAttribute from '../../attribute/boolDeviceAttribute.js';
 import FloatDeviceAttribute from '../../attribute/floatDeviceAttribute.js';
 import BleDeviceProvider from '../../provider/bleDeviceProvider.js';
 import { hsvByteToRgb } from '../../../util/color.js';
+import { NoDeviceProviderConfig } from '../../provider/deviceProviderConfig.js';
 
-export default class AiroticDeviceProvider extends BleDeviceProvider<AiroticDevice>
+export default class AiroticDeviceProvider extends BleDeviceProvider<AiroticDevice, NoDeviceProviderConfig>
 {
     public static readonly providerName = 'airotic';
 
     private static readonly UART_RX_CHAR_UUID = '6e400002b5a3f393e0a9e50e24dcca9e';
     private static readonly UART_TX_CHAR_UUID = '6e400003b5a3f393e0a9e50e24dcca9e';
 
-    private readonly settings: Settings;
+    private readonly knownDeviceRegistry: KnownDeviceRegistry;
 
-    public constructor(deviceManager: DeviceManager, settings: Settings, eventEmitter: EventEmitter, logger: Logger) {
-        super(deviceManager, eventEmitter, logger.child({ name: AiroticDeviceProvider.name }));
+    public constructor(
+        config: NoDeviceProviderConfig,
+        deviceManager: DeviceManager,
+        knownDeviceRegistry: KnownDeviceRegistry,
+        eventEmitter: EventEmitter,
+        logger: Logger
+    ) {
+        super(config, deviceManager, eventEmitter, logger.child({ name: AiroticDeviceProvider.name }));
 
-        this.settings = settings;
+        this.knownDeviceRegistry = knownDeviceRegistry;
     }
 
     public override async init(): Promise<void> {
@@ -56,8 +61,10 @@ export default class AiroticDeviceProvider extends BleDeviceProvider<AiroticDevi
             return undefined;
         }
 
-        const knownDevice = this.createKnownDevice(
+        const knownDevice = this.knownDeviceRegistry.resolve(
             deviceInfo.id,
+            'airotic',
+            AiroticDeviceProvider.providerName,
             deviceInfo.peripheral.advertisement.localName ?? `Airotic ${deviceInfo.id}`,
         );
 
@@ -85,7 +92,7 @@ export default class AiroticDeviceProvider extends BleDeviceProvider<AiroticDevi
             this.logger,
         );
 
-        this.settings.addKnownDevice(knownDevice);
+        this.knownDeviceRegistry.persist(knownDevice);
 
         return device;
     }
@@ -126,15 +133,4 @@ export default class AiroticDeviceProvider extends BleDeviceProvider<AiroticDevi
 
         return false;
     }
-
-    private createKnownDevice(deviceId: DeviceId, deviceName: string): KnownDevice {
-        const knownDevice = this.settings.getKnownDeviceById(deviceId);
-
-        if (undefined !== knownDevice) {
-            this.logger.debug(`Device is already known: ${knownDevice.id}`);
-            return knownDevice;
-        }
-
-        return new KnownDevice(deviceId, deviceName, 'airotic', AiroticDeviceProvider.providerName);
-    }
 }

src/device/protocol/buttplugIo/buttplugIoDeviceFactory.ts

@@ -1,7 +1,6 @@
-import Settings from '../../../settings/settings.js';
 import { ButtplugClientDevice } from 'buttplug';
 import ButtplugIoDevice, { ButtplugIoDeviceAttributeKey, ButtplugIoDeviceAttributes } from './buttplugIoDevice.js';
-import KnownDevice from '../../../settings/knownDevice.js';
+import KnownDeviceRegistry from '../../knownDeviceRegistry.js';
 import Logger from '../../../logging/Logger.js';
 import { DeviceAttributeModifier } from '../../attribute/deviceAttribute.js';
 import IntRangeDeviceAttribute from '../../attribute/intRangeDeviceAttribute.js';
@@ -17,22 +16,33 @@ export default class ButtplugIoDeviceFactory
 {
     private readonly dateFactory: DateFactory;
 
-    private readonly settings: Settings;
+    private readonly knownDeviceRegistry: KnownDeviceRegistry;
 
     private readonly logger: Logger;
 
     private readonly eventEmitterFactory: EventEmitterFactory;
 
-    public constructor(dateFactory: DateFactory, eventEmitterFactory: EventEmitterFactory, settings: Settings, logger: Logger) {
+    public constructor(dateFactory: DateFactory, eventEmitterFactory: EventEmitterFactory, knownDeviceRegistry: KnownDeviceRegistry, logger: Logger) {
         this.dateFactory = dateFactory;
         this.eventEmitterFactory = eventEmitterFactory;
 
-        this.settings = settings;
+        this.knownDeviceRegistry = knownDeviceRegistry;
         this.logger = logger;
     }
 
     public create(buttplugDevice: ButtplugClientDevice, provider: string, useDeviceNameAsId: boolean): ButtplugIoDevice {
-        const knownDevice = this.createKnownDevice(buttplugDevice, provider, useDeviceNameAsId);
+        // Since we don't get a unique identifier for the Bluetooth device from Intiface, we need
+        // to use the index assigned to the device by Intiface. It's the best we have, or the
+        // name if using Intiface-engine without id persistence.
+        const nameString = buttplugDevice.name.replace(/[^a-zA-Z0-9]/g, '');
+        const deviceId = DeviceId.create(useDeviceNameAsId ? `buttplugio-${nameString}` : `buttplugio-${buttplugDevice.index}`);
+
+        const knownDevice = this.knownDeviceRegistry.resolve(
+            deviceId,
+            buttplugDevice.name,
+            provider,
+            buttplugDevice.displayName ?? buttplugDevice.name,
+        );
 
         const deviceAttrs = ButtplugIoDeviceFactory.parseDeviceAttributes(buttplugDevice);
 
@@ -51,7 +61,7 @@ export default class ButtplugIoDeviceFactory
             throw new Error('Unknown device type: ' + knownDevice.name);
         }
 
-        this.settings.addKnownDevice(knownDevice);
+        this.knownDeviceRegistry.persist(knownDevice);
 
         return device;
     }
@@ -109,27 +119,4 @@ export default class ButtplugIoDeviceFactory
 
         return attributes;
     }
-
-    private createKnownDevice(buttplugDevice: ButtplugClientDevice, provider: string, useDeviceNameAsId: boolean): KnownDevice {
-        // Since we don't get a unique identifier for the Bluetooth device from Intiface,
-        // we need to use the index assigned to the device by Intiface. It's the best we have.
-        // or the name if using Intiface-engine without id persistence
-        const nameString = buttplugDevice.name.replace(/[^a-zA-Z0-9]/g, '');
-        const deviceId = DeviceId.create(useDeviceNameAsId ? `buttplugio-${nameString}` : `buttplugio-${buttplugDevice.index}`);
-
-        const knownDevice = this.settings.getKnownDeviceById(deviceId)
-
-        if (undefined !== knownDevice) {
-            // Return already existing device if already known (previously detected serial number)
-            this.logger.debug(`Device is already known: ${knownDevice.id}`);
-            return knownDevice;
-        }
-
-        return new KnownDevice(
-            deviceId,
-            buttplugDevice.displayName ?? buttplugDevice.name,
-            buttplugDevice.name,
-            provider
-        );
-    }
 }

src/device/protocol/buttplugIo/buttplugIoWebsocketConfig.ts

@@ -0,0 +1,12 @@
+import { Type, Static } from '@sinclair/typebox';
+
+export const buttplugIoWebsocketConfigSchema = Type.Object({
+    address: Type.String(),
+    autoScan: Type.Boolean(),
+    useDeviceNameAsId: Type.Boolean(),
+}, {
+    additionalProperties: false,
+});
+
+export type ButtplugIoWebsocketConfigSchema = typeof buttplugIoWebsocketConfigSchema;
+export type ButtplugIoWebsocketConfig = Static<ButtplugIoWebsocketConfigSchema>;

src/device/protocol/buttplugIo/buttplugIoWebsocketDeviceProvider.ts

@@ -9,40 +9,32 @@ import SlvCtrlPlusButtplugWebsocketClientConnector from './slvCtrlPlusButtplugWe
 import DeviceManager from '../../deviceManager.js';
 import { logError } from '../../../util/error.js';
 import { hasProperty } from '../../../util/objects.js';
+import { ButtplugIoWebsocketConfig } from './buttplugIoWebsocketConfig.js';
 
-export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider {
+export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider<ButtplugIoWebsocketConfig> {
     public static readonly providerName = 'buttplugIoWebsocket';
 
     private connectedDevices: Map<number, ButtplugIoDevice> = new Map();
 
-    private buttplugConnector: ButtplugNodeWebsocketClientConnector;
-    private buttplugClient: ButtplugClient;
+    private readonly buttplugConnector: ButtplugNodeWebsocketClientConnector;
+    private readonly buttplugClient: ButtplugClient;
 
     private readonly buttplugIoDeviceFactory: ButtplugIoDeviceFactory;
 
-    private readonly websocketAddress: string;
-    private readonly autoScan: boolean;
-    private readonly useDeviceNameAsId: boolean;
-
     private connectionIntervalRef?: NodeJS.Timeout;
     private autoScanningIntervalRef?: NodeJS.Timeout;
 
     public constructor(
+        config: ButtplugIoWebsocketConfig,
         deviceManager: DeviceManager,
         eventEmitter: EventEmitter,
         deviceFactory: ButtplugIoDeviceFactory,
-        websocketAddress: string,
-        autoScan: boolean,
-        useDeviceNameAsId: boolean,
         logger: Logger
     ) {
-        super(deviceManager, eventEmitter, logger.child({ name: ButtplugIoWebsocketDeviceProvider.name }));
+        super(config, deviceManager, eventEmitter, logger.child({ name: ButtplugIoWebsocketDeviceProvider.name }));
         this.buttplugIoDeviceFactory = deviceFactory;
-        this.websocketAddress = websocketAddress;
-        this.autoScan = autoScan;
-        this.useDeviceNameAsId = useDeviceNameAsId;
 
-        const url = `ws://${this.websocketAddress}/buttplug`;
+        const url = `ws://${this.config.address}/buttplug`;
 
         this.buttplugConnector = new SlvCtrlPlusButtplugWebsocketClientConnector(url);
         this.buttplugClient = new ButtplugClient('SlvCtrlPlus');
@@ -66,7 +58,7 @@ export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider {
             return;
         }
 
-        const url = `ws://${this.websocketAddress}/buttplug`;
+        const url = `ws://${this.config.address}/buttplug`;
 
         try {
             await this.buttplugClient.connect(this.buttplugConnector);
@@ -75,7 +67,7 @@ export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider {
             clearInterval(this.connectionIntervalRef);
             this.connectionIntervalRef = undefined;
 
-            if (this.autoScan) {
+            if (this.config.autoScan) {
                 this.autoScanningIntervalRef ??= setImmediateInterval(() => { this.discoverButtplugIoDevices() }, 60000);
             }
         } catch (e: unknown) {
@@ -108,7 +100,7 @@ export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider {
             .catch((e: unknown) => this.logger.error(`Could not start scanning for buttplug.io devices`, e));
 
         setTimeout(() => {
-            if (undefined === this.buttplugClient || !this.buttplugClient.isScanning) {
+            if (!this.buttplugClient.isScanning) {
                 return;
             }
 
@@ -122,7 +114,7 @@ export default class ButtplugIoWebsocketDeviceProvider extends DeviceProvider {
         this.logger.info(`Device detected: ${buttplugDevice.name}`, buttplugDevice);
 
         try {
-            const device = this.buttplugIoDeviceFactory.create(buttplugDevice, ButtplugIoWebsocketDeviceProvider.providerName, this.useDeviceNameAsId);
+            const device = this.buttplugIoDeviceFactory.create(buttplugDevice, ButtplugIoWebsocketDeviceProvider.providerName, this.config.useDeviceNameAsId);
 
             this.connectedDevices.set(buttplugDevice.index, device);
 

src/device/protocol/estim2b/estim2bSerialDeviceProvider.ts

@@ -13,8 +13,9 @@ import SerialDeviceTransportFactory from '../../transport/serialDeviceTransportF
 import { getErrorFromDecodeResult } from '../deviceProtocol.js';
 import DeviceManager from '../../deviceManager.js';
 import { SerialDeviceInfo } from '../../transport/serialPortObserver.js';
+import { NoDeviceProviderConfig } from '../../provider/deviceProviderConfig.js';
 
-export default class EStim2bSerialDeviceProvider extends SerialDeviceProvider<Estim2bDevice>
+export default class EStim2bSerialDeviceProvider extends SerialDeviceProvider<Estim2bDevice, NoDeviceProviderConfig>
 {
     public static readonly providerName = 'estim2bSerial';
 
@@ -23,14 +24,15 @@ export default class EStim2bSerialDeviceProvider extends SerialDeviceProvider<Es
     private readonly deviceFactory: EStim2bDeviceFactory;
 
     public constructor(
+        config: NoDeviceProviderConfig,
         deviceManager: DeviceManager,
         serialPortFactory: SerialPortFactory,
         transportFactory: SerialDeviceTransportFactory,
         eventEmitter: EventEmitter,
         deviceFactory: EStim2bDeviceFactory,
         logger: Logger
     ) {
-        super(deviceManager, serialPortFactory, eventEmitter, logger.child({ name: EStim2bSerialDeviceProvider.name }));
+        super(config, deviceManager, serialPortFactory, eventEmitter, logger.child({ name: EStim2bSerialDeviceProvider.name }));
 
         this.transportFactory = transportFactory;
         this.deviceFactory = deviceFactory;