sbozarth / homebridge-lc7001

Homebridge plugin to communicate with Legrand LC7001. (RFLC, Adorne, and On-Q)
MIT License
20 stars 9 forks source link

Initial Initiation of LC7001 After Install #10

Closed thk-socal closed 3 years ago

thk-socal commented 4 years ago

I had a problem with the system and the code in platform.ts within the private sub routine findLC7001IndexbyName. The statement: if ('PropertyList' in value) { on line 189 I changed to: if (value) { This change resolved all my issues and this is working perfectly for me. Since value was an object, I changed the code to just check to see if it existed.

sbozarth commented 3 years ago

Do you have any information on what failed about it?

Inside findLC7001IndexByName, value is supposed to be an element of the array this.lc7001.accessories; it is an object representing an accessory on the LC7001.

I test to see if that object has a property 'PropertyList'. This assumes the object exists. If value exists and has a property 'PropertyList' both your code and my code return true. If value exists and does NOT have a property 'PropertyList' your code will throw an error on the next line when I attempt to use that property. If value does not exist, your code will return false and my code will throw an error. I assume this is the problem, so why would the array this.lc7001.accessories have zero objects?

First option is the LC7001 has no accessories. My LC7001 has accessories so I cannot really ever test this possibility. Were you starting it up without any accessories configured on the LC7001?

I have put an enormous amount of debugging information into the code. When homebridge is run with the debug option (-D) I can really trace back where the problem originated.

findLC7001IndexByName is only ever called from one other routing, matchAccessoriesToLC7001. matchAccessoriesToLC7001 is called from three different routines:

1) When the LC7001 emits an 'initialized' event. 2) When the LC7001 emits an 'accessoryUpdate' event. 3) When the API emits a 'didFinishLaunching' event.

1 and 3 are interrelated. The 'didFinishLaunching' will only trigger matchAccessoriesToLC7001 if the LC7001 has reported being initialized, and the 'initialized' event will only trigger matchAccessoriesToLC7001 if 'didFinishLaunching' has already completed. One has to happen before the other, so only the second even triggers the call to matchAccessoriesToLC7001.

But I thought the LC7001 would never set itself as "initialized" unless it had at lease one accessory. The isInitialized property is set by a function, isInitializedTest(). It is never set to true by hand. That function sets the testcase to TRUE and then checks:

1) Does the LC7001 have zones; if not, fail. 2) Does the accessories array have at least one value; if not, fail. This is the this.lc7001.accessories array mentioned before. 3) Is each LC7001 zone in the accessories array; if not, fail.

I looked over this code, and I don't see how this.lc7001.accessories can have 0 elements and not get set to FALSE.

The 'accessoryUpdate' emitter requires that this.lc7001.accessories have at least one element.

I'm not sure I can track this down without the debugging-level logs.

thk-socal commented 3 years ago

I will run down some extra debug data this weekend. I currently have 14 switches/dimmers tied to the LC7001. However this box was added to the Legrand Cloud and connected to Google Home before this move across to Homebridge. So I wonder if something gets set because of that process.

thk-socal commented 3 years ago

[11/13/2020, 7:14:03 PM] TypeError: Cannot use 'in' operator to search for 'PropertyList' in undefined  at C:\Users(remoted)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\platform.ts:189:25  at Array.findIndex ()  at PlatformLC7001.findLC7001IndexByName (C:\Users(removed)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\platform.ts:188:47)  at C:\Users(removed)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\platform.ts:214:32  at Array.forEach ()  at PlatformLC7001.matchAccessoriesToLC7001 (C:\Users(removed)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\platform.ts:212:24)  at EventEmitter. (C:\Users(removed)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\platform.ts:121:14)  at EventEmitter.emit (events.js:315:20)  at LC7001.checkInitialized (C:\Users(removed)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\lc7001.ts:135:26)  at LC7001.processresponseQueue (C:\Users(removed)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\lc7001.ts:351:26)  at LC7001.processBuffer (C:\Users(removed)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\lc7001.ts:306:14)  at Socket. (C:\Users(removed)\AppData\Roaming\npm\node_modules\homebridge-lc7001\src\lc7001.ts:112:22)  at Socket.emit (events.js:315:20)  at addChunk (_stream_readable.js:309:12)  at readableAddChunk (_stream_readable.js:280:11)  at Socket.Readable.push (_stream_readable.js:223:10)

sbozarth commented 3 years ago

I think your issue is that you have non-sequencial zone ID numbers on your LC7001.

The plugin keeps an array of zones. To make programming easier, the array uses the zone ID as the index. If it inserts elements 0, 1, 2, 3, and then 5 (skipping 4) JavaScript places an undefined array element at index 4.

The .forEach() array method handles this well. The .findIndex() method, however, does not. Your fix solved this by testing for the existence of the element, but could theoretically break if the element existed, but did not have a PropertyList property.

The solution was to put an outer if statement that tests for an undefined element.

Please see release v1.0.4. This has been pushed to NPM.

thk-socal commented 3 years ago

Works perfectly now. Thanks for the fix and also the great work on the plugin. Let me move my Google Home platform over to Homebridge and Home Kit with ease. Excellent work!