From 92fd3e3ad5eb611c9d0dbce92ab5d5c94b3317e8 Mon Sep 17 00:00:00 2001 From: Fazli Sapuan Date: Fri, 20 Dec 2019 18:00:21 +0800 Subject: [PATCH] Fix homekit handling of 0 light brightness and fan speed (#29962) * Fix homekit handling of 0 light brightness and fan speed * Update homekit tests for new initial brightness/speed value --- homeassistant/components/homekit/type_fans.py | 22 +++++++++++++++++-- .../components/homekit/type_lights.py | 21 ++++++++++++++++-- tests/components/homekit/test_type_fans.py | 5 ++++- tests/components/homekit/test_type_lights.py | 4 +++- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/homekit/type_fans.py b/homeassistant/components/homekit/type_fans.py index e3fa6c42c58..e6d128d1e28 100644 --- a/homeassistant/components/homekit/type_fans.py +++ b/homeassistant/components/homekit/type_fans.py @@ -88,8 +88,11 @@ class Fan(HomeAccessory): ) if CHAR_ROTATION_SPEED in chars: + # Initial value is set to 100 because 0 is a special value (off). 100 is + # an arbitrary non-zero value. It is updated immediately by update_state + # to set to the correct initial value. self.char_speed = serv_fan.configure_char( - CHAR_ROTATION_SPEED, value=0, setter_callback=self.set_speed + CHAR_ROTATION_SPEED, value=100, setter_callback=self.set_speed ) if CHAR_SWING_MODE in chars: @@ -156,7 +159,22 @@ class Fan(HomeAccessory): speed = new_state.attributes.get(ATTR_SPEED) hk_speed_value = self.speed_mapping.speed_to_homekit(speed) if hk_speed_value is not None and self.char_speed.value != hk_speed_value: - self.char_speed.set_value(hk_speed_value) + # If the homeassistant component reports its speed as the first entry + # in its speed list but is not off, the hk_speed_value is 0. But 0 + # is a special value in homekit. When you turn on a homekit accessory + # it will try to restore the last rotation speed state which will be + # the last value saved by char_speed.set_value. But if it is set to + # 0, HomeKit will update the rotation speed to 100 as it thinks 0 is + # off. + # + # Therefore, if the hk_speed_value is 0 and the device is still on, + # the rotation speed is mapped to 1 otherwise the update is ignored + # in order to avoid this incorrect behavior. + if hk_speed_value == 0: + if state == STATE_ON: + self.char_speed.set_value(1) + else: + self.char_speed.set_value(hk_speed_value) # Handle Oscillating if self.char_swing is not None: diff --git a/homeassistant/components/homekit/type_lights.py b/homeassistant/components/homekit/type_lights.py index 8e1b07fbbff..7f195b276d6 100644 --- a/homeassistant/components/homekit/type_lights.py +++ b/homeassistant/components/homekit/type_lights.py @@ -82,8 +82,11 @@ class Light(HomeAccessory): ) if CHAR_BRIGHTNESS in self.chars: + # Initial value is set to 100 because 0 is a special value (off). 100 is + # an arbitrary non-zero value. It is updated immediately by update_state + # to set to the correct initial value. self.char_brightness = serv_light.configure_char( - CHAR_BRIGHTNESS, value=0, setter_callback=self.set_brightness + CHAR_BRIGHTNESS, value=100, setter_callback=self.set_brightness ) if CHAR_COLOR_TEMPERATURE in self.chars: min_mireds = self.hass.states.get(self.entity_id).attributes.get( @@ -183,7 +186,21 @@ class Light(HomeAccessory): if not self._flag[CHAR_BRIGHTNESS] and isinstance(brightness, int): brightness = round(brightness / 255 * 100, 0) if self.char_brightness.value != brightness: - self.char_brightness.set_value(brightness) + # The homeassistant component might report its brightness as 0 but is + # not off. But 0 is a special value in homekit. When you turn on a + # homekit accessory it will try to restore the last brightness state + # which will be the last value saved by char_brightness.set_value. + # But if it is set to 0, HomeKit will update the brightness to 100 as + # it thinks 0 is off. + # + # Therefore, if the the brighness is 0 and the device is still on, + # the brightness is mapped to 1 otherwise the update is ignored in + # order to avoid this incorrect behavior. + if brightness == 0: + if state == STATE_ON: + self.char_brightness.set_value(1) + else: + self.char_brightness.set_value(brightness) self._flag[CHAR_BRIGHTNESS] = False # Handle color temperature diff --git a/tests/components/homekit/test_type_fans.py b/tests/components/homekit/test_type_fans.py index 781df6cfb7b..5631791d7a2 100644 --- a/tests/components/homekit/test_type_fans.py +++ b/tests/components/homekit/test_type_fans.py @@ -197,7 +197,10 @@ async def test_fan_speed(hass, hk_driver, cls, events): ) await hass.async_block_till_done() acc = cls.fan(hass, hk_driver, "Fan", entity_id, 2, None) - assert acc.char_speed.value == 0 + + # Initial value can be anything but 0. If it is 0, it might cause HomeKit to set the + # speed to 100 when turning on a fan on a freshly booted up server. + assert acc.char_speed.value != 0 await hass.async_add_job(acc.run) assert ( diff --git a/tests/components/homekit/test_type_lights.py b/tests/components/homekit/test_type_lights.py index 3444b2778f6..510cfa4f666 100644 --- a/tests/components/homekit/test_type_lights.py +++ b/tests/components/homekit/test_type_lights.py @@ -101,7 +101,9 @@ async def test_light_brightness(hass, hk_driver, cls, events): await hass.async_block_till_done() acc = cls.light(hass, hk_driver, "Light", entity_id, 2, None) - assert acc.char_brightness.value == 0 + # Initial value can be anything but 0. If it is 0, it might cause HomeKit to set the + # brightness to 100 when turning on a light on a freshly booted up server. + assert acc.char_brightness.value != 0 await hass.async_add_job(acc.run) await hass.async_block_till_done()