Refactor settings system with type-safe configuration and validation

app/enervent.ts

@@ -24,23 +24,49 @@ export const MUTUALLY_EXCLUSIVE_MODES: Record<string, number> = {
     'eco': 40,
 }
 
-export const AVAILABLE_SETTINGS: Record<string, number> = {
-    'overPressureDelay': 57,
-    'awayVentilationLevel': 100,
-    'awayTemperatureReduction': 101,
-    'longAwayVentilationLevel': 102,
-    'longAwayTemperatureReduction': 103,
-    'temperatureControlMode': 136,
-    'temperatureTarget': 135,
-    'coolingAllowed': 52,
-    'heatingAllowed': 54,
-    'awayCoolingAllowed': 19,
-    'awayHeatingAllowed': 18,
-    'longAwayCoolingAllowed': 21,
-    'longAwayHeatingAllowed': 20,
-    'defrostingAllowed': 55,
-    'supplyFanOverPressure': 54,
-    'exhaustFanOverPressure': 55,
+interface BaseSettingConfiguration {
+    dataAddress: number
+    registerType: 'coil' | 'holding'
+}
+
+export interface CoilSettingConfiguration extends BaseSettingConfiguration {
+    registerType: 'coil'
+}
+
+export interface HoldingRegisterSettingConfiguration extends BaseSettingConfiguration {
+    registerType: 'holding'
+    decimals: number
+    registerScale?: number
+    min?: number
+    max?: number
+}
+
+export type SettingConfiguration = CoilSettingConfiguration | HoldingRegisterSettingConfiguration
+
+export const AVAILABLE_SETTINGS: Record<string, SettingConfiguration> = {
+    'overPressureDelay': { dataAddress: 57, decimals: 0, registerType: 'holding', min: 0, max: 60 },
+    'awayVentilationLevel': { dataAddress: 100, decimals: 0, registerType: 'holding', min: 20, max: 100 },
+    'awayTemperatureReduction': { dataAddress: 101, decimals: 0, registerType: 'holding', registerScale: 10 },
+    'longAwayVentilationLevel': { dataAddress: 102, decimals: 0, registerType: 'holding', min: 20, max: 100 },
+    'longAwayTemperatureReduction': { dataAddress: 103, decimals: 0, registerType: 'holding', registerScale: 10 },
+    'temperatureControlMode': { dataAddress: 136, decimals: 0, registerType: 'holding' },
+    'temperatureTarget': {
+        dataAddress: 135,
+        decimals: 1,
+        registerType: 'holding',
+        registerScale: 10,
+        min: 10,
+        max: 30,
+    },
+    'coolingAllowed': { dataAddress: 52, registerType: 'coil' },
+    'heatingAllowed': { dataAddress: 54, registerType: 'coil' },
+    'awayCoolingAllowed': { dataAddress: 19, registerType: 'coil' },
+    'awayHeatingAllowed': { dataAddress: 18, registerType: 'coil' },
+    'longAwayCoolingAllowed': { dataAddress: 21, registerType: 'coil' },
+    'longAwayHeatingAllowed': { dataAddress: 20, registerType: 'coil' },
+    'defrostingAllowed': { dataAddress: 55, registerType: 'coil' },
+    'supplyFanOverPressure': { dataAddress: 54, decimals: 0, registerType: 'holding', min: 20, max: 100 },
+    'exhaustFanOverPressure': { dataAddress: 55, decimals: 0, registerType: 'holding', min: 20, max: 100 },
 }
 
 export enum TemperatureControlState {

app/modbus.ts

@@ -24,6 +24,8 @@ import {
     AlarmStatus,
     HeatingType,
     TemperatureControlState,
+    CoilSettingConfiguration,
+    HoldingRegisterSettingConfiguration,
 } from './enervent'
 import ModbusRTU from 'modbus-serial'
 import { ReadCoilResult, ReadRegisterResult } from 'modbus-serial/ModbusRTU'
@@ -277,71 +279,65 @@ export const getSettings = async (modbusClient: ModbusRTU): Promise<Settings> =>
     return settings as Settings
 }
 
-export const setSetting = async (modbusClient: ModbusRTU, setting: string, value: string | boolean) => {
-    const dataAddress = AVAILABLE_SETTINGS[setting]
-    if (dataAddress === undefined) {
-        throw new Error('Unknown setting')
-    }
-
-    switch (typeof value) {
-        case 'string':
-            return setIntegerSetting(modbusClient, setting, parseInt(value, 10))
-        case 'boolean':
-            return setBooleanSetting(modbusClient, setting, value)
-    }
-}
-
-const setBooleanSetting = async (modbusClient: ModbusRTU, setting: string, boolValue: boolean) => {
-    const dataAddress = AVAILABLE_SETTINGS[setting]
-    if (dataAddress === undefined) {
-        throw new Error('Unknown setting')
+const parseSettingValue = (settingConfig: HoldingRegisterSettingConfiguration, value: string): number => {
+    if (settingConfig.decimals > 0) {
+        const multiplier = Math.pow(10, settingConfig.decimals)
+        return Math.round(parseFloat(value) * multiplier) / multiplier
+    } else {
+        return parseInt(value, 10)
     }
-
-    await mutex.runExclusive(async () => tryWriteCoil(modbusClient, dataAddress, boolValue))
 }
 
-const setIntegerSetting = async (modbusClient: ModbusRTU, setting: string, intValue: number) => {
-    const dataAddress = AVAILABLE_SETTINGS[setting]
-    if (dataAddress === undefined) {
+export const setSetting = async (modbusClient: ModbusRTU, setting: string, value: string | boolean) => {
+    const settingConfig = AVAILABLE_SETTINGS[setting]
+    if (settingConfig === undefined) {
         throw new Error('Unknown setting')
     }
 
-    switch (setting) {
-        case 'awayVentilationLevel':
-        case 'longAwayVentilationLevel':
-            if (intValue < 20 || intValue > 100) {
-                throw new RangeError('level out of range')
+    switch (settingConfig.registerType) {
+        case 'holding': {
+            // Holding registers expect numeric (string) values
+            if (typeof value !== 'string') {
+                throw new TypeError(`Setting '${setting}' expects a numeric value, got ${typeof value}`)
             }
-
-            break
-        case 'temperatureTarget':
-            if (intValue < 10 || intValue > 30) {
-                throw new RangeError('temperature out of range')
-            }
-
-            intValue *= 10
-            break
-        case 'overPressureDelay':
-            if (intValue < 0 || intValue > 60) {
-                throw new RangeError('delay out of range')
+            const numericValue = parseSettingValue(settingConfig, value)
+            return setNumericSetting(modbusClient, settingConfig, numericValue)
+        }
+        case 'coil': {
+            // Coils expect boolean values
+            if (typeof value !== 'boolean') {
+                throw new TypeError(`Setting '${setting}' expects a boolean value, got ${typeof value}`)
             }
+            return setBooleanSetting(modbusClient, settingConfig, value)
+        }
+    }
+}
 
-            break
-        case 'awayTemperatureReduction':
-        case 'longAwayTemperatureReduction':
-            // No minimum/maximum values specified in the register documentation
-            intValue *= 10
-            break
-        case 'supplyFanOverPressure':
-        case 'exhaustFanOverPressure':
-            if (intValue < 20 || intValue > 100) {
-                throw new RangeError('level out of range')
-            }
+const setBooleanSetting = async (
+    modbusClient: ModbusRTU,
+    settingConfig: CoilSettingConfiguration,
+    boolValue: boolean
+) => {
+    await mutex.runExclusive(async () => tryWriteCoil(modbusClient, settingConfig.dataAddress, boolValue))
+}
 
-            break
+const setNumericSetting = async (
+    modbusClient: ModbusRTU,
+    settingConfig: HoldingRegisterSettingConfiguration,
+    numericValue: number
+) => {
+    // Validate against min/max if specified
+    if (settingConfig.min !== undefined && numericValue < settingConfig.min) {
+        throw new RangeError(`value ${numericValue} below minimum ${settingConfig.min}`)
     }
+    if (settingConfig.max !== undefined && numericValue > settingConfig.max) {
+        throw new RangeError(`value ${numericValue} above maximum ${settingConfig.max}`)
+    }
+
+    // Apply register scaling (default 1 = no scaling)
+    const scaledValue = numericValue * (settingConfig.registerScale ?? 1)
 
-    await mutex.runExclusive(async () => tryWriteHoldingRegister(modbusClient, dataAddress, intValue))
+    await mutex.runExclusive(async () => tryWriteHoldingRegister(modbusClient, settingConfig.dataAddress, scaledValue))
 }
 
 export const getDeviceInformation = async (modbusClient: ModbusRTU): Promise<DeviceInformation> => {

tests/modbus.test.ts

@@ -1,4 +1,5 @@
-import { validateDevice, parseDevice, ModbusDeviceType } from '../app/modbus'
+import { validateDevice, parseDevice, ModbusDeviceType, setSetting } from '../app/modbus'
+import ModbusRTU from 'modbus-serial'
 
 test('validateDevice', () => {
     expect(validateDevice('/dev/ttyUSB0')).toEqual(true)
@@ -23,3 +24,106 @@ test('parseDevice', () => {
         port: 502,
     })
 })
+
+describe('setSetting', () => {
+    let mockClient: ModbusRTU
+
+    beforeEach(() => {
+        mockClient = {
+            writeRegister: jest.fn().mockResolvedValue(undefined),
+            writeCoil: jest.fn().mockResolvedValue(undefined),
+        } as any
+    })
+
+    describe('holding register settings (numeric)', () => {
+        test('should accept string values for numeric settings', async () => {
+            await setSetting(mockClient, 'temperatureTarget', '22.5')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(135, 225) // 22.5 * 10
+        })
+
+        test('should parse and round decimal values correctly', async () => {
+            await setSetting(mockClient, 'temperatureTarget', '22.0')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(135, 220)
+
+            await setSetting(mockClient, 'temperatureTarget', '18.75')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(135, 188) // rounds to 18.8 * 10
+
+            await setSetting(mockClient, 'temperatureTarget', '18.74')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(135, 187) // rounds to 18.7 * 10
+        })
+
+        test('should parse integer strings for settings without decimals', async () => {
+            await setSetting(mockClient, 'awayVentilationLevel', '50')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(100, 50)
+
+            await setSetting(mockClient, 'overPressureDelay', '30')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(57, 30)
+        })
+
+        test('should truncate decimals for integer-only settings', async () => {
+            await setSetting(mockClient, 'awayVentilationLevel', '50.5')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(100, 50) // truncates to 50
+        })
+
+        test('should reject boolean values for numeric settings', async () => {
+            await expect(setSetting(mockClient, 'temperatureTarget', true)).rejects.toThrow(
+                "Setting 'temperatureTarget' expects a numeric value, got boolean"
+            )
+        })
+
+        test('should apply registerScale when set', async () => {
+            await setSetting(mockClient, 'awayTemperatureReduction', '5')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(101, 50) // 5 * 10
+        })
+
+        test('should not scale when registerScale is not set', async () => {
+            await setSetting(mockClient, 'awayVentilationLevel', '75')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(100, 75) // no scaling
+        })
+
+        test('should enforce min/max validation', async () => {
+            await expect(setSetting(mockClient, 'temperatureTarget', '5')).rejects.toThrow('value 5 below minimum 10')
+            await expect(setSetting(mockClient, 'temperatureTarget', '35')).rejects.toThrow('value 35 above maximum 30')
+        })
+
+        test('should allow values within min/max range', async () => {
+            await setSetting(mockClient, 'temperatureTarget', '20')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(135, 200) // 20 * 10
+        })
+
+        test('should accept boundary values for min/max', async () => {
+            await setSetting(mockClient, 'temperatureTarget', '10')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(135, 100) // 10 * 10
+
+            await setSetting(mockClient, 'temperatureTarget', '30')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(135, 300) // 30 * 10
+        })
+
+        test('should handle settings without min/max', async () => {
+            await setSetting(mockClient, 'temperatureControlMode', '2')
+            expect(mockClient.writeRegister).toHaveBeenCalledWith(136, 2) // no validation, no scaling
+        })
+    })
+
+    describe('coil settings (boolean)', () => {
+        test('should accept boolean values for coil settings', async () => {
+            await setSetting(mockClient, 'coolingAllowed', true)
+            expect(mockClient.writeCoil).toHaveBeenCalledWith(52, true)
+
+            await setSetting(mockClient, 'heatingAllowed', false)
+            expect(mockClient.writeCoil).toHaveBeenCalledWith(54, false)
+        })
+
+        test('should reject string values for coil settings', async () => {
+            await expect(setSetting(mockClient, 'coolingAllowed', '1')).rejects.toThrow(
+                "Setting 'coolingAllowed' expects a boolean value, got string"
+            )
+        })
+    })
+
+    describe('unknown settings', () => {
+        test('should reject unknown setting names', async () => {
+            await expect(setSetting(mockClient, 'nonExistentSetting', '123')).rejects.toThrow('Unknown setting')
+        })
+    })
+})