chanomie / homebridge-connectedbytcp

Home bridge Plugin for Connected by TCP
6 stars 3 forks source link

Offline devices cause an error #2

Open smarl opened 8 years ago

smarl commented 8 years ago

If a device is off, the result from getcarousel doesn't contain level.

Here's how I patched...

pi@raspberrypi:~ $ diff /usr/lib/node_modules/homebridge-connectedbytcp/index.js homebridge-connectedbytcp/index.js
88,90c88,91
<             level = 0;
<       if( result.gip.room[i].device[j].level ) {
<         level = result.gip.room[i].device[j].level[k];

---
>             self.log(JSON.stringify(result.gip.room[i].device[j]));
>             var level = 0;
>             if("level" in result.gip.room[i].device[j]) {
>               level = result.gip.room[i].device[j].level[k];
162c163,167
<               tcpLightbulb.level = result.gip.room[i].device[j].level ? result.gip.room[i].device[j].level[k] : 0;

---
>               if("level" in result.gip.room[i].device[j]) {
>                 tcpLightbulb.level = result.gip.room[i].device[j].level[k];
>               } else {
>                 tcpLightbulb.level = 0;
>               }
lokki911 commented 8 years ago

what file did you change to stop this from happening?

smarl commented 8 years ago

Never mind, the code I posted is terrible. HomeKit is requesting all of the devices at once, individually, so all of them occur without cache, and we're back where we started. Need to either introduce a synchronous block or set a global lock variable and recall with a timeout until the lock expires.

smarl commented 8 years ago

Did my pull request work?

chanomie commented 8 years ago

Haven't had a chance to test this yet - but thanks. I will give it a go.

chanomie commented 8 years ago

So this change I totally get and makes sense to me:

tcpLightbulb.level = result.gip.room[i].device[j].level ? result.gip.room[i].device[j].level[k] : 0;

Is the other change correcting a problem? Checking if the key exists in the Array is working for me the way it is (and is my standard way to check):

Original:

if("level" in result.gip.room[i].device[j]) {

Change:

if( result.gip.room[i].device[j].level ) {
smarl commented 8 years ago

No good reason for that change of syntax.

By the way, these changes have been stable for me. It's working as well as my Hue bridge at this point, with about the same number of lights on each.

lokki911 commented 8 years ago

no it didn't work

smarl commented 8 years ago

@lokki911 Try my branch... https://github.com/smarl/homebridge-connectedbytcp Clone it, then run npm install using the path to the directory instead of the module name.

lokki911 commented 8 years ago

Tried this, and this is now what i get. I'll try playing around with this tomorrow when I have time.

[10/6/2016, 8:59:58 AM] ==================== [10/6/2016, 8:59:58 AM] ERROR LOADING PLUGIN homebridge-connectedbytcp: [10/6/2016, 8:59:58 AM] Error: Cannot find module 'node-cache' at Function.Module._resolveFilename (module.js:325:15) at Function.Module._load (module.js:276:25) at Module.require (module.js:353:17) at require (internal/module.js:12:17) at Object. (/usr/local/lib/node_modules/homebridge-connectedbytcp/index.js:2:17) at Module._compile (module.js:409:26) at Object.Module._extensions..js (module.js:416:10) at Module.load (module.js:343:32) at Function.Module._load (module.js:300:12) at Module.require (module.js:353:17) at require (internal/module.js:12:17) [10/6/2016, 8:59:58 AM] ==================== [10/6/2016, 8:59:58 AM] No plugins found. See the README for information on installing plugins. [10/6/2016, 8:59:58 AM] Loaded config.json with 0 accessories and 1 platforms. [10/6/2016, 8:59:58 AM] --- [10/6/2016, 8:59:58 AM] Loading 1 platforms... /usr/local/lib/node_modules/homebridge/lib/api.js:118 throw new Error("The requested platform '" + name + "' was not registered by any plugin."); ^

Error: The requested platform 'ConnectedByTcp' was not registered by any plugin. at API.platform (/usr/local/lib/node_modules/homebridge/lib/api.js:118:13) at Server._loadPlatforms (/usr/local/lib/node_modules/homebridge/lib/server.js:281:45) at Server.run (/usr/local/lib/node_modules/homebridge/lib/server.js:77:36) at module.exports (/usr/local/lib/node_modules/homebridge/lib/cli.js:40:10) at Object. (/usr/local/lib/node_modules/homebridge/bin/homebridge:17:22) at Module._compile (module.js:409:26) at Object.Module._extensions..js (module.js:416:10) at Module.load (module.js:343:32) at Function.Module._load (module.js:300:12) at Function.Module.runMain (module.js:441:10)

smarl commented 8 years ago

Aye. That's why I said npm install instead of just copying. If you cd into the directory and run:

npm install . -g

That should get the dependency. Or just:

npm install node-cache -g
lokki911 commented 8 years ago

the first command (npm install . -g) is what i used in the local dir this morning. Only got the error above. I deleted everything and did a fresh install of Nodejs, homebridge, and your branch for tcp with a local install of that. and now it seems to be working just fine. I'll let you know if i see any other issues over the next few days.

chanomie commented 7 years ago

I merged the change into mine as well and published version 1.0.2.