vojtamolda / homebridge-ecobee3-sensors

Homebridge plugin that exposes Ecobee 3 sensors as HomeKit accessories.
https://www.npmjs.com/package/homebridge-ecobee3-sensors
MIT License
26 stars 15 forks source link

Plugin crashes with empty/non-existent config file #17

Closed AppleTechy closed 7 years ago

AppleTechy commented 7 years ago

I tried running Homebridge without a config but it appears there is some issue with the plugin since it can't even get past loading the plugin. I would appreciate fixing this fast, due to it totally having shut down my Homebridge.

Load homebridge-ecobee3-sensors.Ecobee 3 Sensors
/Users/applegeek/.nvm/versions/node/v5.9.1/lib/node_modules/homebridge-ecobee3-sensors/source/platform.js:21
  this.excludeSensors = config.exclude_sensors || false;
                              ^
TypeError: Cannot read property 'exclude_sensors' of null
    at new EcobeePlatform (/Users/applegeek/.nvm/versions/node/v5.9.1/lib/node_modules/homebridge-ecobee3-sensors/source/platform.js:21:31)
    at Server._loadDynamicPlatforms (/Users/applegeek/.nvm/versions/node/v5.9.1/lib/node_modules/homebridge/lib/server.js:315:30)
    at Server.run (/Users/applegeek/.nvm/versions/node/v5.9.1/lib/node_modules/homebridge/lib/server.js:82:8)
    at module.exports (/Users/applegeek/.nvm/versions/node/v5.9.1/lib/node_modules/homebridge/lib/cli.js:40:10)
    at Object.<anonymous> (/Users/applegeek/.nvm/versions/node/v5.9.1/lib/node_modules/homebridge/bin/homebridge:17:22)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Function.Module.runMain (module.js:447:10)
    at startup (node.js:142:18)
    at node.js:939:3
vojtamolda commented 7 years ago

Hello @AppleTechy,

Sorry for the trouble. There's a couple of new config options added in version 0.1.3 and I guess that's what's causing the trouble. Adding a few lines to the config.json in the same fashion as below should resolve your issue:

"platforms": [
  {
    "platform": "Ecobee 3 Sensors",
    "name": "Ecobee",
    "exclude_sensors": false,
    "exclude_thermostat": false
  }
]
AppleTechy commented 7 years ago

Hello. I had been trying this without a config.json in place. So I wasn't even trying to initiate the plugin. Any ideas are you on Slack?

vojtamolda commented 7 years ago

I don't think this plugin will work without the config.json file. I've never tested it like that. Why are you running Homebridge like this? Maybe I'm missing something.

I have Slack, but I don't use it much...

AppleTechy commented 7 years ago

I was running it without a config.json to test if it was a problem with how my config.json was setup. Which it clearly isn't due to the plugin failing when it loads not when it tries to initiate the plugin. It is possible to run it this way. Even when using the example config you give in the plugin, I still get the error. I am on node 7.5.0 but also had issues when i tried running it on node 6.9.5

vojtamolda commented 7 years ago

So... I checked it and the plugin really fails to start without config.json present in ~/.homebridge/ directory. I'll fix it as sson as I can and release a new version.

In the meantime adding the "Ecobee" section from my previous comment should fix the problem. The current implementation needs this section to initialize correctly.

AppleTechy commented 7 years ago

I added the ecobee section but it is still throughing the error. When I edited the platform.js file and removed config.excude_sensors, and just made the value false along with config.exclude_thermostats to false, I was able to get Homebridge up and running. But after it displayed the pin I needed to add to my account, it immediately crashed almost as if how you set maybe a var might be causing issues.

vojtamolda commented 7 years ago

Hi @AppleTechy,

Your fix is a good, idea, but as you write it breaks things a little bit further down the road. It won't work when the plugin actually tries to access the config.exclude_thermostat value which will be undefined (or null). Primary reason why the whole thing crashes is that config variable passed into the plugin constructor from Homebridge is null in cases where the config.json doesn't exists or there's no "Ecobee" platform section in there. Your fix just postpones this crash to accessing an individual member of config. Then it crashes for the very same reason.

We have to make sure that in cases when the config var passed into EcobeePlugin(...) is null we replace it with an empty dictionary. Then acessing config won't crash and all the settings used by the plugin will be set to their default values. We should change line #19 of platform.js to this:

 this.config = config || {};

Chances are there is a couple of other places (typically where we read from the config var) that will need a similar change to deal with the case of config being an empty dict and use a sensible default value.

AppleTechy commented 7 years ago

Where you able to take a look at it and fix the node 6+ compatibility?

vojtamolda commented 7 years ago

Not yet... Please, bear with me.

vojtamolda commented 7 years ago

Fixed in version 0.1.4