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

Add Fan and Dry as valid thermostat modes #693

Conversation

adrianmo
Copy link

According to the docs and code, the 'fan' and 'dry' modes are valid HVAC modes.

I have a Z-Wave thermostat that is wrongly reporting the 'dry' and 'fan' modes as presets, which is wrong and causes issues with frontend cards and weird behaviors when changing the modes from the wall controller.

I've checked other climate integrations and they do set the 'dry' and 'fan' as HVAC modes properly.

As additional context, this is what the Z-Wave node reports:

[...]
2023-06-28T12:40:51.013Z CNTRLR   [Node 011] Interviewing Thermostat Mode...
2023-06-28T12:40:51.013Z CNTRLR » [Node 011] querying supported thermostat modes...
2023-06-28T12:40:51.022Z DRIVER » [Node 011] [REQ] [SendData]
                                  │ transmit options: 0x25
                                  │ callback id:      76
                                  └─[ThermostatModeCCSupportedGet]
2023-06-28T12:40:51.030Z DRIVER « [RES] [SendData]
                                    was sent: true
2023-06-28T12:40:51.046Z DRIVER « [REQ] [SendData]
                                    callback id:     76
                                    transmit status: OK
2023-06-28T12:40:51.060Z DRIVER « [Node 011] [REQ] [ApplicationCommand]
                                  └─[ThermostatModeCCSupportedReport]
                                      supported modes:
                                      · Off
                                      · Heat
                                      · Cool
                                      · Fan
                                      · Dry
                                      · Auto changeover
2023-06-28T12:40:51.066Z CNTRLR « [Node 011] received supported thermostat modes:
                                  · Off
                                  · Heat
                                  · Cool
                                  · Fan
                                  · Dry
                                  · Auto changeover
[...]

And this is what I get in Home Assistant:

hvac_modes:
  - 'off'
  - heat
  - cool
  - heat_cool
min_temp: 7
max_temp: 35
fan_modes:
  - Low
  - High
  - Auto medium
  - Medium
preset_modes:
  - none
  - Fan  <--- this should be in hvac_modes
  - Dry  <--- this should be in hvac_modes
current_temperature: 28.9
temperature: null
target_temp_high: null
target_temp_low: null
fan_mode: Low
preset_mode: none
icon: mdi:snowflake
friendly_name: Downstairs Air Conditioner
supported_features: 27

@raman325
Copy link
Contributor

raman325 commented Jun 30, 2023

hi @adrianmo thank you for the submission. You are right that we should move these modes out of presets, but we are concerned about making it a breaking change. So after discussion, here's what we are proposing:

  1. Move these constants out of the lib and into HA since they are HA specific
  2. Include fan and dry modes in both the HVAC modes and the preset modes so we don't break existing automations.
  3. Log a warning when fan and dry are used as a preset indicating that they are deprecated and will be removed in release

Do you want to submit this change? If not, I can take it from here, just let me know!

@adrianmo
Copy link
Author

@raman325 thanks a lot for your quick review!

I took the freedom of drafting a PR with the proposed changes you mentioned (not fully sure about point 1 though).

Can you please have a look at it? Happy to give you access to my fork if you want to push changes to the branch directly.

The PR: home-assistant/core#95634

@raman325 raman325 closed this Jul 3, 2023
@raman325
Copy link
Contributor

raman325 commented Jul 3, 2023

thanks @adrianmo will review and provide feedback there

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