robertklep / homebridge-nefit-easy

DEPRECATED — Homebridge plugin for Nefit Easy™
14 stars 3 forks source link

Update fix for NaN errors, fix slow down Homebridge warning, implement target state, set characteristic props, add outdoor temperature accessory. #46

Closed nickmoone closed 10 months ago

nickmoone commented 10 months ago
nickmoone commented 10 months ago

Everything has been tested and works correctly.

robertklep commented 10 months ago

Thanks!

I'm not sure if creating two separate connections (one for the thermostat, one for the outdoor temperature) to the backend won't cause issues. Ideally, it would be one connection that both services can use.

Also, calling client.connect() every time isn't necessary because a client will only ever connect once.

Since I'm unable to test this myself (not having an Easy anymore), would you be open to taking over the plugin entirely? I can transfer the NPM repository for it to you.

nickmoone commented 10 months ago

Thanks for the feedback.

I will have a look at how I can create only one connection and share it amongst the two accessories.

Do you think we can get rid of all client.connect() calls, except for the one in the initialization function? Do you remember why they were there in all set and get functions in the first place? I don't think it makes a difference because like you said the client only connects when the connection does not yet exist, but then it might be better to remove the unnecessary calls.

Yes, you can share or transfer the NPM repository to me if that works better for you. What information do you need from me to do that?

robertklep commented 10 months ago

You only have to call connect() once for a client. Afterwards, you can still call it whenever you like, but it will just reuse the first (existing) connection.

As for transferring the package: I can add you as a maintainer of the package if you have an npmjs.com account (I need your npmjs.com username for that). Then after you accepted being a maintainer, I can remove myself as a maintainer and it's basically transferred to you. You can then use your own Git repo to publish new releases to NPM.

nickmoone commented 10 months ago

My NPMjs.com username is nickmoone. Of course you can also keep yourself on the list of maintainers if you want. After this pull request is finished, are you able to update the package one more time? I haven't used NPMjs very often to publish packages and need to investigate how I can do this myself.

About the shared connections amongst the two accessories, currently the only way I see to do this is to make the connection a global variable. Since the credentials in the config are only available in the accessory initialization function, and they cannot be combined. Do you think creating a global variable for this is a good idea?

robertklep commented 10 months ago

About the shared connections amongst the two accessories, currently the only way I see to do this is to make the connection a global variable. Since the credentials in the config are only available in the accessory initialization function, and they cannot be combined.

Good point. I'm fairly sure that an accessory can have multiple services, so perhaps it's an idea to just have NefitEasyAccessory that has two services, the thermostat and the (outdoor) temperature.

However, I'm not sure if that's doable with the current setup of the plugin. If you look at the current documentation, accessory plugins are nowadays implemented as a proper class that has a getServices method that returns the services that the accessory implements.

I'd do a rewrite myself (it looks quite simple, and I'd rather upgrade the plugin to using async/await too), but like I said, I don't have a Nefit Easy anymore so cannot test anything, which complicates things.

nickmoone commented 10 months ago

Yes, I found out that the structure of more recent plugins is different too. I haven't been able to convert it to a proper class according to the documentation yet.

I have been able to add the outdoor temperature as a separate service of the NefitEasyAsseccory in this commit, but I later found out that this disabled me from separating the indoor temperature and the outdoor temperature in Apple Home because the outdoor temperature now was a fixed sub-accessory. This was very annoying because it displayed my room temperature as 10°C (the outdoor temperature), so I reverted it.

robertklep commented 10 months ago

This was very annoying because it displayed my room temperature as 10°C (the outdoor temperature), so I reverted it.

It might be required the set the thermostat service to be the primary service:

this.thermostatService.setPrimaryService();
nickmoone commented 10 months ago

I have tried to configure the two accessories using setPrimaryService, unlinkService, and addLinkedService. But the outdoor temperature sensor is still a fixed subaccessory of the thermostat (making it impossible to move the temperature sensor to a different “outdoor” room). Having the outdoor temperature sensor in the same room as the thermostat is not very useful because this causes the room temperature to be displayed as (amongst others) the outdoor temperature.

IMG_4772

I can have a look if it possible to separate them after making the class, but it might take a while before I have time for it. I can also make the connection as a global variable for now.

robertklep commented 10 months ago

Too bad it isn't working. I think using a global client variable is the best solution for now 👍🏻

nickmoone commented 10 months ago

I just pushed a version with the device client as a global variable. The connection is still created for both accessories, since it is checked in nefit-easy-core if the connection already exists. Are you able to release this update?

robertklep commented 10 months ago

I published it as v2.3.0 because of the additional functionality.

nickmoone commented 10 months ago

Thanks!