Fixed race condition in Generic Thermostat (#15784)

* Fixed race condition in Generic Thermostat

* Added a comment to clarify the meaning of the `time` argument.
pull/15949/head
Lev Aronsky 2018-08-12 23:28:47 +03:00 committed by Anders Melchiorsen
parent e8218c4b29
commit b7486e5605
1 changed files with 58 additions and 82 deletions

View File

@ -123,6 +123,7 @@ class GenericThermostat(ClimateDevice):
self._enabled = True self._enabled = True
self._active = False self._active = False
self._cur_temp = None self._cur_temp = None
self._temp_lock = asyncio.Lock()
self._min_temp = min_temp self._min_temp = min_temp
self._max_temp = max_temp self._max_temp = max_temp
self._target_temp = target_temp self._target_temp = target_temp
@ -140,7 +141,7 @@ class GenericThermostat(ClimateDevice):
if self._keep_alive: if self._keep_alive:
async_track_time_interval( async_track_time_interval(
hass, self._async_keep_alive, self._keep_alive) hass, self._async_control_heating, self._keep_alive)
sensor_state = hass.states.get(sensor_entity_id) sensor_state = hass.states.get(sensor_entity_id)
if sensor_state and sensor_state.state != STATE_UNKNOWN: if sensor_state and sensor_state.state != STATE_UNKNOWN:
@ -234,31 +235,30 @@ class GenericThermostat(ClimateDevice):
if operation_mode == STATE_HEAT: if operation_mode == STATE_HEAT:
self._current_operation = STATE_HEAT self._current_operation = STATE_HEAT
self._enabled = True self._enabled = True
self._async_control_heating() await self._async_control_heating()
elif operation_mode == STATE_COOL: elif operation_mode == STATE_COOL:
self._current_operation = STATE_COOL self._current_operation = STATE_COOL
self._enabled = True self._enabled = True
self._async_control_heating() await self._async_control_heating()
elif operation_mode == STATE_OFF: elif operation_mode == STATE_OFF:
self._current_operation = STATE_OFF self._current_operation = STATE_OFF
self._enabled = False self._enabled = False
if self._is_device_active: if self._is_device_active:
self._heater_turn_off() await self._async_heater_turn_off()
else: else:
_LOGGER.error("Unrecognized operation mode: %s", operation_mode) _LOGGER.error("Unrecognized operation mode: %s", operation_mode)
return return
# Ensure we update the current operation after changing the mode # Ensure we update the current operation after changing the mode
self.schedule_update_ha_state() self.schedule_update_ha_state()
@asyncio.coroutine async def async_set_temperature(self, **kwargs):
def async_set_temperature(self, **kwargs):
"""Set new target temperature.""" """Set new target temperature."""
temperature = kwargs.get(ATTR_TEMPERATURE) temperature = kwargs.get(ATTR_TEMPERATURE)
if temperature is None: if temperature is None:
return return
self._target_temp = temperature self._target_temp = temperature
self._async_control_heating() await self._async_control_heating()
yield from self.async_update_ha_state() await self.async_update_ha_state()
@property @property
def min_temp(self): def min_temp(self):
@ -278,15 +278,14 @@ class GenericThermostat(ClimateDevice):
# Get default temp from super class # Get default temp from super class
return super().max_temp return super().max_temp
@asyncio.coroutine async def _async_sensor_changed(self, entity_id, old_state, new_state):
def _async_sensor_changed(self, entity_id, old_state, new_state):
"""Handle temperature changes.""" """Handle temperature changes."""
if new_state is None: if new_state is None:
return return
self._async_update_temp(new_state) self._async_update_temp(new_state)
self._async_control_heating() await self._async_control_heating()
yield from self.async_update_ha_state() await self.async_update_ha_state()
@callback @callback
def _async_switch_changed(self, entity_id, old_state, new_state): def _async_switch_changed(self, entity_id, old_state, new_state):
@ -295,14 +294,6 @@ class GenericThermostat(ClimateDevice):
return return
self.async_schedule_update_ha_state() self.async_schedule_update_ha_state()
@callback
def _async_keep_alive(self, time):
"""Call at constant intervals for keep-alive purposes."""
if self._is_device_active:
self._heater_turn_on()
else:
self._heater_turn_off()
@callback @callback
def _async_update_temp(self, state): def _async_update_temp(self, state):
"""Update thermostat with latest state from sensor.""" """Update thermostat with latest state from sensor."""
@ -314,9 +305,9 @@ class GenericThermostat(ClimateDevice):
except ValueError as ex: except ValueError as ex:
_LOGGER.error("Unable to update from sensor: %s", ex) _LOGGER.error("Unable to update from sensor: %s", ex)
@callback async def _async_control_heating(self, time=None):
def _async_control_heating(self):
"""Check if we need to turn heating on or off.""" """Check if we need to turn heating on or off."""
async with self._temp_lock:
if not self._active and None not in (self._cur_temp, if not self._active and None not in (self._cur_temp,
self._target_temp): self._target_temp):
self._active = True self._active = True
@ -324,10 +315,7 @@ class GenericThermostat(ClimateDevice):
"Generic thermostat active. %s, %s", "Generic thermostat active. %s, %s",
self._cur_temp, self._target_temp) self._cur_temp, self._target_temp)
if not self._active: if not self._active or not self._enabled:
return
if not self._enabled:
return return
if self.min_cycle_duration: if self.min_cycle_duration:
@ -341,35 +329,27 @@ class GenericThermostat(ClimateDevice):
if not long_enough: if not long_enough:
return return
if self.ac_mode: too_cold = \
is_cooling = self._is_device_active self._target_temp - self._cur_temp >= self._cold_tolerance
if is_cooling: too_hot = \
too_cold = self._target_temp - self._cur_temp >= \ self._cur_temp - self._target_temp >= self._hot_tolerance
self._cold_tolerance if self._is_device_active:
if too_cold: if (self.ac_mode and too_cold) or \
_LOGGER.info("Turning off AC %s", self.heater_entity_id) (not self.ac_mode and too_hot):
self._heater_turn_off()
else:
too_hot = self._cur_temp - self._target_temp >= \
self._hot_tolerance
if too_hot:
_LOGGER.info("Turning on AC %s", self.heater_entity_id)
self._heater_turn_on()
else:
is_heating = self._is_device_active
if is_heating:
too_hot = self._cur_temp - self._target_temp >= \
self._hot_tolerance
if too_hot:
_LOGGER.info("Turning off heater %s", _LOGGER.info("Turning off heater %s",
self.heater_entity_id) self.heater_entity_id)
self._heater_turn_off() await self._async_heater_turn_off()
elif time is not None:
# The time argument is passed only in keep-alive case
await self._async_heater_turn_on()
else: else:
too_cold = self._target_temp - self._cur_temp >= \ if (self.ac_mode and too_hot) or \
self._cold_tolerance (not self.ac_mode and too_cold):
if too_cold:
_LOGGER.info("Turning on heater %s", self.heater_entity_id) _LOGGER.info("Turning on heater %s", self.heater_entity_id)
self._heater_turn_on() await self._async_heater_turn_on()
elif time is not None:
# The time argument is passed only in keep-alive case
await self._async_heater_turn_off()
@property @property
def _is_device_active(self): def _is_device_active(self):
@ -381,36 +361,32 @@ class GenericThermostat(ClimateDevice):
"""Return the list of supported features.""" """Return the list of supported features."""
return self._support_flags return self._support_flags
@callback async def _async_heater_turn_on(self):
def _heater_turn_on(self):
"""Turn heater toggleable device on.""" """Turn heater toggleable device on."""
data = {ATTR_ENTITY_ID: self.heater_entity_id} data = {ATTR_ENTITY_ID: self.heater_entity_id}
self.hass.async_add_job( await self.hass.services.async_call(HA_DOMAIN, SERVICE_TURN_ON, data)
self.hass.services.async_call(HA_DOMAIN, SERVICE_TURN_ON, data))
@callback async def _async_heater_turn_off(self):
def _heater_turn_off(self):
"""Turn heater toggleable device off.""" """Turn heater toggleable device off."""
data = {ATTR_ENTITY_ID: self.heater_entity_id} data = {ATTR_ENTITY_ID: self.heater_entity_id}
self.hass.async_add_job( await self.hass.services.async_call(HA_DOMAIN, SERVICE_TURN_OFF, data)
self.hass.services.async_call(HA_DOMAIN, SERVICE_TURN_OFF, data))
@property @property
def is_away_mode_on(self): def is_away_mode_on(self):
"""Return true if away mode is on.""" """Return true if away mode is on."""
return self._is_away return self._is_away
def turn_away_mode_on(self): async def async_turn_away_mode_on(self):
"""Turn away mode on by setting it on away hold indefinitely.""" """Turn away mode on by setting it on away hold indefinitely."""
self._is_away = True self._is_away = True
self._saved_target_temp = self._target_temp self._saved_target_temp = self._target_temp
self._target_temp = self._away_temp self._target_temp = self._away_temp
self._async_control_heating() await self._async_control_heating()
self.schedule_update_ha_state() await self.async_update_ha_state()
def turn_away_mode_off(self): async def async_turn_away_mode_off(self):
"""Turn away off.""" """Turn away off."""
self._is_away = False self._is_away = False
self._target_temp = self._saved_target_temp self._target_temp = self._saved_target_temp
self._async_control_heating() await self._async_control_heating()
self.schedule_update_ha_state() await self.async_update_ha_state()