From 0329e759d80a5fc61771ec014f42ddb55103f809 Mon Sep 17 00:00:00 2001 From: Chris ter Beke <1134120+ChrisTerBeke@users.noreply.github.com> Date: Wed, 22 May 2024 21:03:59 +0200 Subject: [PATCH] Fix: network crashes (#9) * Clean up unused properties docs, moved to a GH issue * Try/catch around all device calls that use the network * Add debug functionality, improve IP change handling * Bump version to v0.0.4 Changelog: Fix crashes caused by network errors, add debug function --- .homeychangelog.json | 3 + .homeycompose/app.json | 2 +- app.json | 35 ++++++++++- drivers/uponor/device.ts | 93 ++++++++++++++++++++---------- drivers/uponor/driver.compose.json | 35 ++++++++++- drivers/uponor/driver.ts | 44 +++++++------- lib/UponorHTTPClient.ts | 53 +++++------------ 7 files changed, 168 insertions(+), 97 deletions(-) diff --git a/.homeychangelog.json b/.homeychangelog.json index b5a7757..753ae07 100644 --- a/.homeychangelog.json +++ b/.homeychangelog.json @@ -7,5 +7,8 @@ }, "0.0.3": { "en": "Fixed crashes caused by network issues" + }, + "0.0.4": { + "en": "Fix crashes caused by network errors, add debug function" } } diff --git a/.homeycompose/app.json b/.homeycompose/app.json index fbbaf84..b5b287c 100644 --- a/.homeycompose/app.json +++ b/.homeycompose/app.json @@ -1,6 +1,6 @@ { "id": "com.christerbeke.uponor-smatrix", - "version": "0.0.3", + "version": "0.0.4", "compatibility": ">=5.0.0", "sdk": 3, "platforms": [ diff --git a/app.json b/app.json index b034a23..8703fd6 100644 --- a/app.json +++ b/app.json @@ -1,7 +1,7 @@ { "_comment": "This file is generated. Please edit .homeycompose/app.json instead.", "id": "com.christerbeke.uponor-smatrix", - "version": "0.0.3", + "version": "0.0.4", "compatibility": ">=5.0.0", "sdk": 3, "platforms": [ @@ -82,13 +82,42 @@ { "id": "address", "label": { - "en": "Override IP Address" + "en": "Override IP Address", + "nl": "IP-adres overschrijven" }, "type": "text", "required": false, "hint": { - "en": "Overrides the auto-discovered IP address of the device." + "en": "Overrides the auto-discovered IP address of the device.", + "nl": "Overschrijft het automatisch ontdekte IP-adres van het apparaat." } + }, + { + "type": "group", + "label": { + "en": "Troubleshooting", + "nl": "Probleemoplossing" + }, + "children": [ + { + "id": "debugEnabled", + "type": "checkbox", + "label": { + "en": "Enable debug data", + "nl": "Schakel probleemverhelping in" + }, + "value": false + }, + { + "id": "apiData", + "type": "textarea", + "label": { + "en": "Last API response", + "nl": "Laatste API-reactie" + }, + "value": "{}" + } + ] } ] } diff --git a/drivers/uponor/device.ts b/drivers/uponor/device.ts index 83c1a14..f8e6b9b 100644 --- a/drivers/uponor/device.ts +++ b/drivers/uponor/device.ts @@ -1,6 +1,6 @@ import { isIPv4 } from 'net' -import { Device, DiscoveryResult } from 'homey' -import { UponorHTTPClient, Mode } from '../../lib/UponorHTTPClient' +import { Device, DiscoveryResultMAC } from 'homey' +import { UponorHTTPClient } from '../../lib/UponorHTTPClient' const POLL_INTERVAL_MS = 1000 * 60 * 1 @@ -22,20 +22,20 @@ class UponorThermostatDevice extends Device { this._uninit() } - onDiscoveryResult(discoveryResult: DiscoveryResult): boolean { + onDiscoveryResult(discoveryResult: DiscoveryResultMAC): boolean { return this.getData().id.includes(discoveryResult.id) } - async onDiscoveryAvailable(discoveryResult: DiscoveryResult): Promise { - this._updateAddress(discoveryResult.id) + async onDiscoveryAvailable(discoveryResult: DiscoveryResultMAC): Promise { + this._updateAddress(discoveryResult.address, true) } - async onDiscoveryAddressChanged(discoveryResult: DiscoveryResult): Promise { - this._updateAddress(discoveryResult.id) + async onDiscoveryAddressChanged(discoveryResult: DiscoveryResultMAC): Promise { + this._updateAddress(discoveryResult.address, true) } - async onDiscoveryLastSeenChanged(discoveryResult: DiscoveryResult): Promise { - this._updateAddress(discoveryResult.id) + async onDiscoveryLastSeenChanged(discoveryResult: DiscoveryResultMAC): Promise { + this._updateAddress(discoveryResult.address, true) } async onSettings({ newSettings }: { newSettings: { [key: string]: any } }): Promise { @@ -51,21 +51,29 @@ class UponorThermostatDevice extends Device { private _getAddress(): string | undefined { const settingAddress = this.getSetting('address') - if (settingAddress) return settingAddress + if (settingAddress && settingAddress.length > 0) return settingAddress const storeAddress = this.getStoreValue('address') - if (storeAddress) return storeAddress + if (storeAddress && storeAddress.length > 0) return storeAddress return undefined } - private async _updateAddress(newAddress: string): Promise { - if (newAddress.length > 0) { + private async _updateAddress(newAddress: string, persist = false): Promise { + if (newAddress && newAddress.length > 0) { const isValidIP = isIPv4(newAddress) if (!isValidIP) return false const client = new UponorHTTPClient(newAddress) - const canConnect = await client.testConnection() - if (!canConnect) return false + try { + const canConnect = await client.testConnection() + if (!canConnect) return false + } catch (error) { + return false + } + } + + if (persist) { + this.setStoreValue('address', newAddress) } - this.setStoreValue('address', newAddress) + this._init() return true } @@ -74,12 +82,17 @@ class UponorThermostatDevice extends Device { await this._uninit() const address = this._getAddress() if (!address) return this.setUnavailable('No IP address configured') - const client = new UponorHTTPClient(address) - const canConnect = await client.testConnection() - if (!canConnect) return this.setUnavailable(`Could not connect to Uponor controller on IP address ${address}`) - this._client = client - this._syncInterval = setInterval(this._syncAttributes.bind(this), POLL_INTERVAL_MS) - this._syncAttributes() + + try { + const client = new UponorHTTPClient(address) + const canConnect = await client.testConnection() + if (!canConnect) return this.setUnavailable(`Could not connect to Uponor controller on IP address ${address}`) + this._client = client + this._syncInterval = setInterval(this._syncAttributes.bind(this), POLL_INTERVAL_MS) + this._syncAttributes() + } catch (error) { + this.setUnavailable(`Could not connect to Uponor controller on IP address ${address}`) + } } async _uninit(): Promise { @@ -91,19 +104,37 @@ class UponorThermostatDevice extends Device { private async _syncAttributes(): Promise { if (!this._client) return this.setUnavailable('No Uponor client') - await this._client.syncAttributes() - const { controllerID, thermostatID } = this.getData() - const data = this._client.getThermostat(controllerID, thermostatID) - if (!data) return this.setUnavailable('Could not find thermostat data') - this.setAvailable() - this.setCapabilityValue('measure_temperature', data.temperature) - this.setCapabilityValue('target_temperature', data.setPoint) + + try { + await this._client.syncAttributes() + const { controllerID, thermostatID } = this.getData() + const data = this._client.getThermostat(controllerID, thermostatID) + if (!data) return this.setUnavailable('Could not find thermostat data') + this.setAvailable() + this.setCapabilityValue('measure_temperature', data.temperature) + this.setCapabilityValue('target_temperature', data.setPoint) + } catch (error) { + this.setUnavailable('Could not fetch data from Uponor controller') + } + + try { + const { debugEnabled } = this.getSettings() + if (!debugEnabled) return + const debug = await this._client.debug() + this.setSettings({ apiData: JSON.stringify(debug) }) + } catch (error) { } } private async _setTargetTemperature(value: number): Promise { if (!this._client) return - const { controllerID, thermostatID } = this.getData() - await this._client.setTargetTemperature(controllerID, thermostatID, value) + + try { + const { controllerID, thermostatID } = this.getData() + await this._client.setTargetTemperature(controllerID, thermostatID, value) + } catch (error) { + this.setUnavailable('Could not send data to Uponor controller') + } + await this._syncAttributes() } } diff --git a/drivers/uponor/driver.compose.json b/drivers/uponor/driver.compose.json index d69d6bb..b572fae 100644 --- a/drivers/uponor/driver.compose.json +++ b/drivers/uponor/driver.compose.json @@ -45,13 +45,42 @@ { "id": "address", "label": { - "en": "Override IP Address" + "en": "Override IP Address", + "nl": "IP-adres overschrijven" }, "type": "text", "required": false, "hint": { - "en": "Overrides the auto-discovered IP address of the device." + "en": "Overrides the auto-discovered IP address of the device.", + "nl": "Overschrijft het automatisch ontdekte IP-adres van het apparaat." } + }, + { + "type": "group", + "label": { + "en": "Troubleshooting", + "nl": "Probleemoplossing" + }, + "children": [ + { + "id": "debugEnabled", + "type": "checkbox", + "label": { + "en": "Enable debug data", + "nl": "Schakel probleemverhelping in" + }, + "value": false + }, + { + "id": "apiData", + "type": "textarea", + "label": { + "en": "Last API response", + "nl": "Laatste API-reactie" + }, + "value": "{}" + } + ] } ] -} \ No newline at end of file +} diff --git a/drivers/uponor/driver.ts b/drivers/uponor/driver.ts index 3dfc556..96c83f2 100644 --- a/drivers/uponor/driver.ts +++ b/drivers/uponor/driver.ts @@ -1,5 +1,5 @@ import { Driver } from 'homey' -import { UponorHTTPClient } from '../../lib/UponorHTTPClient' +import { Thermostat, UponorHTTPClient } from '../../lib/UponorHTTPClient' import { PairSession } from 'homey/lib/Driver'; class UponorDriver extends Driver { @@ -32,27 +32,31 @@ class UponorDriver extends Driver { } private async _findDevices(ipAddress: string, systemID: string): Promise { - const devices: any[] = [] const client = new UponorHTTPClient(ipAddress) - const connected = await client.testConnection() - if (!connected) return devices - - await client.syncAttributes() - client.getThermostats().forEach((thermostat) => { - devices.push({ - name: thermostat.name, - data: { - id: `${systemID}_${thermostat.id}`, - controllerID: thermostat.controllerID, - thermostatID: thermostat.thermostatID, - }, - store: { - address: ipAddress, - } - }) - }) - return devices + try { + const connected = await client.testConnection() + if (!connected) return [] + await client.syncAttributes() + const thermostats = Array.from(client.getThermostats().values()) + return thermostats.map(this._mapDevice.bind(this, ipAddress, systemID)) + } catch (error) { + return [] + } + } + + private _mapDevice(ipAddress: string, systemID: string, thermostat: Thermostat): any { + return { + name: thermostat.name, + data: { + id: `${systemID}_${thermostat.id}`, + controllerID: thermostat.controllerID, + thermostatID: thermostat.thermostatID, + }, + store: { + address: ipAddress, + } + } } } diff --git a/lib/UponorHTTPClient.ts b/lib/UponorHTTPClient.ts index dd37586..f4da6a4 100644 --- a/lib/UponorHTTPClient.ts +++ b/lib/UponorHTTPClient.ts @@ -57,6 +57,19 @@ export class UponorHTTPClient { this._thermostats = this._syncThermostats() } + public async debug(): Promise { + try { + const request = await fetch(this._url, { + method: 'POST', + headers: { 'x-jnap-action': 'http://phyn.com/jnap/uponorsky/GetAttributes' }, + body: '{}', + }) + return await request.json() + } catch (error) { + return false + } + } + public async testConnection(): Promise { try { const request = await fetch(this._url, { @@ -97,45 +110,7 @@ export class UponorHTTPClient { const controllerID = matches[1] // first capture group const thermostatID = matches[2] // second capture group const ctKey = UponorHTTPClient._createKey(controllerID, thermostatID) - - // TODO: calculate actual mode using heat/cool/eco/holiday/comfort mode attributes - // C2_T6_cool_allowed = "0" - // C2_T6_manual_cool_allowed = "0" - // C2_T6_heat_cool_mode = "0" - // C1_T1_heat_cool_slave = "0" - // C2_T6_stat_cb_comfort_eco_mode = "0" - // C1_T1_mode_comfort_eco = "0" - // C2_T6_stat_cb_eco_forced = "0" - // C1_T1_eco_profile_number = "0" - // C2_T6_stat_cb_holiday_mode = "0" - // C2_T6_stat_cb_heat_cool_mode = "0" - // C2_T6_stat_comfort_eco_mode = "0" - // C2_T6_stat_eco_program = "0" - // C2_T6_eco_setting = "0" - // C1_T1_eco_offset = "72" - - // TODO: implement alarms - // C2_T6_stat_cb_fallbk_heatalarm = "0" - // C2_T6_stat_air_sensor_error = "0" - // C2_T6_stat_external_sensor_err = "0" - // C2_T6_stat_rh_sensor_error = "0" - // C2_T6_stat_tamper_alarm = "0" - // C2_T6_stat_rf_error = "0" - // C2_T6_stat_battery_error = "0" - // C2_T6_stat_rf_low_sig_warning = "0" - // C2_T6_stat_valve_position_err = "0" - - // TODO: other - // C1_T1_head1_supply_temp = "54" - // C1_T1_head1_valve_pos_percent = "0" - // C1_T1_head1_valve_pos = "0" - // C1_T1_head2_valve_pos_percent = "0" - // C1_T1_head2_valve_pos = "0" - // C1_T1_head2_supply_temp = "0" - // C1_T1_channel_position = "7" - // C1_T1_head_number = "0" - // C1_T1_bypass_enable = "0" - + thermostats.set(ctKey, { id: ctKey, name: value,