From 369ffe228847f05ac062bb6334406bde6dbfc7d2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 27 Mar 2020 23:02:51 -0500 Subject: [PATCH] Fix setting HomeKit temperatures with imperial units (#33315) In PR#24804 the step size was changed to match the step size of the device that HomeKit is controlling. Since HomeKit is always working in TEMP_CELSIUS and the step size is based on the Home Assistant units setting, we were mixing data with different units of measure when Home Assistant was using imperial units. This regression presented the symptom that setting the temperature to 73F would result in 74F. Other values are affected where the math doesn't happen to work out. HomeKit currently has a default of 0.1 in the spec and the override of this value has been removed. --- .../components/homekit/type_thermostats.py | 42 ++++++++----------- .../homekit/test_type_thermostats.py | 10 ++--- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/homeassistant/components/homekit/type_thermostats.py b/homeassistant/components/homekit/type_thermostats.py index 79a9d156f10..19f7899d79b 100644 --- a/homeassistant/components/homekit/type_thermostats.py +++ b/homeassistant/components/homekit/type_thermostats.py @@ -12,7 +12,6 @@ from homeassistant.components.climate.const import ( ATTR_MIN_TEMP, ATTR_TARGET_TEMP_HIGH, ATTR_TARGET_TEMP_LOW, - ATTR_TARGET_TEMP_STEP, CURRENT_HVAC_COOL, CURRENT_HVAC_HEAT, CURRENT_HVAC_IDLE, @@ -55,7 +54,6 @@ from .const import ( DEFAULT_MAX_TEMP_WATER_HEATER, DEFAULT_MIN_TEMP_WATER_HEATER, PROP_MAX_VALUE, - PROP_MIN_STEP, PROP_MIN_VALUE, SERV_THERMOSTAT, ) @@ -67,6 +65,7 @@ HC_HOMEKIT_VALID_MODES_WATER_HEATER = { "Heat": 1, } UNIT_HASS_TO_HOMEKIT = {TEMP_CELSIUS: 0, TEMP_FAHRENHEIT: 1} + UNIT_HOMEKIT_TO_HASS = {c: s for s, c in UNIT_HASS_TO_HOMEKIT.items()} HC_HASS_TO_HOMEKIT = { HVAC_MODE_OFF: 0, @@ -99,9 +98,6 @@ class Thermostat(HomeAccessory): self._flag_coolingthresh = False self._flag_heatingthresh = False min_temp, max_temp = self.get_temperature_range() - temp_step = self.hass.states.get(self.entity_id).attributes.get( - ATTR_TARGET_TEMP_STEP, 0.5 - ) # Add additional characteristics if auto mode is supported self.chars = [] @@ -162,11 +158,10 @@ class Thermostat(HomeAccessory): self.char_target_temp = serv_thermostat.configure_char( CHAR_TARGET_TEMPERATURE, value=21.0, - properties={ - PROP_MIN_VALUE: min_temp, - PROP_MAX_VALUE: max_temp, - PROP_MIN_STEP: temp_step, - }, + # We do not set PROP_MIN_STEP here and instead use the HomeKit + # default of 0.1 in order to have enough precision to convert + # temperature units and avoid setting to 73F will result in 74F + properties={PROP_MIN_VALUE: min_temp, PROP_MAX_VALUE: max_temp}, setter_callback=self.set_target_temperature, ) @@ -182,22 +177,20 @@ class Thermostat(HomeAccessory): self.char_cooling_thresh_temp = serv_thermostat.configure_char( CHAR_COOLING_THRESHOLD_TEMPERATURE, value=23.0, - properties={ - PROP_MIN_VALUE: min_temp, - PROP_MAX_VALUE: max_temp, - PROP_MIN_STEP: temp_step, - }, + # We do not set PROP_MIN_STEP here and instead use the HomeKit + # default of 0.1 in order to have enough precision to convert + # temperature units and avoid setting to 73F will result in 74F + properties={PROP_MIN_VALUE: min_temp, PROP_MAX_VALUE: max_temp}, setter_callback=self.set_cooling_threshold, ) if CHAR_HEATING_THRESHOLD_TEMPERATURE in self.chars: self.char_heating_thresh_temp = serv_thermostat.configure_char( CHAR_HEATING_THRESHOLD_TEMPERATURE, value=19.0, - properties={ - PROP_MIN_VALUE: min_temp, - PROP_MAX_VALUE: max_temp, - PROP_MIN_STEP: temp_step, - }, + # We do not set PROP_MIN_STEP here and instead use the HomeKit + # default of 0.1 in order to have enough precision to convert + # temperature units and avoid setting to 73F will result in 74F + properties={PROP_MIN_VALUE: min_temp, PROP_MAX_VALUE: max_temp}, setter_callback=self.set_heating_threshold, ) @@ -370,11 +363,10 @@ class WaterHeater(HomeAccessory): self.char_target_temp = serv_thermostat.configure_char( CHAR_TARGET_TEMPERATURE, value=50.0, - properties={ - PROP_MIN_VALUE: min_temp, - PROP_MAX_VALUE: max_temp, - PROP_MIN_STEP: 0.5, - }, + # We do not set PROP_MIN_STEP here and instead use the HomeKit + # default of 0.1 in order to have enough precision to convert + # temperature units and avoid setting to 73F will result in 74F + properties={PROP_MIN_VALUE: min_temp, PROP_MAX_VALUE: max_temp}, setter_callback=self.set_target_temperature, ) diff --git a/tests/components/homekit/test_type_thermostats.py b/tests/components/homekit/test_type_thermostats.py index c96cfdae602..df9a10fc409 100644 --- a/tests/components/homekit/test_type_thermostats.py +++ b/tests/components/homekit/test_type_thermostats.py @@ -102,7 +102,7 @@ async def test_thermostat(hass, hk_driver, cls, events): assert acc.char_target_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP assert acc.char_target_temp.properties[PROP_MIN_VALUE] == DEFAULT_MIN_TEMP - assert acc.char_target_temp.properties[PROP_MIN_STEP] == 0.5 + assert acc.char_target_temp.properties[PROP_MIN_STEP] == 0.1 hass.states.async_set( entity_id, @@ -276,10 +276,10 @@ async def test_thermostat_auto(hass, hk_driver, cls, events): assert acc.char_cooling_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP assert acc.char_cooling_thresh_temp.properties[PROP_MIN_VALUE] == DEFAULT_MIN_TEMP - assert acc.char_cooling_thresh_temp.properties[PROP_MIN_STEP] == 0.5 + assert acc.char_cooling_thresh_temp.properties[PROP_MIN_STEP] == 0.1 assert acc.char_heating_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP assert acc.char_heating_thresh_temp.properties[PROP_MIN_VALUE] == DEFAULT_MIN_TEMP - assert acc.char_heating_thresh_temp.properties[PROP_MIN_STEP] == 0.5 + assert acc.char_heating_thresh_temp.properties[PROP_MIN_STEP] == 0.1 hass.states.async_set( entity_id, @@ -517,7 +517,7 @@ async def test_thermostat_temperature_step_whole(hass, hk_driver, cls): await hass.async_add_job(acc.run) await hass.async_block_till_done() - assert acc.char_target_temp.properties[PROP_MIN_STEP] == 1.0 + assert acc.char_target_temp.properties[PROP_MIN_STEP] == 0.1 async def test_thermostat_restore(hass, hk_driver, cls, events): @@ -618,7 +618,7 @@ async def test_water_heater(hass, hk_driver, cls, events): assert ( acc.char_target_temp.properties[PROP_MIN_VALUE] == DEFAULT_MIN_TEMP_WATER_HEATER ) - assert acc.char_target_temp.properties[PROP_MIN_STEP] == 0.5 + assert acc.char_target_temp.properties[PROP_MIN_STEP] == 0.1 hass.states.async_set( entity_id,