Add on/off supported feature to climate

[andrey-git]

Description:

There are different climate platforms in the wild, but up until now the general assumption was that it is a mostly always-on device that can change a mode of operation.

This PR brings a better support for A/C units that are manually turned on and off by adding SUPPORT_ON_OFF to supported_features and turn_on and turn_off services.

Support for on/off is added to Sensibo platform.

Meanwhile Sensibo keeps the aux_heat support to make this change backwards compatible. When on/off support is added to the frontend - Sensibo should drop the aux_heat support.

This PR implements part of #11295

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4306

Checklist:

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

• Local tests with tox run successfully. Your PR cannot be merged unless tests pass
• New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
• New dependencies are only imported inside functions that use them ([example][ex-import]).
• New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
• New files were added to .coveragerc.

[tinloaf]

What's the advantage over adding an 'off' operation_mode to the sensibo platform?

[andrey-git]

@tinloaf The list of operation modes comes from sensibo API, it is not hardcoded into the platform.

Also on/off is unrelated to operation mode. This is like saying that lights don't need on/off as we could set brightness to 0 or rgb color to 'black'.

[MartinHjelmare]

Looks good! A minor comment below.

[MartinHjelmare]

This should preferably return None as we want the Entity base class to handle that case, ie set STATE_UNKNOWN.

[MartinHjelmare]

I'm also torn between adding this on/off service or just let operation mode handle that. But I think we could add it now, and postpone the final decision until we have the new outline decided for how the climate component should be changed for the better.

[tinloaf]

I think it adds even more diversity into the climate component. Even if we are going to do a big refactoring some time soon, this won't make it easier.

Currently the items of the operation_mode list comes from the Sensibo API? If that's the case then I assume they represent different temperature presets? If so, they should have been "hold modes", I think… this is all very messy.

My suggestion would be: add a special 'off' mode to the list of operation_modes. Setting this mode is caught in the platform (and the thermostat is switched off). The only downside I see: If someone configured an "off" mode inside his Sensibo device, that mode is "overwritten". However, I think that's worth it for not introducing more API / services in the climate component.

[andrey-git]

The operations in Sensibo depend on the A/C model. They could include 'heat', 'cold', 'fan', 'dry', 'auto', etc.

But whatever Sensibo does is irrelevant to this PR.

I think different parts of the world have different "types" of thermostats, thus this misunderstanding.

When I first started with HA the current way climate is implemented made no sense to me. The main thing each A/C has is an on/off. What it does when it is on is platform-dependent implementation detail.

To me the suggestion to add 'off' to operations is like to say that lights don't need on/off as 'off' can be added to brightness or rgb color or blink/not-blink mode.

[tinloaf]

I don't have too strong feelings towards this. ;-) You're right, adding a separate on / off switch might make sense in the component overall, maybe we should do this when doing the refactoring. An on / off setting could also be represented via a toggle in the UI, while an operation mode will probably always be a list to pick from.

[ciotlosm]

I have to agree with @andrey-git that adding on/off makes sense. Even with operation_mode when you want "Status" in your component you always have to calculate "on"/"off" state. It should be easily adapted from any current implementation that wants a correct state on the UI.

Inside the component you can easily just passthrough the on/off to the thermostat or make a transformation to operation_mode.

Later edit: Also if Climate gets a toggle switch, you could switch all Climates within a group (that also would get that toggle switch) Off at the same time. This is useful in a house with multiple zones that each have a thermostat when you enter summer time and want to make sure your heating stays off.

[tinloaf]

One general remark: Currently, scenes allow for exactly one service call per entity. Thus, with your current approach, it is not possible to turn a climate device on and set it to a certain temperature, operation mode or anything within a scene. I guess that basically renders climate devices useless in scenes, since you can never be sure that the device is on, thus always have to call SERVICE_TURN_ON, and can never set any "real" value from within the scene.

The way that e.g. the light component currently handles this is to accept all kind of parameters in its SERVICE_TURN_ON. I guess that would be a feasible approach here, too. However, the climate component already has something similar: the SERVICE_SET_TEMPERATURE also accepts an operation mode, to allow setting both via a scene. Thus, one could also make SERVICE_SET_TEMPERATURE accept an additional turn_on flag or something. I would consider adding most (or all?) possible parameters to SERVICE_TURN_ON cleaner, though.

[pvizeli]

That is a problem of scene and how that work or even not. I don't think this PR could solve that or should try it. It's like media_player. In this case the core changes are okay

[tinloaf]

But then we should label this a breaking change, since the configuration of people using climate devices (at least from sensibo and all other platforms that will implement turn_on / turn_off) in a scene will break.

[andrey-git]

The change is breaking if it changes existing functionality or requires a config change.
This PR adds a new service, so it is not a breaking change.

[tinloaf]

True. Existing configurations should never call turn_off, so they should never need to call turn_on at the same time as set_temperature.