search

ksya / ha-nefiteasy

Nefit Easy connection for Home Assistant
74 stars 31 forks source link

Official HomeAssistant integration #198

Open starkillerOG opened 2 years ago

starkillerOG commented 2 years ago

I just got my Nefit Easy thermostat this weekend and found this wonderfull integration. Thanks for all the hard work on this!

Why has this not been submitted as an official HomeAssistant integration?

If the current maintainers agree, I would like to submit this as an official integration. I have done this for multiple integrations already, so I know what is involved.

To me it seems like the integration is a bit heavy on the API site (connect/disconnect logic, Async Lock etc.). So we may have to move some to the upstream library, but that schould not be too difficult. I propose to submit it first as is, and see what the HA core team thinks about it.

RobBie1221 commented 2 years ago

Nice to see you like it @starkillerOG

I think the original maintainer doesn't have a Nefit Easy anymore. I picked it up some time ago and started refactoring. Honestly speaking, I have been doing some work on integrations both inside and outside the core repo, and I like this much more. The ideological architecture approach that they sometimes randomly enforce in the core repo is braking innovation and not providing timely solutions. I also think the current model of maintaining integrations in the core repo with 465 open pull requests is not sustainable. They encourage people to help with reviews but in the end a handful maintainers want to triple check every line of code. Looking than at this integration, it's fully tested against HA core with a higher test coverage than the average core integration.

Having said that, if you have the energy to transfer it to the core repo, feel free to do so :) I started refactoring the connect/disconnect logic, that is the only part that is indeed too heavy. You can start building on that.

For the library, that also needs attention (maybe first needs attention). The current library (aionefit-updated) has been forked from an archived repo because of a python3.9 bug, but that one isn't maintained either. I think we need a new library.

starkillerOG commented 2 years ago

@RobBie1221 I defenitly get the frustrations with the HomeAssistant core team. However I do like to have integrations in the core repo in order for other new users to easily find the integration. I think with core integrations the user base is a lot bigger and therefore also the potential pool of people willing to contribute to the integration making it even better. Besides simple changes will automatically be applied to core integrations while they might cause problems if not applied in a custom integration.

Where have you refactored the connect/disconnect logic? Is there some other branch?

Yea the aionefit-updated library is defentily a problem if there is no current maintainer. Are you willing to fork the aionefit-updated library and publish it on pypi so that new PRs can be made against that library?

RobBie1221 commented 2 years ago

As said, I'm quite happy with this way of working, but if you want to push it, feel free. I would not submit as is, I would first refactor connect/disconnect. I would not expect the current code to be accepted.

I think I have the refactored branch only locally, I'll push it such that you can have a look.

As to the lib, if you want to start working on that you can better fork it yourself. I can't maintain it at the moment.

RobBie1221 commented 2 years ago

See: https://github.com/RobBie1221/ha-nefiteasy/tree/refactor_init

It has been a while ago since I made this. Some things are missing, like the re-auth code in config flow, but the basic refactoring in init.py is done.

starkillerOG commented 2 years ago

@RobBie1221 thanks, I have some other PRs I need to finished before starting on this, but once I have some time I will look into it.