search

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

Minor touch-ups #4

Closed mrose17 closed 7 years ago

mrose17 commented 7 years ago
  1. the "usual" node.js .gitignore
  2. add jshints
  3. report only sensors (let the thermostat report itself)

enjoy!

vojtamolda commented 7 years ago

Good morning @mrose17,

Thanks for the touch-ups. I like your changes a lot! It's great to have another pair of :eyes: go over the code, I'm quite ignorant JS developer :see_no_evil: :smiley:

I have only one question about excluding the thermostat itself. What do you think about making it a config.json option? There's more details in the code review comments.

mrose17 commented 7 years ago

hi. i couldn't find the comments, but i agree that's a better option. the default can remain as you initially coded it, and if the option is set, then only sensors need be considered. how does that sound?

vojtamolda commented 7 years ago

Sounds good!

PS: Sorry, I forgot to submit the review comment... :roll_eyes:

AppleTechy commented 7 years ago

Can you reveal the Thermostat to HomeKit please! I unfortunately have an Ecobee 3 before they rereleased it with HomeKit Support...

vojtamolda commented 7 years ago

Hello @AppleTechy,

Thanks for your comment. Good news :slightly_smiling_face: is that's it's certainly possible to get HomeKit unofficially working with earlier Ecobees, but the bad news :frowning_face: is that it's not currently implemented...

I opened a new issue #7 for you. If you know of any other people or even an online discussion about this problem, please try to get as much support for this as possible.

I don't know how experienced are you with software development, so maybe you can tackle it yourself. It's a challenge, but it's doable since a lot of the boring authorization boilerplate code is done. I wrote some implementation notes in a comment there, while I still remember it... :smiley:

vojtamolda commented 7 years ago

@mrose17 Merged! Thanks a lot for your very nice improvements. I like the new "exclude_thermostat" option a lot.

mrose17 commented 7 years ago

enjoy!