Add service set_hvac_mode #2303
Conversation
| @@ -91,6 +97,17 @@ def set_fan_mode(hass, fan_mode, entity_id=None): | |||
|
|
|||
| hass.services.call(DOMAIN, SERVICE_SET_FAN_MODE, data) | |||
|
|
|||
| def set_hvac_mode(hass, hvac_mode, entity_id=None): | |||
There was a problem hiding this comment.
- 1: E302 expected 2 blank lines, found 1
|
Sooooo, how is this different from the hvac component? Are there thermostats that have HVAC mode or should those devices actually be configured under the HVAC component? |
|
@balloob HVAC mode (auto/cool/heat/etc. is an option mode for all supported thermostats that control whatever HVAC unit they are connected to. Maybe I don't understand the purpose of the HVAC component... aren't all the options of HVAC zwave component already supported by Z-Wave thermostat component? The code for hvac/zwave and thermostat/zwave look redundant to me. Existing thermostat component already provides control for temp, away, fan etc. Seems unnecessary to split up the thermostat component to report/set temp and then HVAC component for changes of certain or even in some case the same operation modes. Or should each thermostat component provide their own service options? The most recent merge of thermostat/ecobee component provides service ecobee_set_fan_min_time which could likely be accomplished with thermostat/set_fan_mode with a given time. My intent is to provide a way to set the hvac mode for all supported thermostats. I could add this as a service as thermostat/ecobee_set_hvac_mode instead but this is an option for all thermostats although some may call auto mode heat&cool mode instead. I think @chilicheech carried some of the set_hvac_mode service over in thermostat/ecobee.py that I included in #2092 . This PR just includes the set_hvac_mode definitions missing from init.py for any thermostat component to make use of it. |
|
I also don't understand the purpose of an hvac component. Is it so you can control your hvac without using a thermostat? As in, control it directly from hass? Making use of z-wave temperature sensors? If so, that makes sense. However, much of the code should probably be shared with the thermostat component. As @devdelay explained, The reason the set_fan_min_on_time was added to ecobee only is that this isn't a feature that is supported universally. However, I do like the idea of giving set_fan_mode either a boolean or an integer, and the individual thermostats can implement how they make use of the integer. In the case of ecobee, when given an integer, it would set the fan mode to auto and the fan_min_on_time to the value. When given True or On it would set the fan mode to On. With that said, I think hvac_mode is something that is universally support by thermostats. Each thermostat may call it something different. Maybe the question here isn't should this functionality be added but what should we name it. Is hvac_mode a good name for it? Even if that's what ecobee calls it? The other architectural decision is do we need an hvac component and, if so, how can we share code between the thermostat and the hvac components since a lot of it is the same? |
|
From what I remember from my talks with @turbokongen is that there were a bunch of services that are HVAC unit specific that thermostats don't have. For example swing mode. A lot of devices have common functionality, there is a thin line between the switch and the light component. Multi level switches making it even more difficult to distinguish. That being said, hvac and thermostat are so similar. A thermostat is pretty much a remote for an HVAC with optional scheduling. Aaah but so is a switch for a light. I'm inconclusive what the best approach is here. I'll be merging this PR as it is unrelated of this discussion but I think it is worth investigating what the overlap is and what is unique per HVAC/thermostat. Also would be good to get @turbokongen to chime in. |
|
@devdelay I didn't pay attention. Your PR does not include a test for the new service. Can you please add it ? |
Description:
Adds service for thermostat/set_hvac_mode to change thermostat to auto/coo/heat/off. This should be universal for Nest/Ecobee/Zwave thermostats. Please provide input as I only have Ecobee to test.
Related issue (if applicable): fixes #
Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If code communicates with devices:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass