search

NRCHKB / node-red-contrib-homekit-bridged

Node-RED Contribution - HomeKit Bridged : Node-RED nodes to simulate Apple HomeKit devices.
https://nrchkb.github.io
MIT License
414 stars 51 forks source link

Crash upon removing bridge from Home #22

Closed nikolaykasyanov closed 5 years ago

nikolaykasyanov commented 6 years ago

Hi, thanks for the fork!

I'm experiencing a crash upon removing bridges from Home, here's the stacktrace:

31 Jul 19:40:15 - TypeError: Cannot read property 'updateAdvertisement' of undefined
    at Bridge.Accessory._handleUnpair (/Users/nkasyanov/Desktop/node-red/node_modules/hap-nodejs/lib/Accessory.js:699:20)
    at HAPServer.emit (events.js:182:13)
    at HAPServer._handlePairings (/Users/nkasyanov/Desktop/node-red/node_modules/hap-nodejs/lib/HAPServer.js:823:10)
    at HAPServer.<anonymous> (/Users/nkasyanov/Desktop/node-red/node_modules/hap-nodejs/lib/HAPServer.js:209:39)
    at IncomingMessage.emit (events.js:182:13)
    at endReadableNT (_stream_readable.js:1081:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)
nikolaykasyanov commented 6 years ago

Here are some observations:

ttww commented 5 years ago

Just for your information: Got the same problem after removing a bridge. Restarting node-red "used" as a work around.

martincollar commented 5 years ago

same here, when i remove bridge from home node-red crashes

20 Jan 19:44:06 - TypeError: Cannot read property 'updateAdvertisement' of undefined at Bridge.Accessory._handleUnpair (/Users/kolar/.node-red/node_modules/@plasma2450/hap-nodejs/lib/Accessory.js:699:19) at emitTwo (events.js:125:13) at HAPServer.emit (events.js:213:7) at HAPServer._handlePairings (/Users/kolar/.node-red/node_modules/@plasma2450/hap-nodejs/lib/HAPServer.js:823:10) at HAPServer.<anonymous> (/Users/kolar/.node-red/node_modules/@plasma2450/hap-nodejs/lib/HAPServer.js:209:39) at emitNone (events.js:105:13) at IncomingMessage.emit (events.js:207:7) at endReadableNT (_stream_readable.js:1045:12) at _combinedTickCallback (internal/process/next_tick.js:138:11) at process._tickCallback (internal/process/next_tick.js:180:9)

1stone commented 5 years ago

I'm experincing the same problem:

Feb 03 11:14:00 openhabian Node-RED[2227]: 3 Feb 11:14:00 - TypeError: Cannot read property 'updateAdvertisement' of undefined
Feb 03 11:14:00 openhabian Node-RED[2227]:     at Bridge.Accessory._handleUnpair (/home/nodered/.node-red/node_modules/hap-nodejs/lib/Accessory.js:701:20)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at emitTwo (events.js:126:13)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at HAPServer.emit (events.js:214:7)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at HAPServer._handlePairings (/home/nodered/.node-red/node_modules/hap-nodejs/lib/HAPServer.js:823:10)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at HAPServer.<anonymous> (/home/nodered/.node-red/node_modules/hap-nodejs/lib/HAPServer.js:209:39)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at emitNone (events.js:106:13)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at IncomingMessage.emit (events.js:208:7)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at endReadableNT (_stream_readable.js:1064:12)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at _combinedTickCallback (internal/process/next_tick.js:138:11)
Feb 03 11:14:00 openhabian Node-RED[2227]:     at process._tickCallback (internal/process/next_tick.js:180:9)

Somehow this creates some inconsistent state, as I'm unable to add that bridge (after modifications) again to the Home App. The only thing that worked so far, is deleteing the Bridge-Node in NR and creating it new again. Unfortunately, that involves adjusting all Service-Nodes...

If someone knows about a better workaround, please let me know.

martincollar commented 5 years ago

im using homekit2mqtt and integrating it with node-red via mqtt, this node red plugin looks broken :/

Shaquu commented 5 years ago

@martincollar in your stack trace I can see you are using fork by @plasma2450. Please remove @plasma2450/node-red-contrib-homekit-bridged and @plasma2450/hap-nodes.

Then download our latest version (released today): npm i node-red-contrib-homekit-bridged

@1stone Could you update to 0.5.0 and try to reproduce error? Can you write complete list of steps to reproduce it?

Shaquu commented 5 years ago

I can say the problem does exists ;) I am going to look at this next week.

Shaquu commented 5 years ago

There will be a try to fix this today. Could be related to #12

Shaquu commented 5 years ago

@nikolaykasyanov @ttww @martincollar @1stone I think I fixed it on my last commit to dev branch.

What was the problem? When we remove Service nodes from flow then we usually still have unused Bridge node. Bridge node is not published since it doesn't have active Service nodes. And since it's not published it crash on advertiser update because advertiser is not started!

What I did? I overrided unpair event handler for Bridge server and added check for advertiser if it's null. It works for me! :)

Shaquu commented 5 years ago

I made a PR in HAP-nodejs. If they accept it then I will remove override from our code.

Shaquu commented 5 years ago

Retest:

  1. Replicating Issue

    • [ ] Use version from master branch
    • [ ] Create new Parent Service with Bridge
    • [ ] Deploy in node-red
    • [ ] Add Bridge to Home.app
    • [ ] Remove Parent Service from flow
    • [ ] Deploy in node-red
    • [ ] Remove Bridge from Home.app
    • [ ] node-red should crash with error 'TypeError: Cannot read property 'updateAdvertisement' of undefined'

  2. Checking fix

    • [ ] Use version from dev branch (you can run module in debug mode DEBUG=NRCHKB node-red)
    • [ ] Create new Parent Service with Bridge
    • [ ] Deploy in node-red
    • [ ] Add Bridge to Home.app
    • [ ] Remove Parent Service from flow
    • [ ] Deploy in node-red
    • [ ] Remove Bridge from Home.app
    • [ ] node-red should unpair successfully (in debug there should be information about this)

Please retest using above. I summon my best associates @radokristof @crxporter @flic @radionoise @emilingerslev aka @NRCHKB/test

Shaquu commented 5 years ago

The last thing I want to do is to release something tested by me only. But if I have to...

Shaquu commented 5 years ago

Please update to new version 0.6.1 using node-red. Then please retest this bug if it's still occurs.

Shaquu commented 5 years ago

So as my fix is merged to newest HAP-nodejs then we need to remove fix from our code. Opening so I will not forget.