From f6cb2833cacf0c197b358dec9e7758b5b3d09917 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 5 Jul 2022 09:25:30 -0500 Subject: [PATCH] Improve fans in homekit_controller (#74440) --- .../components/homekit_controller/climate.py | 84 +++++++++++------ .../components/homekit_controller/fan.py | 37 +++++++- .../components/homekit_controller/number.py | 71 --------------- .../homekit_controller/fixtures/haa_fan.json | 4 +- .../specific_devices/test_ecobee_501.py | 3 + .../specific_devices/test_haa_fan.py | 8 +- .../homekit_controller/test_climate.py | 30 +++++++ .../components/homekit_controller/test_fan.py | 77 +++++++++++++++- .../homekit_controller/test_number.py | 89 ------------------- 9 files changed, 205 insertions(+), 198 deletions(-) diff --git a/homeassistant/components/homekit_controller/climate.py b/homeassistant/components/homekit_controller/climate.py index 44ecee13875..b76ed1ea6a9 100644 --- a/homeassistant/components/homekit_controller/climate.py +++ b/homeassistant/components/homekit_controller/climate.py @@ -25,6 +25,8 @@ from homeassistant.components.climate.const import ( ATTR_HVAC_MODE, ATTR_TARGET_TEMP_HIGH, ATTR_TARGET_TEMP_LOW, + FAN_AUTO, + FAN_ON, SWING_OFF, SWING_VERTICAL, ClimateEntityFeature, @@ -72,6 +74,7 @@ TARGET_HEATER_COOLER_STATE_HOMEKIT_TO_HASS = { TargetHeaterCoolerStateValues.COOL: HVACMode.COOL, } + # Map of hass operation modes to homekit modes MODE_HASS_TO_HOMEKIT = {v: k for k, v in MODE_HOMEKIT_TO_HASS.items()} @@ -104,19 +107,65 @@ async def async_setup_entry( conn.add_listener(async_add_service) -class HomeKitHeaterCoolerEntity(HomeKitEntity, ClimateEntity): - """Representation of a Homekit climate device.""" +class HomeKitBaseClimateEntity(HomeKitEntity, ClimateEntity): + """The base HomeKit Controller climate entity.""" + + _attr_temperature_unit = TEMP_CELSIUS def get_characteristic_types(self) -> list[str]: """Define the homekit characteristics the entity cares about.""" return [ + CharacteristicsTypes.TEMPERATURE_CURRENT, + CharacteristicsTypes.FAN_STATE_TARGET, + ] + + @property + def current_temperature(self) -> float | None: + """Return the current temperature.""" + return self.service.value(CharacteristicsTypes.TEMPERATURE_CURRENT) + + @property + def fan_modes(self) -> list[str] | None: + """Return the available fan modes.""" + if self.service.has(CharacteristicsTypes.FAN_STATE_TARGET): + return [FAN_ON, FAN_AUTO] + return None + + @property + def fan_mode(self) -> str | None: + """Return the current fan mode.""" + fan_mode = self.service.value(CharacteristicsTypes.FAN_STATE_TARGET) + return FAN_AUTO if fan_mode else FAN_ON + + async def async_set_fan_mode(self, fan_mode: str) -> None: + """Turn fan to manual/auto.""" + await self.async_put_characteristics( + {CharacteristicsTypes.FAN_STATE_TARGET: int(fan_mode == FAN_AUTO)} + ) + + @property + def supported_features(self) -> int: + """Return the list of supported features.""" + features = 0 + + if self.service.has(CharacteristicsTypes.FAN_STATE_TARGET): + features |= ClimateEntityFeature.FAN_MODE + + return features + + +class HomeKitHeaterCoolerEntity(HomeKitBaseClimateEntity): + """Representation of a Homekit climate device.""" + + def get_characteristic_types(self) -> list[str]: + """Define the homekit characteristics the entity cares about.""" + return super().get_characteristic_types() + [ CharacteristicsTypes.ACTIVE, CharacteristicsTypes.CURRENT_HEATER_COOLER_STATE, CharacteristicsTypes.TARGET_HEATER_COOLER_STATE, CharacteristicsTypes.TEMPERATURE_COOLING_THRESHOLD, CharacteristicsTypes.TEMPERATURE_HEATING_THRESHOLD, CharacteristicsTypes.SWING_MODE, - CharacteristicsTypes.TEMPERATURE_CURRENT, ] async def async_set_temperature(self, **kwargs: Any) -> None: @@ -162,11 +211,6 @@ class HomeKitHeaterCoolerEntity(HomeKitEntity, ClimateEntity): } ) - @property - def current_temperature(self) -> float: - """Return the current temperature.""" - return self.service.value(CharacteristicsTypes.TEMPERATURE_CURRENT) - @property def target_temperature(self) -> float | None: """Return the temperature we try to reach.""" @@ -321,7 +365,7 @@ class HomeKitHeaterCoolerEntity(HomeKitEntity, ClimateEntity): @property def supported_features(self) -> int: """Return the list of supported features.""" - features = 0 + features = super().supported_features if self.service.has(CharacteristicsTypes.TEMPERATURE_COOLING_THRESHOLD): features |= ClimateEntityFeature.TARGET_TEMPERATURE @@ -334,22 +378,16 @@ class HomeKitHeaterCoolerEntity(HomeKitEntity, ClimateEntity): return features - @property - def temperature_unit(self) -> str: - """Return the unit of measurement.""" - return TEMP_CELSIUS - -class HomeKitClimateEntity(HomeKitEntity, ClimateEntity): +class HomeKitClimateEntity(HomeKitBaseClimateEntity): """Representation of a Homekit climate device.""" def get_characteristic_types(self) -> list[str]: """Define the homekit characteristics the entity cares about.""" - return [ + return super().get_characteristic_types() + [ CharacteristicsTypes.HEATING_COOLING_CURRENT, CharacteristicsTypes.HEATING_COOLING_TARGET, CharacteristicsTypes.TEMPERATURE_COOLING_THRESHOLD, - CharacteristicsTypes.TEMPERATURE_CURRENT, CharacteristicsTypes.TEMPERATURE_HEATING_THRESHOLD, CharacteristicsTypes.TEMPERATURE_TARGET, CharacteristicsTypes.RELATIVE_HUMIDITY_CURRENT, @@ -411,11 +449,6 @@ class HomeKitClimateEntity(HomeKitEntity, ClimateEntity): } ) - @property - def current_temperature(self) -> float | None: - """Return the current temperature.""" - return self.service.value(CharacteristicsTypes.TEMPERATURE_CURRENT) - @property def target_temperature(self) -> float | None: """Return the temperature we try to reach.""" @@ -558,7 +591,7 @@ class HomeKitClimateEntity(HomeKitEntity, ClimateEntity): @property def supported_features(self) -> int: """Return the list of supported features.""" - features = 0 + features = super().supported_features if self.service.has(CharacteristicsTypes.TEMPERATURE_TARGET): features |= ClimateEntityFeature.TARGET_TEMPERATURE @@ -573,11 +606,6 @@ class HomeKitClimateEntity(HomeKitEntity, ClimateEntity): return features - @property - def temperature_unit(self) -> str: - """Return the unit of measurement.""" - return TEMP_CELSIUS - ENTITY_TYPES = { ServicesTypes.HEATER_COOLER: HomeKitHeaterCoolerEntity, diff --git a/homeassistant/components/homekit_controller/fan.py b/homeassistant/components/homekit_controller/fan.py index 80c2f9870c1..159a1d936fa 100644 --- a/homeassistant/components/homekit_controller/fan.py +++ b/homeassistant/components/homekit_controller/fan.py @@ -15,6 +15,10 @@ from homeassistant.components.fan import ( from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.entity_platform import AddEntitiesCallback +from homeassistant.util.percentage import ( + percentage_to_ranged_value, + ranged_value_to_percentage, +) from . import KNOWN_DEVICES, HomeKitEntity @@ -48,13 +52,32 @@ class BaseHomeKitFan(HomeKitEntity, FanEntity): """Return true if device is on.""" return self.service.value(self.on_characteristic) == 1 + @property + def _speed_range(self) -> tuple[int, int]: + """Return the speed range.""" + return (self._min_speed, self._max_speed) + + @property + def _min_speed(self) -> int: + """Return the minimum speed.""" + return ( + round(self.service[CharacteristicsTypes.ROTATION_SPEED].minValue or 0) + 1 + ) + + @property + def _max_speed(self) -> int: + """Return the minimum speed.""" + return round(self.service[CharacteristicsTypes.ROTATION_SPEED].maxValue or 100) + @property def percentage(self) -> int: """Return the current speed percentage.""" if not self.is_on: return 0 - return self.service.value(CharacteristicsTypes.ROTATION_SPEED) + return ranged_value_to_percentage( + self._speed_range, self.service.value(CharacteristicsTypes.ROTATION_SPEED) + ) @property def current_direction(self) -> str: @@ -88,7 +111,7 @@ class BaseHomeKitFan(HomeKitEntity, FanEntity): def speed_count(self) -> int: """Speed count for the fan.""" return round( - min(self.service[CharacteristicsTypes.ROTATION_SPEED].maxValue or 100, 100) + min(self._max_speed, 100) / max(1, self.service[CharacteristicsTypes.ROTATION_SPEED].minStep or 0) ) @@ -104,7 +127,11 @@ class BaseHomeKitFan(HomeKitEntity, FanEntity): return await self.async_turn_off() await self.async_put_characteristics( - {CharacteristicsTypes.ROTATION_SPEED: percentage} + { + CharacteristicsTypes.ROTATION_SPEED: round( + percentage_to_ranged_value(self._speed_range, percentage) + ) + } ) async def async_oscillate(self, oscillating: bool) -> None: @@ -129,7 +156,9 @@ class BaseHomeKitFan(HomeKitEntity, FanEntity): percentage is not None and self.supported_features & FanEntityFeature.SET_SPEED ): - characteristics[CharacteristicsTypes.ROTATION_SPEED] = percentage + characteristics[CharacteristicsTypes.ROTATION_SPEED] = round( + percentage_to_ranged_value(self._speed_range, percentage) + ) if characteristics: await self.async_put_characteristics(characteristics) diff --git a/homeassistant/components/homekit_controller/number.py b/homeassistant/components/homekit_controller/number.py index 07d22c27314..7a6d0a01ab6 100644 --- a/homeassistant/components/homekit_controller/number.py +++ b/homeassistant/components/homekit_controller/number.py @@ -67,8 +67,6 @@ async def async_setup_entry( if description := NUMBER_ENTITIES.get(char.type): entities.append(HomeKitNumber(conn, info, char, description)) - elif entity_type := NUMBER_ENTITY_CLASSES.get(char.type): - entities.append(entity_type(conn, info, char)) else: return False @@ -130,72 +128,3 @@ class HomeKitNumber(CharacteristicEntity, NumberEntity): self._char.type: value, } ) - - -class HomeKitEcobeeFanModeNumber(CharacteristicEntity, NumberEntity): - """Representation of a Number control for Ecobee Fan Mode request.""" - - def get_characteristic_types(self) -> list[str]: - """Define the homekit characteristics the entity is tracking.""" - return [self._char.type] - - @property - def name(self) -> str: - """Return the name of the device if any.""" - prefix = "" - if name := super().name: - prefix = name - return f"{prefix} Fan Mode" - - @property - def native_min_value(self) -> float: - """Return the minimum value.""" - return self._char.minValue or DEFAULT_MIN_VALUE - - @property - def native_max_value(self) -> float: - """Return the maximum value.""" - return self._char.maxValue or DEFAULT_MAX_VALUE - - @property - def native_step(self) -> float: - """Return the increment/decrement step.""" - return self._char.minStep or DEFAULT_STEP - - @property - def native_value(self) -> float: - """Return the current characteristic value.""" - return self._char.value - - async def async_set_native_value(self, value: float) -> None: - """Set the characteristic to this value.""" - - # Sending the fan mode request sometimes ends up getting ignored by ecobee - # and this might be because it the older value instead of newer, and ecobee - # thinks there is nothing to do. - # So in order to make sure that the request is executed by ecobee, we need - # to send a different value before sending the target value. - # Fan mode value is a value from 0 to 100. We send a value off by 1 first. - - if value > self.min_value: - other_value = value - 1 - else: - other_value = self.min_value + 1 - - if value != other_value: - await self.async_put_characteristics( - { - self._char.type: other_value, - } - ) - - await self.async_put_characteristics( - { - self._char.type: value, - } - ) - - -NUMBER_ENTITY_CLASSES: dict[str, type] = { - CharacteristicsTypes.VENDOR_ECOBEE_FAN_WRITE_SPEED: HomeKitEcobeeFanModeNumber, -} diff --git a/tests/components/homekit_controller/fixtures/haa_fan.json b/tests/components/homekit_controller/fixtures/haa_fan.json index 14a215d01fe..a144a9501ba 100644 --- a/tests/components/homekit_controller/fixtures/haa_fan.json +++ b/tests/components/homekit_controller/fixtures/haa_fan.json @@ -70,7 +70,7 @@ "perms": ["pr", "pw", "ev"], "ev": true, "format": "bool", - "value": false + "value": true }, { "aid": 1, @@ -83,7 +83,7 @@ "minValue": 0, "maxValue": 3, "minStep": 1, - "value": 3 + "value": 2 } ] }, diff --git a/tests/components/homekit_controller/specific_devices/test_ecobee_501.py b/tests/components/homekit_controller/specific_devices/test_ecobee_501.py index 89443008683..ca91607bd09 100644 --- a/tests/components/homekit_controller/specific_devices/test_ecobee_501.py +++ b/tests/components/homekit_controller/specific_devices/test_ecobee_501.py @@ -2,6 +2,7 @@ from homeassistant.components.climate.const import ( + SUPPORT_FAN_MODE, SUPPORT_TARGET_HUMIDITY, SUPPORT_TARGET_TEMPERATURE, SUPPORT_TARGET_TEMPERATURE_RANGE, @@ -43,9 +44,11 @@ async def test_ecobee501_setup(hass): SUPPORT_TARGET_TEMPERATURE | SUPPORT_TARGET_TEMPERATURE_RANGE | SUPPORT_TARGET_HUMIDITY + | SUPPORT_FAN_MODE ), capabilities={ "hvac_modes": ["off", "heat", "cool", "heat_cool"], + "fan_modes": ["on", "auto"], "min_temp": 7.2, "max_temp": 33.3, "min_humidity": 20, diff --git a/tests/components/homekit_controller/specific_devices/test_haa_fan.py b/tests/components/homekit_controller/specific_devices/test_haa_fan.py index 9d5983650d7..39169ea5af9 100644 --- a/tests/components/homekit_controller/specific_devices/test_haa_fan.py +++ b/tests/components/homekit_controller/specific_devices/test_haa_fan.py @@ -1,6 +1,6 @@ """Make sure that a H.A.A. fan can be setup.""" -from homeassistant.components.fan import SUPPORT_SET_SPEED +from homeassistant.components.fan import ATTR_PERCENTAGE, SUPPORT_SET_SPEED from homeassistant.helpers.entity import EntityCategory from tests.components.homekit_controller.common import ( @@ -18,7 +18,9 @@ async def test_haa_fan_setup(hass): accessories = await setup_accessories_from_file(hass, "haa_fan.json") await setup_test_accessories(hass, accessories) - # FIXME: assert round(state.attributes["percentage_step"], 2) == 33.33 + haa_fan_state = hass.states.get("fan.haa_c718b3") + attributes = haa_fan_state.attributes + assert attributes[ATTR_PERCENTAGE] == 66 await assert_devices_and_entities_created( hass, @@ -55,7 +57,7 @@ async def test_haa_fan_setup(hass): entity_id="fan.haa_c718b3", friendly_name="HAA-C718B3", unique_id="homekit-C718B3-1-8", - state="off", + state="on", supported_features=SUPPORT_SET_SPEED, capabilities={ "preset_modes": None, diff --git a/tests/components/homekit_controller/test_climate.py b/tests/components/homekit_controller/test_climate.py index 646804a86d6..c750c428437 100644 --- a/tests/components/homekit_controller/test_climate.py +++ b/tests/components/homekit_controller/test_climate.py @@ -10,6 +10,7 @@ from aiohomekit.model.services import ServicesTypes from homeassistant.components.climate.const import ( DOMAIN, + SERVICE_SET_FAN_MODE, SERVICE_SET_HUMIDITY, SERVICE_SET_HVAC_MODE, SERVICE_SET_SWING_MODE, @@ -32,6 +33,9 @@ def create_thermostat_service(accessory): char = service.add_char(CharacteristicsTypes.HEATING_COOLING_CURRENT) char.value = 0 + char = service.add_char(CharacteristicsTypes.FAN_STATE_TARGET) + char.value = 0 + char = service.add_char(CharacteristicsTypes.TEMPERATURE_COOLING_THRESHOLD) char.minValue = 15 char.maxValue = 40 @@ -144,6 +148,32 @@ async def test_climate_change_thermostat_state(hass, utcnow): }, ) + await hass.services.async_call( + DOMAIN, + SERVICE_SET_FAN_MODE, + {"entity_id": "climate.testdevice", "fan_mode": "on"}, + blocking=True, + ) + helper.async_assert_service_values( + ServicesTypes.THERMOSTAT, + { + CharacteristicsTypes.FAN_STATE_TARGET: 0, + }, + ) + + await hass.services.async_call( + DOMAIN, + SERVICE_SET_FAN_MODE, + {"entity_id": "climate.testdevice", "fan_mode": "auto"}, + blocking=True, + ) + helper.async_assert_service_values( + ServicesTypes.THERMOSTAT, + { + CharacteristicsTypes.FAN_STATE_TARGET: 1, + }, + ) + async def test_climate_check_min_max_values_per_mode(hass, utcnow): """Test that we we get the appropriate min/max values for each mode.""" diff --git a/tests/components/homekit_controller/test_fan.py b/tests/components/homekit_controller/test_fan.py index faaaa2e666f..9d166531562 100644 --- a/tests/components/homekit_controller/test_fan.py +++ b/tests/components/homekit_controller/test_fan.py @@ -1,4 +1,4 @@ -"""Basic checks for HomeKit motion sensors and contact sensors.""" +"""Basic checks for HomeKit fans.""" from aiohomekit.model.characteristics import CharacteristicsTypes from aiohomekit.model.services import ServicesTypes @@ -41,6 +41,20 @@ def create_fanv2_service(accessory): swing_mode.value = 0 +def create_fanv2_service_non_standard_rotation_range(accessory): + """Define fan v2 with a non-standard rotation range.""" + service = accessory.add_service(ServicesTypes.FAN_V2) + + cur_state = service.add_char(CharacteristicsTypes.ACTIVE) + cur_state.value = 0 + + speed = service.add_char(CharacteristicsTypes.ROTATION_SPEED) + speed.value = 0 + speed.minValue = 0 + speed.maxValue = 3 + speed.minStep = 1 + + def create_fanv2_service_with_min_step(accessory): """Define fan v2 characteristics as per HAP spec.""" service = accessory.add_service(ServicesTypes.FAN_V2) @@ -730,3 +744,64 @@ async def test_v2_oscillate_read(hass, utcnow): ServicesTypes.FAN_V2, {CharacteristicsTypes.SWING_MODE: 1} ) assert state.attributes["oscillating"] is True + + +async def test_v2_set_percentage_non_standard_rotation_range(hass, utcnow): + """Test that we set fan speed with a non-standard rotation range.""" + helper = await setup_test_component( + hass, create_fanv2_service_non_standard_rotation_range + ) + + await helper.async_update(ServicesTypes.FAN_V2, {CharacteristicsTypes.ACTIVE: 1}) + + await hass.services.async_call( + "fan", + "set_percentage", + {"entity_id": "fan.testdevice", "percentage": 100}, + blocking=True, + ) + helper.async_assert_service_values( + ServicesTypes.FAN_V2, + { + CharacteristicsTypes.ROTATION_SPEED: 3, + }, + ) + + await hass.services.async_call( + "fan", + "set_percentage", + {"entity_id": "fan.testdevice", "percentage": 66}, + blocking=True, + ) + helper.async_assert_service_values( + ServicesTypes.FAN_V2, + { + CharacteristicsTypes.ROTATION_SPEED: 2, + }, + ) + + await hass.services.async_call( + "fan", + "set_percentage", + {"entity_id": "fan.testdevice", "percentage": 33}, + blocking=True, + ) + helper.async_assert_service_values( + ServicesTypes.FAN_V2, + { + CharacteristicsTypes.ROTATION_SPEED: 1, + }, + ) + + await hass.services.async_call( + "fan", + "set_percentage", + {"entity_id": "fan.testdevice", "percentage": 0}, + blocking=True, + ) + helper.async_assert_service_values( + ServicesTypes.FAN_V2, + { + CharacteristicsTypes.ACTIVE: 0, + }, + ) diff --git a/tests/components/homekit_controller/test_number.py b/tests/components/homekit_controller/test_number.py index 78bdb394f0c..6b375b60d9b 100644 --- a/tests/components/homekit_controller/test_number.py +++ b/tests/components/homekit_controller/test_number.py @@ -26,26 +26,6 @@ def create_switch_with_spray_level(accessory): return service -def create_switch_with_ecobee_fan_mode(accessory): - """Define battery level characteristics.""" - service = accessory.add_service(ServicesTypes.OUTLET) - - ecobee_fan_mode = service.add_char( - CharacteristicsTypes.VENDOR_ECOBEE_FAN_WRITE_SPEED - ) - - ecobee_fan_mode.value = 0 - ecobee_fan_mode.minStep = 1 - ecobee_fan_mode.minValue = 0 - ecobee_fan_mode.maxValue = 100 - ecobee_fan_mode.format = "float" - - cur_state = service.add_char(CharacteristicsTypes.ON) - cur_state.value = True - - return service - - async def test_read_number(hass, utcnow): """Test a switch service that has a sensor characteristic is correctly handled.""" helper = await setup_test_component(hass, create_switch_with_spray_level) @@ -106,72 +86,3 @@ async def test_write_number(hass, utcnow): ServicesTypes.OUTLET, {CharacteristicsTypes.VENDOR_VOCOLINC_HUMIDIFIER_SPRAY_LEVEL: 3}, ) - - -async def test_write_ecobee_fan_mode_number(hass, utcnow): - """Test a switch service that has a sensor characteristic is correctly handled.""" - helper = await setup_test_component(hass, create_switch_with_ecobee_fan_mode) - - # Helper will be for the primary entity, which is the outlet. Make a helper for the sensor. - fan_mode = Helper( - hass, - "number.testdevice_fan_mode", - helper.pairing, - helper.accessory, - helper.config_entry, - ) - - await hass.services.async_call( - "number", - "set_value", - {"entity_id": "number.testdevice_fan_mode", "value": 1}, - blocking=True, - ) - fan_mode.async_assert_service_values( - ServicesTypes.OUTLET, - {CharacteristicsTypes.VENDOR_ECOBEE_FAN_WRITE_SPEED: 1}, - ) - - await hass.services.async_call( - "number", - "set_value", - {"entity_id": "number.testdevice_fan_mode", "value": 2}, - blocking=True, - ) - fan_mode.async_assert_service_values( - ServicesTypes.OUTLET, - {CharacteristicsTypes.VENDOR_ECOBEE_FAN_WRITE_SPEED: 2}, - ) - - await hass.services.async_call( - "number", - "set_value", - {"entity_id": "number.testdevice_fan_mode", "value": 99}, - blocking=True, - ) - fan_mode.async_assert_service_values( - ServicesTypes.OUTLET, - {CharacteristicsTypes.VENDOR_ECOBEE_FAN_WRITE_SPEED: 99}, - ) - - await hass.services.async_call( - "number", - "set_value", - {"entity_id": "number.testdevice_fan_mode", "value": 100}, - blocking=True, - ) - fan_mode.async_assert_service_values( - ServicesTypes.OUTLET, - {CharacteristicsTypes.VENDOR_ECOBEE_FAN_WRITE_SPEED: 100}, - ) - - await hass.services.async_call( - "number", - "set_value", - {"entity_id": "number.testdevice_fan_mode", "value": 0}, - blocking=True, - ) - fan_mode.async_assert_service_values( - ServicesTypes.OUTLET, - {CharacteristicsTypes.VENDOR_ECOBEE_FAN_WRITE_SPEED: 0}, - )