[DNM] Gree Integration rewrite#373
Draft
p-monteiro wants to merge 104 commits intoRobHofmann:masterfrom
Draft
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
Contributor
Author
|
@RobHofmann I was adding the temperature step back, but I'm not sure of its utility. |
Contributor
|
@p-monteiro |
Contributor
Author
|
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... |
Contributor
Author
|
@ChillingSilence, Your device doesn't really want to play along with the fetch_device_info() for getting the firmware version, etc. I allow binding to occur even with an error in fetch info; let's see if the binding fails now. Please try again in alpha.81 and provide the log, even if the bind is successful |
|
@p-monteiro Hope this helps, using 5e3c35f, tried auto-add, then manually adding with IP+MAC, then manually specifying the encryption key on Auto Encryption Version. |
Contributor
Author
|
That's some really odd behaviour there @ChillingSilence. Just so we are on the same page:
|
|
Home Assistant is an x86_64 HAOS install. The network is a flat 192.168.22.0/24 with an Edgerouter-X giving out DHCP and a Ubiquiti U6-LR. There's no firewall or anything on the LAN, the router is doing NAT only and the WiFi is bridging to the LAN. Other things that use broadcasting are fine, like Chromecast etc I've just checked now. It seems the device doesn't work on the other public version either. It responds to pings from within the HA "Terminal & SSH" addon, and also from another workstation on my network. It is a relatively new AC purchase / install / unit so I wonder if it's perhaps just a new type? I found this repo originally and this PR seemed to be getting the most love do honestly I started here. Trying the other primary public repo now though and sadly also no joy. The app shows the following, in case it's helpful: Ran nmap and it showed all TCP ports closed, but it does respond to pings... Confirmed it's the right device too, the arp table matches the DHCP lease which matches the details from luc10/gree-api-client |
Contributor
Author
|
@ChillingSilence ahh, alright. I was under the assumption it was working in the "master" version, and I was actually losing my head, because nothing is functionally different here. So if that is not the case, we shall move to another kind of troubleshooting. I'm not sure the device information is very useful for Gree. I have the same model ID and version as you. However, my firmware is in the range v1.XX and yours is at v3.XX Netcat shows no output, correct? Or does it error? I think I read somewhere that Gree uses MQTT in some units, so maybe newer firmwares dropped the legacy UDP protocol we are using here. I will dig around. Can you send the complete model of your indoor unit? |
|
Ahh my apologies @p-monteiro I had only just tried it now with the master version so didn't know it wasn't behaving there otherwise. My bad. Netcat shows now output, correct. No errors, just, nothing. I have 2x Gree Kingfisher GWH24ATEXF units. I can confirm they work on the WiFi, and I can disconnect my phone to use 4G and still control them... So they're talking somehow. I wonder if I can packet capture from the router 🤔 |
Contributor
Author
|
@ChillingSilence, packet capture would be the best way if you actually know how to do it. I'm not good at it. |
|
It looks like it's still doing stuff on UDP port 7000? All the five packets are pretty much the same Is this helpful as a starting point thought @p-monteiro ? |
Contributor
Author
|
@ChillingSilence It's performing the normal Also, I would prefer not to pollute this PR with this since it is unrelated to it. Open a new issue, and we can continue discussing there. |
Adds a binary sensor that shows if there is a fault reported by the device
Contributor
Author
|
@RobHofmann I'm down to get this merged into a different branch and published as an alpha/beta version. Fixes can then be made against that. I'll let you decide whatever you deem fit for the project. Remember, this is a breaking update. Refer to the original PR message (#373 (comment)) |
This workflow uses the version from the manifest to automatically create a tag and a release. Also prevents two consecutive releases with the same version
Use proper types V2 decrypt and verify combined Debug trimmed garbage (may be useful)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Caution
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 no longer overwrites the official integration.
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:
Breaking changes
Features
DataUpdateCoordinatorGreeDeviceabstraction for device logicRestoreEntitysupport inClimateEntityImprovements
Fixes
Known problems:
As you can see, there are a lot of changes, and I understand if this does not go through as is.