search

thoukydides / homebridge-homeconnect

Home Connect home appliances plugin for Homebridge
https://www.thouky.co.uk
ISC License
142 stars 15 forks source link

Update to 1.0.0: devices stuck on last displayed state #294

Closed ZyroBlue closed 2 months ago

ZyroBlue commented 2 months ago

Description of Issue

After updating to 1.0.0., all my devices on the Devices tab are stuck at what they were when I last used the Devices tab. Nothing is being forwarded to HomeKit either. After downgrading, all works perfectly again.

Expected Behaviour

Behaviour from last version.

Steps to Reproduce

Update.

Plugin Version

1.0.0.

Environment

Home Connect Appliance(s)

No response

HomeKit App(s)

No response

Diagnostic Checks

Log File

[2.9.2024, 12:28:21] [HB Supervisor] OS: Linux 6.1.21+ arm
[2.9.2024, 12:28:21] [HB Supervisor] Node.js v20.17.0 /opt/homebridge/bin/node
[2.9.2024, 12:28:21] [HB Supervisor] Homebridge Path: /var/lib/homebridge/node_modules/homebridge/bin/homebridge
[2.9.2024, 12:28:21] [HB Supervisor] UI Path: /opt/homebridge/lib/node_modules/homebridge-config-ui-x/dist/bin/standalone.js
[2.9.2024, 12:28:49] [Homebridge UI] Homebridge UI v4.57.1 is listening on :: port 8581
[2.9.2024, 12:28:52] [HB Supervisor] Delaying Homebridge startup by 20 seconds on low powered server
[2.9.2024, 12:29:12] [HB Supervisor] Starting Homebridge with extra flags: -I -P /var/lib/homebridge/node_modules -D --strict-plugin-resolution
[2.9.2024, 12:29:12] [HB Supervisor] Started Homebridge v1.8.4 with PID: xxxxx
[2.9.2024, 12:29:21] Loaded config.json with 0 accessories and 4 platforms.
[2.9.2024, 12:29:22] Loaded 3 cached accessories from cachedAccessories.
[2.9.2024, 12:29:22] ---
(node:26957) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
[2.9.2024, 12:29:30] Loaded plugin: homebridge-homeconnect@1.0.0
[2.9.2024, 12:29:30] Registering platform 'homebridge-homeconnect.HomeConnect'
[2.9.2024, 12:29:30] ---
[2.9.2024, 12:29:54] Loaded plugin: homebridge-hue@0.13.70
[2.9.2024, 12:29:54] Registering platform 'homebridge-hue.Hue'
[2.9.2024, 12:29:54] ---
[2.9.2024, 12:29:55] Loaded plugin: homebridge-smartthings-ik@1.5.21
[2.9.2024, 12:29:55] Registering platform 'homebridge-smartthings-ik.HomeBridgeSmartThings'
[2.9.2024, 12:29:55] ---
[2.9.2024, 12:29:55] Loading 4 platforms...
[2.9.2024, 12:29:55] [HomeConnect] Initializing HomeConnect platform...
[2.9.2024, 12:29:57] [HomeConnect] Restored 2 cached accessories
[2.9.2024, 12:29:57] [HomeConnect] homebridge-homeconnect version 1.0.0
[2.9.2024, 12:29:57] [HomeConnect] Node.js version 20.17.0 (satisfies >=22.0.0 || ^20.12.2 || ^18.20.2)
[2.9.2024, 12:29:57] [HomeConnect] Homebridge version 1.8.4 (satisfies ^1.8.0|| ^2.0.0-beta.0)
[2.9.2024, 12:29:57] [HomeConnect] Homebridge API version 2.7 (satisfies ^2.7)
[2.9.2024, 12:29:57] [HomeConnect] Homebridge HAP version 0.12.2 (satisfies >=0.9.0)
[2.9.2024, 12:29:57] [HomeConnect] Attempting authorisation
[2.9.2024, 12:29:59] [HomeConnect] Using saved access token
[2.9.2024, 12:29:59] [HomeConnect] Successfully authorised
[2.9.2024, 12:29:59] [HomeConnect] Home Connect request #1: GET /api/homeappliances
[2.9.2024, 12:29:59] [HomeConnect] Saving configuration schema data
[2.9.2024, 12:30:04] [HomeConnect] Home Connect request #1: 200 OK +4803ms
[2.9.2024, 12:30:04] [HomeConnect] Saving configuration schema data
[2.9.2024, 12:30:04] [HomeConnect] [Reading list of home appliances] TypeError: Cannot read properties of undefined (reading 'enabled')
[2.9.2024, 12:30:05] [HomeConnect] TypeError: Cannot read properties of undefined (reading 'enabled')
[2.9.2024, 12:30:05] [HomeConnect]     at file:///var/lib/homebridge/node_modules/homebridge-homeconnect/src/platform.ts:203:90
[2.9.2024, 12:30:05] [HomeConnect]     at Array.filter (<anonymous>)
[2.9.2024, 12:30:05] [HomeConnect]     at HomeConnectPlatform.addRemoveAccessories (file:///var/lib/homebridge/node_modules/homebridge-homeconnect/src/platform.ts:203:46)
[2.9.2024, 12:30:05] [HomeConnect]     at HomeConnectPlatform.updateAppliances (file:///var/lib/homebridge/node_modules/homebridge-homeconnect/src/platform.ts:189:17)

[2.9.2024, 12:30:06] [Hue] npm registry: request 1: GET /homebridge-hue/latest
[2.9.2024, 12:30:06] Homebridge v1.8.4 (HAP v0.12.2) (Homebridge CB6A) is running on port xxxxx.
[2.9.2024, 12:30:06] 

NOTICE TO USERS AND PLUGIN DEVELOPERS
> Homebridge 2.0 is on the way and brings some breaking changes to existing plugins.
> Please visit the following link to learn more about the changes and how to prepare:
> https://github.com/homebridge/homebridge/wiki/Updating-To-Homebridge-v2.0

Configuration

{
    "platform": "HomeConnect",
    "name": "HomeConnect",
    "clientid": "[redacted]",
    "simulator": false,
    "language": {
        "api": "de-DE"
    },
    "debug": [
        "Log Debug as Info"
    ]
}

Additional Information

No response

thoukydides commented 2 months ago

Thanks for the report. I can see what the issue is. It occurs if your config.json doesn't specify a configuration for a particular appliance.

This crept in due to VSCode auto-fixing eslint's @typescript-eslint/no-unnecessary-condition rule, which incorrectly assumes that Record<key, value> types have entries for every key. I am also using VSCode's source.fixAll.eslint option which attempts to fix this problem by changing ?. optional chaining to . non-optional chaining whenever the source file is saved - without any warning. My testing had the configuration fully-populated, so didn't encounter this problem.

I need to (i) find a way to prevent VSCode from breaking working code like this (preferably without completely disabling the @typescript-eslint/no-unnecessary-condition rule which has found some real logic errors) and (ii) check for anywhere else that might have been similarly affected. This may take a while...

thoukydides commented 2 months ago

There appear to be quite a few issues raised against eslint for this problem; all of them rejected, saying that either the Record type should include undefined as a possible value (i.e. Record<KeyType, ValueType | undefined>) or Typescript's noUncheckedIndexedAccess option should be used. Unfortunately, the former then incorrectly leads eslint to think that it is possible to have a key defined with an undefined value which breaks a lot of code that iterates over keys, and the latter results in incorrect claims that types have no overlap and are almost impossible to fix without just suppressing eslint for the problem lines.

I think the best option will be to add undefined as a possible type where the object's keys aren't enumerated, but that probably won't be a complete fix.

thoukydides commented 2 months ago

On the plus side, adding undefined as a possible value means that eslint no longer complains about the object[key] ?? default pattern, which previously required key in object ? object[key] : default.

thoukydides commented 2 months ago

Checking the diffs between v0.42.4 and v1.0.0 these are the places where optional chaining operators were removed:

src/api-ua.ts:
-        if (body?.length) {
+        if (body.length) {
             this.log.debug(`${name} body:`);
--
src/api-value.ts:
-            const typeName = props?.find(prop => prop.name === event.event)?.ttype?.name;
+            const typeName = props?.find(prop => prop.name === event.event)?.ttype.name;
             if (!typeName) {
--
src/has-programs.ts:
-                    if (!name?.length) throw new Error("No 'name' field provided for program");
-                    if (!key?.length)  throw new Error("No 'key' field provided for program");
+                    if (!name.length) throw new Error("No 'name' field provided for program");
+                    if (!key.length)  throw new Error("No 'key' field provided for program");
--
src/has-programs.ts:
-            const constraints = option?.constraints ?? {};
+            const constraints = option.constraints ?? {};
             const allowedValues: string[] = constraints.allowedvalues ?? [];
--
src/homebridge-ui/schema-data.ts
-            const findProgram = (key: string) => appliance?.programs.find(p => p.key === key);
+            const findProgram = (key: string): SchemaProgramWithOptions | undefined => appliance.programs.find(p => p.key === key);
             appliance.programs = newPrograms.map(program => ({ ...findProgram(program.key), ...program }));
--
src/platform.ts:
-        const enabledAppliances = appliances.filter(ha => this.configAppliances[ha.haId]?.enabled ?? true);
+        const enabledAppliances = appliances.filter(ha => this.configAppliances[ha.haId].enabled ?? true);

It looks like all of the other changes were safe.

thoukydides commented 2 months ago

This should be fixed in v1.0.1.

Thanks again for reporting it, and providing a log that led directly to the problem.

ZyroBlue commented 2 months ago

This is insane!