[DNM] Gree Integration rewrite#373
Conversation
- Change config flow to: be multi step and allow setting the device features; allow to reconfigure an entry - Separate HA and Device API logic: device API is now declarative and handles device behaviour. HA entities expose the device - Implement a coordinator - Async device communication - More error handling
- Move consts around - Proper error report on config_flow - Proper translations on config_flow - Configuration of timeout - Fetch device info during binding
|
@RobHofmann I was adding the temperature step back, but I'm not sure of its utility. |
|
@p-monteiro |
|
Hi @domialex, that's fine. I'm more interested in the real use case for it. Should it be locked to integers? I don't think any device supports half degrees or even 0.1 steps, as is the lower limit for the current slider... |
|
I`ve installed your code, everything works- the 1-st is GEH18AAXF-K6DNA1 and the second is GWH12AVCXD-K6DNA1A. Both with FW 1.31. |
|
I tried your code as well but I am not able to connect the devices any longer. With the "original" version it worked fine. It says that it was not able to obtain the encryption key. The "original" code is capable of doing this (encryption version = 2). I also picked the encryption keys from the logs from the original version but it did not help. The devices are also no auto discovered anymore. None of the 3 devices is found. The original integration was able to find these but adding still required manual input. |
|
@chkr1011 Sorry, I didn't have much time lately to continue this code, apart from minor tweaks. Can you provide more information about your case, if it is the latest code, what kind of setup you have (normal or VRF), and if possible, the logs from the integration (enable debug mode before searching or adding a device). Hopefully, next month I'll be able to take a deeper look into the issues. |
|
Here is what I get when trying to discover the devices: To me this looks like that the integration is not trying encryption version 2 when auto discovering devices. The following log is from the manual config flow: For the manual config flow I filled IP, MAC, Encryption Version 2 and the Encryption Key as well. These were working properly with the original version. The device is a Vaillant Vai5. I don't know which regular gree model this is. But no VRF. |
|
@chkr1011 Thanks for the logs, will take a look when I can. I think the original code did discovery using V1 only and V2 was only used for binding/communicating, will investigate. |
|
I |
Inspired by github.com/cmroche/greeclimate
|
@RobHofmann @domialex |
|
I kindly ask the people who where having problems with this branch to retry once again. If failures happen, please provide logs in DEBUG mode and if possible try the "Download Diagnostics" for both config entries and devices. |
|
can we somehow break this PR up in smaller pieces. I'm really not happy with 1 big PR overhauling everything. I'd like to keep the agile approach. |
|
@RobHofmann I don't mind breaking it up, but I'm not sure how much can be separated. It mainly touches these points:
Do you have any suggestions on how to go about it? |
|
It’s essentially a full rewrite, and I don’t have the time to test or review all the changes in detail. @RobHofmann I don't recommend breaking the PR into smaller pieces since this is a rewrite, but I do recommend making a beta release instead and see how it goes. Put the release as a "pre-release", HACS should handle the rest. |
|
@domialex Thats fair. @p-monteiro do you know if this is a breaking release (will entities be recreated or have different names for example)? And I need some insights on feature parity with the current version. I will do some testing on my side when we clean the above things up. |
|
@RobHofmann This is very much a breaking release, because configs cannot be migrated (change in Domain). I didn't know HACS supported pre-preleases; that would be sweet because it's missing testing. @domialex, I am trying to support it as best as I can, from what I gather in this PR, VRF is an issue, and discovery seems to fail in encryption v1 (?). I need people to test and send logs, because my multisplit is working well, as is. I'd love to make more meaningful logs, but I'm not sure how, because when it fails, it basically gives an exception communicating with the device, probably because a bad request was made. But the device is a bit of a black box. Regarding feature regressions, I don't think there are any more. Some things might be configured differently during the flow, but the end result should be good. |
|
@ChillingSilence Thanks for the report, will look into it. Do you know the correct encryption version of your device? |
I don't sorry @p-monteiro but if you have a link to how I can extract that info, I've tried the luc10/gree-api-client but I don't believe that tells me the encryption version: I have two of these devices, both showing the same symptoms. Is there a way for me to extract that info from the app? |
Not that I am aware of. You can try manually adding the device using that key and testing both versions. |
|
I tried it again after doing a git pull, then restarted HA: I tried initially to autodiscover. That failed, so I used the output from the gree-api-client: |
|
@ChillingSilence I suspected it was happening, but you are NOT on the latest commit. Try updating to the latest commit on the correct branch (0468123) If you are on it, check the folders, delete the 2 pycache folders inside the integration and restart HA |


[DO NOT MERGE]
⚠️ATTENTION ⚠
This PR changes the domain of the integration, so current users will lose their device configs!
The config schema also changed, so verify your YAML if you use it to configure the devices.
This PR introduces a rewrite of this integration. The main goal of this was to better align the integration with the Home Assistant (HA) development workflow with two main changes:
Other changes that are present in this PR:
I want full feature parity with the current integration and iron out some bugs before this is ready to merge (if you so wish). Currently missing and wishlist:
As you can see, there are a lot of changes, and I understand if this does not go through as is.
I am creating the PR mainly to gather feedback and track progress since most features are now present in this PR.
Don't treat this PR as blocking. I am tracking new changes and integrating them as they are committed.
Also, since this is a big rewrite, I wonder if we should use it to bump the major version (4?) and possibly change the domain to not replace the official integration (as I saw in the bug reports that it might impose some problems).