NorthernMan54 / node-red-contrib-homebridge-automation

Homebridge and Node-RED Integration
Apache License 2.0
107 stars 18 forks source link

List of available devices does not refresh under certain conditions #28

Closed dxdc closed 4 years ago

dxdc commented 4 years ago

After adding the Homebridge PIN, I saw these messages in the browser console: (Should I be adding the NodeRED pin instead??). I am running homebridge in insecure mode and confirmed this - I can see the accessories tab in Homebridge Config UI X.

After multiple rounds of deploying, etc. it finally worked! No idea what I did to make that happen but just sharing in case there's something specific here. It seems like it was unable to retrieve the device list for a while for some reason then.

TypeError: Cannot read property 'toList' of undefined
    at /home/pi/.node-red/node_modules/node-red-contrib-homebridge-automation/HAP-NodeRed.js:499:34
    at Layer.handle [as handle_request] (/usr/lib/node_modules/node-red/node_modules/express/lib/router/layer.js:95:5)
    at next (/usr/lib/node_modules/node-red/node_modules/express/lib/router/route.js:137:13)
    at /usr/lib/node_modules/node-red/node_modules/@node-red/editor-api/lib/auth/index.js:74:13
    at Layer.handle [as handle_request] (/usr/lib/node_modules/node-red/node_modules/express/lib/router/layer.js:95:5)
    at next (/usr/lib/node_modules/node-red/node_modules/express/lib/router/route.js:137:13)
    at Route.dispatch (/usr/lib/node_modules/node-red/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/usr/lib/node_modules/node-red/node_modules/express/lib/router/layer.js:95:5)
    at /usr/lib/node_modules/node-red/node_modules/express/lib/router/index.js:281:22
    at param (/usr/lib/node_modules/node-red/node_modules/express/lib/router/index.js:354:14)
    at param (/usr/lib/node_modules/node-red/node_modules/express/lib/router/index.js:365:14)
    at Function.process_params (/usr/lib/node_modules/node-red/node_modules/express/lib/router/index.js:410:3)
    at next (/usr/lib/node_modules/node-red/node_modules/express/lib/router/index.js:275:10)
    at expressInit (/usr/lib/node_modules/node-red/node_modules/express/lib/middleware/init.js:40:5)
    at Layer.handle [as handle_request] (/usr/lib/node_modules/node-red/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/usr/lib/node_modules/node-red/node_modules/express/lib/router/index.js:317:13)

I also saw ben, problem getting evDevices, and error in console. Seems some of it is from HAP-NodeRed.html? Also there are a few other errors:

(Unrelated, but this code in HAP-NodeRed.html - should it be deleted?)

        $.ajax({
          data: JSON.stringify(account),
          url: 'alexa-home/new-account',
          contentType: 'application/json',
          type: 'POST',
          processData: false
        });
dxdc commented 4 years ago

Lastly.. was wondering your thoughts on an embedded mqtt plugin that interfaces directly to homebridge and publishes on all topics? I was just wondering since HAP NodeJS is already setting all values/states, what if these just got replicated to an MQTT server? In that way, it wouldn't be required to be homebridge into an insecure mode.

Meaning, just for read-only values (e.g., sensors) and states - those could be used as triggers via the mqtt topics without a need to connect them directly.

dxdc commented 4 years ago

@NorthernMan54 First of all, amazing plugin. I've had a chance to experiment with it this weekend and it is a game changer!

I found out (maybe?) some more information on what might have happened with my issue.

What happened was, I added some new devices to my Homebridge and wanted this plugin to pick them up so I could use them. It wouldn't detect any changes. I restarted NodeRed, Homebridge, nothing. Then, I ended up making a change in NodeRed - I deleted an existing HB Control - and then deploying. Then, the refresh worked great.

It seems like the problem I had initially is that no devices were picked up at all, and that caused those console errors. Maybe it needs to be deployed first before requesting devices?

Another simple way to replicate this, I found, was to rename a device within Homebridge's cachedAccessories file (I shutdown Homebridge first). The name change was not picked up at all by the plugin, even after multiple refreshes. But, after deleting an existing HB Control, deploying, then refreshing, it did in fact refresh the list.

NorthernMan54 commented 4 years ago

In regards to embedding MQTT into HAP-NodeJS, that is really a question for the maintainer of HAP-NodeJS.

I do realize that there are some issues with getting current data into the node, but as it works as long as it is the last thing started, I'm okay with it. In my ecosystem, I start all my home bridge instances, wait 30 seconds then start this. No issues.

dxdc commented 4 years ago

In regards to embedding MQTT into HAP-NodeJS, that is really a question for the maintainer of HAP-NodeJS.

Thanks, I may pose it to them. Though, your solution may be superior honestly after using it. The only disadvantage is the 'insecure' aspect, but I'm not sure how much of a concern that really is, and it opens up a whole world of possibility.

I have a prototype virtual thermostat built off of https://flows.nodered.org/node/node-red-contrib-comfort which works spectacularly.

I do realize that there are some issues with getting current data into the node, but as it works as long as it is the last thing started, I'm okay with it.

Agreed. I think what I discovered is that there are some issues refreshing the data before the node is deployed, and that leads to some console errors and unwanted behavior.

Did you see my note about console.log for ben, pass, as well as the Alexa devices? Were those intentionally committed?

NorthernMan54 commented 4 years ago

They are remnants from the code base I used as a base to develop the node from, let me add them to the list for my next code review and promote.

NorthernMan54 commented 4 years ago

I cleaned up those messages and code, so we should be able to close this

dxdc commented 4 years ago

Nice! Shouldn't this be removed as well? You've removed the definition of var pass= just above it.

if (pass != '_PWD_') {
        var account = {         var account = {
          id: node.id,            id: node.id,
          user: user,             user: user,
          pass: pass              pass: pass
        }
 }
NorthernMan54 commented 4 years ago

Oops, just removed that as well