Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fan modes for 99432 #2565

Merged
merged 1 commit into from
May 11, 2021
Merged

Fix fan modes for 99432 #2565

merged 1 commit into from
May 11, 2021

Conversation

Koenkk
Copy link
Owner

@Koenkk Koenkk commented May 10, 2021

@cmccambridge
Copy link

cmccambridge commented May 11, 2021

Modes look good 👍 I'll update my PR to only add the fan's existing modes in the HA discovery message.

My only remaining question (which might belong back on the zigbee2mqtt PR) is whether we can do anything special with the home assistant discovery to present this specific fan as "4 speeds" plus "1 preset", versus the generic ZCL "3 speeds" plus "2 presets" (on, smart). And if we do: Does that happen here in the converters, or only in the home assistant extension?

Summarizing the ideal state of a generic zigbee fan versus the 99432 fan:

ZCL HVAC FanMode Attribute ZCL Description Generic Fan HA Discovery 99432 Meaning 99432 HA Discovery
0x00 Off State: Off Off State: Off
0x01 Low Speed: 1 Low Speed: 1
0x02 Medium Speed: 2 Medium Speed: 2
0x03 High Speed: 3 High Speed: 3
0x04 On State: On Very High Speed: 4
0x05 Auto (the fan speed is self-regulated) Preset: auto (unused) --
0x06 Smart (when the heated/cooled space is occupied, the fan is always on) Preset: smart "Comfort Breeze" Preset: Smart

(Edit: Just to be clear: The modes in this PR are definitely correct. If we were looking at the "Exposes" tab in the zigbee2mqtt web frontend, those are exactly the modes I would expect to see in the list for 99432 😄. The question about "4 speeds and 1 preset" is only a question for the Home Assistant discovery extension.)

@Koenkk
Copy link
Owner Author

Koenkk commented May 11, 2021

@cmccambridge we should deal with this in the home assistant extension since it's HA specific.

@Koenkk Koenkk merged commit cff83c0 into master May 11, 2021
@Koenkk Koenkk deleted the hampton branch May 11, 2021 15:58
@cmccambridge
Copy link

Perfect, looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants