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.

Sign up for GitHub

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

Jump to bottom

Add climate fan modes for Airzone Cloud Aidoo devices #103574

Merged
merged 11 commits into from
Mar 25, 2024
Merged

Add climate fan modes for Airzone Cloud Aidoo devices #103574

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
51dea98
airzone_cloud: climate: add Aidoo speeds support
Noltari Nov 5, 2023
cdbe317
Add suggested changes
Noltari Dec 31, 2023
ced6a54
Merge remote-tracking branch 'upstream/dev' into airzone-cloud-climat…
Noltari Dec 31, 2023
7034607
Merge remote-tracking branch 'upstream/dev' into airzone-cloud-climat…
Noltari Feb 21, 2024
edea357
airzone_cloud: climate: rename _set_fan_speeds()
Noltari Feb 21, 2024
1feff04
airzone_cloud: climate: drop BASE_FAN_SPEEDS
Noltari Feb 21, 2024
65d0049
Merge remote-tracking branch 'upstream/dev' into airzone-cloud-climat…
Noltari Mar 5, 2024
2d63acc
airzone_cloud: climate: refactor FAN_AUTO speed
Noltari Mar 5, 2024
0f4566f
airzone_cloud: climate: document standard speeds replacement
Noltari Mar 5, 2024
681967f
airzone_cloud: climate: fix Homekit comment
Noltari Mar 13, 2024
438c5b6
airzone_cloud: climate: consistent fan speed names
Noltari Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions homeassistant/components/airzone_cloud/climate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
API_PARAMS,
API_POWER,
API_SETPOINT,
API_SPEED_CONF,
API_UNITS,
API_VALUE,
AZD_ACTION,
Expand All @@ -23,6 +24,8 @@
AZD_NUM_DEVICES,
AZD_NUM_GROUPS,
AZD_POWER,
AZD_SPEED,
AZD_SPEEDS,
AZD_TEMP,
AZD_TEMP_SET,
AZD_TEMP_SET_MAX,
Expand All @@ -33,6 +36,10 @@

from homeassistant.components.climate import (
ATTR_HVAC_MODE,
FAN_AUTO,
FAN_HIGH,
FAN_LOW,
FAN_MEDIUM,
ClimateEntity,
ClimateEntityFeature,
HVACAction,
Expand All @@ -54,6 +61,20 @@
AirzoneZoneEntity,
)

FAN_SPEED_MAPS: Final[dict[int, dict[int, str]]] = {
2: {
0: FAN_AUTO,
1: FAN_LOW,
2: FAN_HIGH,
},
3: {
0: FAN_AUTO,
1: FAN_LOW,
2: FAN_MEDIUM,
3: FAN_HIGH,
},
}

HVAC_ACTION_LIB_TO_HASS: Final[dict[OperationAction, HVACAction]] = {
OperationAction.COOLING: HVACAction.COOLING,
OperationAction.DRYING: HVACAction.DRYING,
Expand Down Expand Up @@ -274,6 +295,9 @@ async def async_set_hvac_mode(self, hvac_mode: HVACMode) -> None:
class AirzoneAidooClimate(AirzoneAidooEntity, AirzoneDeviceClimate):
"""Define an Airzone Cloud Aidoo climate."""

_speeds: dict[int, str]
_speeds_reverse: dict[str, int]

def __init__(
self,
coordinator: AirzoneUpdateCoordinator,
Expand All @@ -290,9 +314,58 @@ def __init__(
]
if HVACMode.OFF not in self._attr_hvac_modes:
self._attr_hvac_modes += [HVACMode.OFF]
if (
self.get_airzone_value(AZD_SPEED) is not None
and self.get_airzone_value(AZD_SPEEDS) is not None
):
self._initialize_fan_speeds()

self._async_update_attrs()

def _initialize_fan_speeds(self) -> None:
"""Initialize Aidoo fan speeds."""
azd_speeds: dict[int, int] = self.get_airzone_value(AZD_SPEEDS)
max_speed = max(azd_speeds)

fan_speeds: dict[int, str]
if speeds_map := FAN_SPEED_MAPS.get(max_speed):
fan_speeds = speeds_map
else:
fan_speeds = {}

for speed in azd_speeds:
if speed == 0:
fan_speeds[speed] = FAN_AUTO
else:
fan_speeds[speed] = f"{int(round((speed * 100) / max_speed, 0))}%"
emontnemery marked this conversation as resolved.
Show resolved Hide resolved

fan_speeds[1] = FAN_LOW
fan_speeds[int(round((max_speed + 1) / 2, 0))] = FAN_MEDIUM
fan_speeds[max_speed] = FAN_HIGH
Copy link
Contributor

@emontnemery emontnemery Mar 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want the list to be:

[
        FAN_AUTO,
        FAN_LOW,
        "40%",
        FAN_MEDIUM,
        "80%",
        FAN_HIGH,
    ]

Instead of:

[
        FAN_AUTO,
        "20%",
        "40%",
        "60%",
        "80%",
        "100%",
    ]

If there's a good reason, it wouldn't hurt to explain it in a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because only the default speeds are exposed via Homekit/Alexa, and this is the only way of having N speeds and also getting the default ones exposed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 0f4566f

Copy link
Contributor

@emontnemery emontnemery Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only the default speeds are exposed via Homekit/Alexa

The Alexa integration does not support fan speeds for climate entities.

For homekit there is such a limitation though

core/homeassistant/components/homekit/type_thermostats.py

Lines 313 to 317 in 9b11807

if fan_modes and PRE_DEFINED_FAN_MODES.intersection(fan_modes):
self.ordered_fan_speeds = [
speed for speed in ORDERED_FAN_SPEEDS if speed in fan_modes
]
self.fan_chars.append(CHAR_ROTATION_SPEED)

I'd like a second opinion on this design, integrations should not have to solve limitations in voice assistant integrations.

gree adds additional fan modes with a list like this, but maybe it doesn't work here if there can be many fan speeds:

FAN_MODES = {
    FanSpeed.Auto: FAN_AUTO,
    FanSpeed.Low: FAN_LOW,
    FanSpeed.MediumLow: FAN_MEDIUM_LOW,
    FanSpeed.Medium: FAN_MEDIUM,
    FanSpeed.MediumHigh: FAN_MEDIUM_HIGH,
    FanSpeed.High: FAN_HIGH,
}

Copy link
Contributor Author

@Noltari Noltari Mar 13, 2024 •
edited by emontnemery
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like a second opinion on this design, integrations should not have to solve limitations in voice assistant integrations.

@emontnemery this was already proposed by @bdraco during the same review for the local Airzone integration, see #92840
The Airzone local integration code:

core/homeassistant/components/airzone/climate.py

Lines 158 to 177 in 7c4747b

def _set_fan_speeds(self) -> None:
self._attr_supported_features |= ClimateEntityFeature.FAN_MODE
speeds = self.get_airzone_value(AZD_SPEEDS)
max_speed = max(speeds)
if _speeds := FAN_SPEED_MAPS.get(max_speed):
self._speeds = _speeds
else:
for speed in speeds:
if speed == 0:
self._speeds[speed] = FAN_AUTO
else:
self._speeds[speed] = f"{int(round((speed * 100) / max_speed, 0))}%"
self._speeds[1] = FAN_LOW
self._speeds[int(round((max_speed + 1) / 2, 0))] = FAN_MEDIUM
self._speeds[max_speed] = FAN_HIGH
self._speeds_reverse = {v: k for k, v in self._speeds.items()}
self._attr_fan_modes = list(self._speeds_reverse)

gree adds additional fan modes with a list like this, but maybe it doesn't work here if there can be many fan speeds

According to Airzone there can be up to 8 speeds (+ Auto).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Alexa integration does not support fan speeds for climate entities.

You're right, comment fixed in 681967f

Copy link
Contributor

@emontnemery emontnemery Mar 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was already proposed by @bdraco during the same review for the local Airzone integration

That's not a good proposal though, other integrations should not have to implement workarounds because of limitations in homekit, please change to a consistent naming both in this PR and in the airzone integration.

Maybe the climate integration could make use of some clarification of the fan modes, or splitting between an ordered list of fan speed and fan modes (such as auto), that'd need an architecture discussion though.

Copy link
Contributor Author

@Noltari Noltari Mar 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a good proposal though, other integrations should not have to implement workarounds because of limitations in homekit, please change to a consistent naming both in this PR and in the airzone integration.

Done in 438c5b6.
I don't think that doing it for the local integration is a good idea since it will break the compatibility for existing users...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, maybe you're right that the breaking change is not worth it.
We should maybe add a comment in the local integration saying we don't want that pattern in other integrations though?


if 0 in fan_speeds and 0 not in azd_speeds:
fan_speeds.pop(0)
Noltari marked this conversation as resolved.
Show resolved Hide resolved

self._speeds = {}
for key, value in fan_speeds.items():
_key = azd_speeds.get(key)
if _key is not None:
self._speeds[_key] = value

self._speeds_reverse = {v: k for k, v in self._speeds.items()}
self._attr_fan_modes = list(self._speeds_reverse)

self._attr_supported_features |= ClimateEntityFeature.FAN_MODE

async def async_set_fan_mode(self, fan_mode: str) -> None:
"""Set Aidoo fan mode."""
params: dict[str, Any] = {
API_SPEED_CONF: {
API_VALUE: self._speeds_reverse.get(fan_mode),
}
}
await self._async_update_params(params)

async def async_set_hvac_mode(self, hvac_mode: HVACMode) -> None:
"""Set hvac mode."""
params: dict[str, Any] = {}
Expand All @@ -310,6 +383,14 @@ async def async_set_hvac_mode(self, hvac_mode: HVACMode) -> None:
}
await self._async_update_params(params)

@callback
def _async_update_attrs(self) -> None:
"""Update Aidoo climate attributes."""
super()._async_update_attrs()

if self.supported_features & ClimateEntityFeature.FAN_MODE:
self._attr_fan_mode = self._speeds.get(self.get_airzone_value(AZD_SPEED))


class AirzoneGroupClimate(AirzoneGroupEntity, AirzoneDeviceGroupClimate):
"""Define an Airzone Cloud Group climate."""
Expand Down
71 changes: 71 additions & 0 deletions tests/components/airzone_cloud/test_climate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,20 @@
from homeassistant.components.climate import (
ATTR_CURRENT_HUMIDITY,
ATTR_CURRENT_TEMPERATURE,
ATTR_FAN_MODE,
ATTR_FAN_MODES,
ATTR_HVAC_ACTION,
ATTR_HVAC_MODE,
ATTR_HVAC_MODES,
ATTR_MAX_TEMP,
ATTR_MIN_TEMP,
ATTR_TARGET_TEMP_STEP,
DOMAIN as CLIMATE_DOMAIN,
FAN_AUTO,
FAN_HIGH,
FAN_LOW,
FAN_MEDIUM,
SERVICE_SET_FAN_MODE,
SERVICE_SET_HVAC_MODE,
SERVICE_SET_TEMPERATURE,
HVACAction,
Expand Down Expand Up @@ -42,6 +49,12 @@ async def test_airzone_create_climates(hass: HomeAssistant) -> None:
assert state.state == HVACMode.OFF
assert ATTR_CURRENT_HUMIDITY not in state.attributes
assert state.attributes[ATTR_CURRENT_TEMPERATURE] == 21.0
assert state.attributes[ATTR_FAN_MODE] == FAN_HIGH
assert state.attributes[ATTR_FAN_MODES] == [
FAN_LOW,
FAN_MEDIUM,
FAN_HIGH,
]
assert state.attributes[ATTR_HVAC_ACTION] == HVACAction.OFF
assert state.attributes[ATTR_HVAC_MODES] == [
HVACMode.HEAT_COOL,
Expand All @@ -60,6 +73,15 @@ async def test_airzone_create_climates(hass: HomeAssistant) -> None:
assert state.state == HVACMode.COOL
assert ATTR_CURRENT_HUMIDITY not in state.attributes
assert state.attributes[ATTR_CURRENT_TEMPERATURE] == 20.0
assert state.attributes[ATTR_FAN_MODE] == FAN_MEDIUM
assert state.attributes[ATTR_FAN_MODES] == [
FAN_AUTO,
FAN_LOW,
"40%",
FAN_MEDIUM,
"80%",
FAN_HIGH,
]
assert state.attributes[ATTR_HVAC_ACTION] == HVACAction.COOLING
assert state.attributes[ATTR_HVAC_MODES] == [
HVACMode.HEAT_COOL,
Expand All @@ -79,6 +101,8 @@ async def test_airzone_create_climates(hass: HomeAssistant) -> None:
assert state.state == HVACMode.COOL
assert state.attributes[ATTR_CURRENT_HUMIDITY] == 27
assert state.attributes[ATTR_CURRENT_TEMPERATURE] == 22.5
assert ATTR_FAN_MODE not in state.attributes
assert ATTR_FAN_MODES not in state.attributes
assert state.attributes[ATTR_HVAC_ACTION] == HVACAction.COOLING
assert state.attributes[ATTR_HVAC_MODES] == [
HVACMode.COOL,
Expand All @@ -97,6 +121,8 @@ async def test_airzone_create_climates(hass: HomeAssistant) -> None:
assert state.state == HVACMode.COOL
assert state.attributes[ATTR_CURRENT_HUMIDITY] == 27
assert state.attributes[ATTR_CURRENT_TEMPERATURE] == 21.5
assert ATTR_FAN_MODE not in state.attributes
assert ATTR_FAN_MODES not in state.attributes
assert state.attributes[ATTR_HVAC_ACTION] == HVACAction.COOLING
assert state.attributes[ATTR_HVAC_MODES] == [
HVACMode.HEAT_COOL,
Expand All @@ -116,6 +142,8 @@ async def test_airzone_create_climates(hass: HomeAssistant) -> None:
assert state.state == HVACMode.OFF
assert state.attributes[ATTR_CURRENT_HUMIDITY] == 24
assert state.attributes[ATTR_CURRENT_TEMPERATURE] == 25.0
assert ATTR_FAN_MODE not in state.attributes
assert ATTR_FAN_MODES not in state.attributes
assert state.attributes[ATTR_HVAC_ACTION] == HVACAction.OFF
assert state.attributes[ATTR_HVAC_MODES] == [
HVACMode.COOL,
Expand All @@ -133,6 +161,8 @@ async def test_airzone_create_climates(hass: HomeAssistant) -> None:
assert state.state == HVACMode.COOL
assert state.attributes[ATTR_CURRENT_HUMIDITY] == 30
assert state.attributes[ATTR_CURRENT_TEMPERATURE] == 20.0
assert ATTR_FAN_MODE not in state.attributes
assert ATTR_FAN_MODES not in state.attributes
assert state.attributes[ATTR_HVAC_ACTION] == HVACAction.COOLING
assert state.attributes[ATTR_HVAC_MODES] == [
HVACMode.COOL,
Expand Down Expand Up @@ -269,6 +299,47 @@ async def test_airzone_climate_turn_on_off(hass: HomeAssistant) -> None:
assert state.state == HVACMode.OFF


async def test_airzone_climate_set_fan_mode(hass: HomeAssistant) -> None:
"""Test setting the fan mode."""

await async_init_integration(hass)

# Aidoos
with patch(
"homeassistant.components.airzone_cloud.AirzoneCloudApi.api_patch_device",
return_value=None,
):
await hass.services.async_call(
CLIMATE_DOMAIN,
SERVICE_SET_FAN_MODE,
{
ATTR_ENTITY_ID: "climate.bron",
ATTR_FAN_MODE: FAN_LOW,
},
blocking=True,
)

state = hass.states.get("climate.bron")
assert state.attributes[ATTR_FAN_MODE] == FAN_LOW

with patch(
"homeassistant.components.airzone_cloud.AirzoneCloudApi.api_patch_device",
return_value=None,
):
await hass.services.async_call(
CLIMATE_DOMAIN,
SERVICE_SET_FAN_MODE,
{
ATTR_ENTITY_ID: "climate.bron_pro",
ATTR_FAN_MODE: FAN_AUTO,
},
blocking=True,
)

state = hass.states.get("climate.bron_pro")
assert state.attributes[ATTR_FAN_MODE] == FAN_AUTO


async def test_airzone_climate_set_hvac_mode(hass: HomeAssistant) -> None:
"""Test setting the HVAC mode."""

Expand Down
Loading