chrisjshull / homebridge-nest

Nest plugin for HomeBridge
707 stars 111 forks source link

updates for hap-nodejs 1.0, update dependencies #663

Closed bwp91 closed 1 week ago

bwp91 commented 3 months ago
  1. updated dependencies
  2. replace Characteristic.getValue() with Characteristic.value as getValue() was removed in hap-nodejs 1.0

I was hoping you could perhaps publish as a beta version so I can install on my homebridge instance 😀

bwp91 commented 3 months ago

@chrisjshull are you around to take a look at this?

chrisjshull commented 2 months ago

@adriancable are you up to review / release here?

@bwp91 see also discussion in https://github.com/chrisjshull/homebridge-nest/issues/642#issuecomment-2264069070 with respect to maintainers. If you want to offer let's discuss there.

chrisjshull commented 2 months ago

@bwp91 you should now have all the permissions you need to merge and publish as a beta version.

bwp91 commented 2 months ago

This is currently published as v4.6.10-beta.0. Plugin is now correctly starting for me, although to properly test I would need to start a fire in my house for my protects to start bleeping 😆

kylehakala commented 2 months ago

Works for me on my Thermostat Gen3 on the latest stable release of HB.

I’ll test this on an HB v2 beta this weekend too probably.

bwp91 commented 2 months ago

@kylehakala i have published 4.6.10-beta.1 which includes the PRs i just merged into develop plus the changes I have made in this PR.

Would appreciate another review from you once you are happy about the plugin working on HB v2. Then I can look into merging this into develop and publishing as a new version.

kylehakala commented 2 months ago

Hey @bwp91 — I tested this out more this morning and afternoon.

All of the fiddling was done on a standalone / single-plugin Homebridge installation, homebridge-nest is the only plugin being used, same results in both a child and non-child bridge, running off macOS [non-containerized]. The instance I'm testing on has only 1 Nest Gen3 thermostat, with 3 or 4 temp sensors, and using the Google Auth cookie method.

HB v1.8.4:

When running hb-nest 4.6.10-beta (.0 or .1), I'm noticing that incoming changes from the Nest service aren't picked up as quickly as they were prior to this beta version (not a major problem for me in this case of gentle thermostat adjustments, but I'm not sure how this would impact other types of Nest devices).

I'm seeing it can take anywhere up to a minute for changes made in the Nest app to be reflected in HomeKit. If I have the HB Accessories screen pulled up and refresh that page as soon as I make a change in the Nest app, the values update and are immediately visible in HomeKit.

No delay/latency issues with changes made from HomeKit; those are immediately reflected in the Nest app / on the thermostat.

HB v.2.0.0-beta.11

I took a stab at the upgrade to HB v2.0 and it did not go well.

Once upgrading from HB 1.8.4 to 2.0.0 beta (bumping Node to v22.5.1 as well), the plugin starts, it begins trying to init the Nest device(s), and it quickly tosses a type error relating to bind on the first device:

[8/4/2024, 1:24:28 PM] [Nest Test] Launched child bridge with PID 84403
[8/4/2024, 1:24:28 PM] Registering platform 'homebridge-nest.Nest'
[8/4/2024, 1:24:28 PM] [Nest Test] Loaded homebridge-nest v4.6.10-beta.1 child bridge successfully
[8/4/2024, 1:24:28 PM] Loaded 5 cached accessories from cachedAccessories.0E########83.
[8/4/2024, 1:24:28 PM] [Nest Test] Fetching Nest devices.
[8/4/2024, 1:24:28 PM] Homebridge v2.0.0-beta.11 (HAP v1.1.1-beta.0) (Nest Test) is running on port 51289.
[8/4/2024, 1:24:32 PM] [Nest Test] initing thermostat (P) "Living Room Thermostat": deviceId: 6#############4 structureId: 7############################c
[8/4/2024, 1:24:32 PM] [Nest Test] TypeError: Cannot read properties of undefined (reading 'bind')
    at new NestDeviceAccessory (/usr/local/lib/node_modules/homebridge-nest/lib/nest-device-accessory.js:54:87)
    at new NestThermostatAccessory (/usr/local/lib/node_modules/homebridge-nest/lib/nest-thermostat-accessory.js:17:9)
    at NestPlatform.<anonymous> (/usr/local/lib/node_modules/homebridge-nest/index.js:60:47)
    at NestPlatform.<anonymous> (/usr/local/lib/node_modules/homebridge-nest/index.js:67:17)
    at HomebridgeAPI.<anonymous> (/usr/local/lib/node_modules/homebridge-nest/index.js:100:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
[8/4/2024, 1:24:32 PM] [Nest Test] NOTE: Because we couldn't connect to the Nest service, your Nest devices in HomeKit will not be responsive.

After that, the devices eventually become unavailable within HomeKit and that's the end of it.

Downgrading back to HB v.1.8.4 (keeping the hb-nest 4.6.10-beta) immediately brings devices back to life in HomeKit (albeit, seeing the same latency noted above for updates coming in from Nest). Fwiw, there are no issues leaving node on v22.5.1 with HB v1.8.4 post-downgrade.

(And while I was poking around, I also tried with/without the HB-ui-config beta v4.56.5.30-beta to see how that was looking. Very excited for that!)

kylehakala commented 2 months ago

Thanks for the extra fixes—up and running on HB v2 now!

Also did some additional testing and confirmed that existing Google Account tokenAuth implementations will continue to work with this hb-nest 4.6.10-beta on both HBv1.8.4 and after upgrading to HBv2.

bwp91 commented 2 months ago

i still want to look further into this

When running hb-nest 4.6.10-beta (.0 or .1), I'm noticing that incoming changes from the Nest service aren't picked up as quickly as they were prior to this beta version (not a major problem for me in this case of gentle thermostat adjustments, but I'm not sure how this would impact other types of Nest devices).

bwp91 commented 1 month ago

@kylehakala

I'm noticing that incoming changes from the Nest service aren't picked up as quickly as they were prior to this beta version

any improvement with the new beta of the plugin?

github-actions[bot] commented 2 weeks ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

kylehakala commented 1 week ago

@bwp91 it isn’t noticeably improved from what I can tell, but it’s also not been all too impactful for me. There is indeed a slightly delay in that one direction (temp changes made in the Nest app take up to a minute or so to propagate to HB and connected bridges), but I wouldn’t call it horrible.

I think given the directionality of the delay, it’s not as concerning since changes made from the HomeKit end immediately propagate to the Nest device, which is what I would want to be immediate.

Changes made to the Nest system via the Neat app (or physical thermostat) taking up to a minute to appear in the UI of third party services (HomeKit, HV) won’t break me personally.