fix: ensure settings contains default and do not throw errors (#2328)

* fix: ensure settings contains default and do not throw errors * fix: json store tests * fix: ensure mqtt doesn't throw in gateway * fix: typo * fix: ensure mqtt is defined * fix: ensure driver is not inited when port is not defined * fix: catch possible error when driver is not enabled
zwave-js · Mar 22, 2022 · 010f22e · 010f22e
1 parent 777725f
commit 010f22e
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 68 deletions.
diff --git a/app.ts b/app.ts
@@ -10,7 +10,6 @@ import ZWaveClient, {
 	configManager,
 	loadManager,
 	SensorTypeScale,
-	deviceConfigPriorityDir,
 } from './lib/ZwaveClient'
 import MqttClient from './lib/MqttClient'
 import Gateway, { GatewayConfig } from './lib/Gateway'
@@ -38,7 +37,6 @@ import {
 	CustomPlugin,
 	PluginConstructor,
 } from './lib/CustomPlugin'
-import merge from 'merge'
 import { libVersion } from 'zwave-js'
 import { serverVersion } from '@zwave-js/server'
 
@@ -861,18 +859,9 @@ app.get(
 
 		const settings = jsonStore.get(store.settings)
 
-		const defaults = {
-			zwave: {
-				deviceConfigPriorityDir,
-				enableSoftReset: true,
-			},
-		}
-
-		const settingsWithDefaults = merge.recursive(defaults, settings)
-
 		const data = {
 			success: true,
-			settings: settingsWithDefaults,
+			settings,
 			devices: gw?.zwave?.devices ?? {},
 			serial_ports: [],
 			scales: scales,

diff --git a/config/store.ts b/config/store.ts
@@ -1,33 +1,41 @@
 // config/store.js
 
-import { GatewayConfig } from "../lib/Gateway";
-import { MqttConfig } from "../lib/MqttClient";
-import { ZwaveConfig } from "../lib/ZwaveClient";
+import { GatewayConfig } from '../lib/Gateway'
+import { MqttConfig } from '../lib/MqttClient'
+import { ZwaveConfig, deviceConfigPriorityDir } from '../lib/ZwaveClient'
 
-export type StoreKeys = "settings" | "scenes" | "nodes" | "users";
+export type StoreKeys = 'settings' | 'scenes' | 'nodes' | 'users'
 
 export interface StoreFile {
-  file: string; 
-  default: any
+	file: string
+	default: any
 }
 
 export interface User {
-  username: string,
-  passwordHash: string,
-  token?: string
+	username: string
+	passwordHash: string
+	token?: string
 }
 
 export interface Settings {
-  mqtt?: MqttConfig,
-  zwave?: ZwaveConfig,
-  gateway?: GatewayConfig
+	mqtt?: MqttConfig
+	zwave?: ZwaveConfig
+	gateway?: GatewayConfig
 }
 
 const store: Record<StoreKeys, StoreFile> = {
-  settings : { file: 'settings.json', default: {} },
-  scenes : { file: 'scenes.json', default: [] },
-  nodes : { file: 'nodes.json', default: [] },
-  users : { file: 'users.json', default: [] }
+	settings: {
+		file: 'settings.json',
+		default: {
+			zwave: {
+				deviceConfigPriorityDir,
+				enableSoftReset: true,
+			},
+		},
+	},
+	scenes: { file: 'scenes.json', default: [] },
+	nodes: { file: 'nodes.json', default: [] },
+	users: { file: 'users.json', default: [] },
 }
 
 export default store
diff --git a/lib/Gateway.ts b/lib/Gateway.ts
@@ -226,6 +226,10 @@ export default class Gateway {
 		return this._closed
 	}
 
+	private get mqttEnabled() {
+		return this.mqtt && !this.mqtt.disabled
+	}
+
 	constructor(config: GatewayConfig, zwave: ZwaveClient, mqtt: MqttClient) {
 		this.config = config || { type: 1 }
 		// clients
@@ -248,7 +252,7 @@ export default class Gateway {
 		// topic levels for subscribes using wildecards
 		this.topicLevels = []
 
-		if (this._mqtt && !this._mqtt.disabled) {
+		if (this.mqttEnabled) {
 			this._mqtt.on('writeRequest', this._onWriteRequest.bind(this))
 			this._mqtt.on('broadcastRequest', this._onBroadRequest.bind(this))
 			this._mqtt.on(
@@ -264,7 +268,7 @@ export default class Gateway {
 			// this is the only event we need to bind to in order to apply gateway values configs like polling
 			this._zwave.on('nodeInited', this._onNodeInited.bind(this))
 
-			if (!this._mqtt.disabled) {
+			if (this.mqttEnabled) {
 				this._zwave.on('nodeStatus', this._onNodeStatus.bind(this))
 				this._zwave.on('valueChanged', this._onValueChanged.bind(this))
 				this._zwave.on('nodeRemoved', this._onNodeRemoved.bind(this))
@@ -392,7 +396,7 @@ export default class Gateway {
 		}
 
 		// close mqtt client after zwave connection is closed
-		if (this._mqtt) {
+		if (this.mqttEnabled) {
 			await this._mqtt.close()
 		}
 	}
@@ -579,7 +583,7 @@ export default class Gateway {
 		options: { deleteDevice?: boolean; forceUpdate?: boolean } = {}
 	): void {
 		try {
-			if (this._mqtt.disabled || !this.config.hassDiscovery) {
+			if (!this.mqttEnabled || !this.config.hassDiscovery) {
 				logger.debug(
 					'Enable MQTT gateway and hass discovery to use this function'
 				)
@@ -684,7 +688,7 @@ export default class Gateway {
 	 * Discover an hass device (from customDevices.js|json)
 	 */
 	discoverDevice(node: Z2MNode, hassDevice: HassDevice): void {
-		if (this._mqtt.disabled || !this.config.hassDiscovery) {
+		if (!this.mqttEnabled || !this.config.hassDiscovery) {
 			logger.info(
 				'Enable MQTT gateway and hass discovery to use this function'
 			)
@@ -1099,7 +1103,7 @@ export default class Gateway {
 	 * Try to guess the best way to discover this valueId in Hass
 	 */
 	discoverValue(node: Z2MNode, vId: string): void {
-		if (this._mqtt.disabled || !this.config.hassDiscovery) {
+		if (!this.mqttEnabled || !this.config.hassDiscovery) {
 			logger.debug(
 				'Enable MQTT gateway and hass discovery to use this function'
 			)
@@ -1662,7 +1666,7 @@ export default class Gateway {
 	 * Removes all retained messages of the specified node
 	 */
 	removeNodeRetained(nodeId: number): void {
-		if (this._mqtt.disabled) {
+		if (!this.mqttEnabled) {
 			logger.info('Enable MQTT gateway to use this function')
 			return
 		}
@@ -1902,7 +1906,7 @@ export default class Gateway {
 			}
 		}
 
-		if (!this._mqtt.disabled && this.config.hassDiscovery) {
+		if (this.mqttEnabled && this.config.hassDiscovery) {
 			for (const id in node.hassDevices) {
 				if (node.hassDevices[id].persistent) {
 					this.publishDiscovery(node.hassDevices[id], node.id)
@@ -1928,7 +1932,7 @@ export default class Gateway {
 	 *
 	 */
 	private _onNodeStatus(node: Z2MNode): void {
-		if (this._mqtt.disabled) {
+		if (!this.mqttEnabled) {
 			return
 		}
 

diff --git a/lib/ZwaveClient.ts b/lib/ZwaveClient.ts
@@ -1167,6 +1167,11 @@ class ZwaveClient extends TypedEventEmitter<ZwaveClientEventCallbacks> {
 				return
 			}
 
+			if (!this.cfg?.port) {
+				logger.warn('Zwave driver not inited, no port configured')
+				return
+			}
+
 			// extend options with hidden `options`
 			const zwaveOptions: utils.DeepPartial<ZWaveOptions> = {
 				storage: {
@@ -1237,7 +1242,7 @@ class ZwaveClient extends TypedEventEmitter<ZwaveClientEventCallbacks> {
 			]
 
 			const envKeys = Object.keys(process.env)
-				.filter((k) => k.startsWith('KEY_'))
+				.filter((k) => k?.startsWith('KEY_'))
 				.map((k) => k.substring(4))
 
 			// load security keys from env
@@ -1600,24 +1605,34 @@ class ZwaveClient extends TypedEventEmitter<ZwaveClientEventCallbacks> {
 	 *
 	 */
 	enableStatistics() {
-		this._driver.enableStatistics({
-			applicationName:
-				pkgjson.name +
-				(this.cfg.serverEnabled ? ' / zwave-js-server' : ''),
-			applicationVersion: pkgjson.version,
-		})
+		if (this._driver) {
+			this._driver.enableStatistics({
+				applicationName:
+					pkgjson.name +
+					(this.cfg.serverEnabled ? ' / zwave-js-server' : ''),
+				applicationVersion: pkgjson.version,
+			})
+			logger.info('Zwavejs usage statistics ENABLED')
+		}
 
-		logger.info('Zwavejs usage statistics ENABLED')
+		logger.warn(
+			'Zwavejs driver is not ready yet, statistics will be enabled on driver initialization'
+		)
 	}
 
 	/**
 	 * Disable Statistics
 	 *
 	 */
 	disableStatistics() {
-		this._driver.disableStatistics()
+		if (this._driver) {
+			this._driver.disableStatistics()
+			logger.info('Zwavejs usage statistics DISABLED')
+		}
 
-		logger.info('Zwavejs usage statistics DISABLED')
+		logger.warn(
+			'Zwavejs driver is not ready yet, statistics will be disabled on driver initialization'
+		)
 	}
 
 	getInfo() {

diff --git a/lib/jsonStore.ts b/lib/jsonStore.ts
@@ -6,6 +6,7 @@ import { storeDir } from '../config/app'
 import { StoreFile, StoreKeys } from '../config/store'
 import { module } from './logger'
 import * as utils from './utils'
+import { recursive as merge } from 'merge'
 
 const logger = module('Store')
 
@@ -46,15 +47,18 @@ export class StorageHelper {
 
 		// ignore ENOENT error
 		if (err) {
-			if (err.code !== 'ENOENT') throw err
-			else {
+			if (err.code !== 'ENOENT') {
+				logger.error('Error reading file: ' + config.file, err)
+			} else {
 				logger.warn(`${config.file} not found`)
 			}
 		}
 
 		// replace data with default
 		if (!data) {
 			data = config.default
+		} else {
+			data = merge(config.default, data)
 		}
 
 		return { file: config.file, data: data }