Improve fans in homekit_controller (#74440)

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):
+class HomeKitBaseClimateEntity(HomeKitEntity, ClimateEntity):
-    """Representation of a Homekit climate device."""
+    """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(HomeKitBaseClimateEntity):
-class HomeKitClimateEntity(HomeKitEntity, ClimateEntity):
     """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,

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)

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,
-}

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
           }
         ]
       },

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,

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,

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."""

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,
+        },
+    )

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},
-    )