From 08d08e52de0bc8963b634e319a5d52b6d19db10a Mon Sep 17 00:00:00 2001 From: Jean-Marc Collin Date: Wed, 15 Feb 2023 23:44:09 +0100 Subject: [PATCH] FIX A thermostat stays with security_default_on_percent when the preset change during security mode #49 --- .../versatile_thermostat/climate.py | 70 ++++--- .../versatile_thermostat/tests/commons.py | 25 ++- .../versatile_thermostat/tests/const.py | 2 + .../tests/test_security.py | 180 ++++++++++++++++++ .../versatile_thermostat/tests/test_start.py | 43 +++-- 5 files changed, 276 insertions(+), 44 deletions(-) create mode 100644 custom_components/versatile_thermostat/tests/test_security.py diff --git a/custom_components/versatile_thermostat/climate.py b/custom_components/versatile_thermostat/climate.py index 0fb7f0f8..bd19cf74 100644 --- a/custom_components/versatile_thermostat/climate.py +++ b/custom_components/versatile_thermostat/climate.py @@ -13,6 +13,8 @@ callback, CoreState, DOMAIN as HA_DOMAIN, + Event, + State, ) from homeassistant.components.climate import ClimateEntity @@ -197,6 +199,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): # The list of VersatileThermostat entities # No more needed # _registry: dict[str, object] = {} + _last_temperature_mesure: datetime + _last_ext_temperature_mesure: datetime def __init__(self, hass: HomeAssistant, unique_id, name, entry_infos) -> None: """Initialize the thermostat.""" @@ -343,8 +347,8 @@ def post_init(self, entry_infos): self._presets_away, ) # Will be restored if possible - self._attr_preset_mode = None - self._saved_preset_mode = None + self._attr_preset_mode = PRESET_NONE + self._saved_preset_mode = PRESET_NONE # Power management self._device_power = entry_infos.get(CONF_DEVICE_POWER) @@ -1076,6 +1080,16 @@ async def _async_set_preset_mode_internal(self, preset_mode, force=False): if preset_mode == self._attr_preset_mode and not force: # I don't think we need to call async_write_ha_state if we didn't change the state return + + # In security mode don't change preset but memorise the new expected preset when security will be off + if preset_mode != PRESET_SECURITY and self._security_state: + _LOGGER.debug( + "%s - is in security mode. Just memorise the new expected ", self + ) + if preset_mode not in HIDDEN_PRESETS: + self._saved_preset_mode = preset_mode + return + old_preset_mode = self._attr_preset_mode if preset_mode == PRESET_NONE: self._attr_preset_mode = PRESET_NONE @@ -1213,9 +1227,9 @@ async def entry_update_listener( _LOGGER.info("%s - Change entry with the values: %s", self, config_entry.data) @callback - async def _async_temperature_changed(self, event): - """Handle temperature changes.""" - new_state = event.data.get("new_state") + async def _async_temperature_changed(self, event: Event): + """Handle temperature of the temperature sensor changes.""" + new_state: State = event.data.get("new_state") _LOGGER.debug( "%s - Temperature changed. Event.new_state is %s", self, @@ -1228,9 +1242,9 @@ async def _async_temperature_changed(self, event): self.recalculate() await self._async_control_heating(force=False) - async def _async_ext_temperature_changed(self, event): - """Handle external temperature changes.""" - new_state = event.data.get("new_state") + async def _async_ext_temperature_changed(self, event: Event): + """Handle external temperature opf the sensor changes.""" + new_state: State = event.data.get("new_state") _LOGGER.debug( "%s - external Temperature changed. Event.new_state is %s", self, @@ -1409,14 +1423,16 @@ async def _async_climate_changed(self, event): await self._async_control_heating(True) @callback - async def _async_update_temp(self, state): + async def _async_update_temp(self, state: State): """Update thermostat with latest state from sensor.""" try: cur_temp = float(state.state) if math.isnan(cur_temp) or math.isinf(cur_temp): raise ValueError(f"Sensor has illegal state {state.state}") self._cur_temp = cur_temp - self._last_temperature_mesure = datetime.now() + self._last_temperature_mesure = ( + state.last_changed if state.last_changed is not None else datetime.now() + ) # try to restart if we were in security mode if self._security_state: await self.check_security() @@ -1425,14 +1441,16 @@ async def _async_update_temp(self, state): _LOGGER.error("Unable to update temperature from sensor: %s", ex) @callback - async def _async_update_ext_temp(self, state): + async def _async_update_ext_temp(self, state: State): """Update thermostat with latest state from sensor.""" try: cur_ext_temp = float(state.state) if math.isnan(cur_ext_temp) or math.isinf(cur_ext_temp): raise ValueError(f"Sensor has illegal state {state.state}") self._cur_ext_temp = cur_ext_temp - self._last_ext_temperature_mesure = datetime.now() + self._last_ext_temperature_mesure = ( + state.last_changed if state.last_changed is not None else datetime.now() + ) # try to restart if we were in security mode if self._security_state: await self.check_security() @@ -1713,9 +1731,11 @@ async def check_overpowering(self) -> bool: async def check_security(self) -> bool: """Check if last temperature date is too long""" now = datetime.now() - delta_temp = (now - self._last_temperature_mesure).total_seconds() / 60.0 + delta_temp = ( + now - self._last_temperature_mesure.replace(tzinfo=None) + ).total_seconds() / 60.0 delta_ext_temp = ( - now - self._last_ext_temperature_mesure + now - self._last_ext_temperature_mesure.replace(tzinfo=None) ).total_seconds() / 60.0 mode_cond = self._is_over_climate or self._hvac_mode != HVACMode.OFF @@ -1735,6 +1755,17 @@ async def check_security(self) -> bool: >= self._security_min_on_percent ) + _LOGGER.debug( + "%s - checking security delta_temp=%.1f delta_ext_temp=%.1f mod_cond=%s temp_cond=%s climate_cond=%s switch_cond=%s", + self, + delta_temp, + delta_ext_temp, + mode_cond, + temp_cond, + climate_cond, + switch_cond, + ) + ret = False if mode_cond and temp_cond and climate_cond: if not self._security_state: @@ -1748,17 +1779,6 @@ async def check_security(self) -> bool: ) ret = True - _LOGGER.debug( - "%s - checking security delta_temp=%.1f delta_ext_temp=%.1f mod_cond=%s temp_cond=%s climate_cond=%s switch_cond=%s", - self, - delta_temp, - delta_ext_temp, - mode_cond, - temp_cond, - climate_cond, - switch_cond, - ) - if mode_cond and temp_cond and switch_cond: if not self._security_state: _LOGGER.warning( diff --git a/custom_components/versatile_thermostat/tests/commons.py b/custom_components/versatile_thermostat/tests/commons.py index 1df6e73b..1e69edab 100644 --- a/custom_components/versatile_thermostat/tests/commons.py +++ b/custom_components/versatile_thermostat/tests/commons.py @@ -1,7 +1,7 @@ """ Some common resources """ from unittest.mock import patch -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, Event, EVENT_STATE_CHANGED, State from homeassistant.const import UnitOfTemperature from homeassistant.config_entries import ConfigEntryState @@ -9,7 +9,7 @@ from homeassistant.helpers.entity_component import EntityComponent from ..climate import VersatileThermostat -from ..const import DOMAIN +from ..const import DOMAIN, PRESET_SECURITY, PRESET_POWER, EventType from homeassistant.components.climate import ( ClimateEntity, @@ -32,6 +32,11 @@ MOCK_PRESENCE_CONFIG, MOCK_ADVANCED_CONFIG, # MOCK_DEFAULT_FEATURE_CONFIG, + PRESET_BOOST, + PRESET_COMFORT, + PRESET_NONE, + PRESET_ECO, + PRESET_ACTIVITY, ) FULL_SWITCH_CONFIG = ( @@ -93,3 +98,19 @@ def find_my_entity(entity_id) -> ClimateEntity: entity = find_my_entity(entity_id) return entity + + +async def send_temperature_change_event(entity: VersatileThermostat, new_temp, date): + """Sending a new temperature event simulating a change on temperature sensor""" + temp_event = Event( + EVENT_STATE_CHANGED, + { + "new_state": State( + entity_id=entity.entity_id, + state=new_temp, + last_changed=date, + last_updated=date, + ) + }, + ) + await entity._async_temperature_changed(temp_event) diff --git a/custom_components/versatile_thermostat/tests/const.py b/custom_components/versatile_thermostat/tests/const.py index 2e8e3f29..45cf9e8b 100644 --- a/custom_components/versatile_thermostat/tests/const.py +++ b/custom_components/versatile_thermostat/tests/const.py @@ -2,6 +2,8 @@ PRESET_BOOST, PRESET_COMFORT, PRESET_ECO, + PRESET_NONE, + PRESET_ACTIVITY, ) from custom_components.versatile_thermostat.const import ( CONF_NAME, diff --git a/custom_components/versatile_thermostat/tests/test_security.py b/custom_components/versatile_thermostat/tests/test_security.py new file mode 100644 index 00000000..e578ed4b --- /dev/null +++ b/custom_components/versatile_thermostat/tests/test_security.py @@ -0,0 +1,180 @@ +""" Test the Security featrure """ +from unittest.mock import patch, call + +from .commons import * # pylint: disable=wildcard-import, unused-wildcard-import + +from datetime import timedelta, datetime +import logging + +logging.getLogger().setLevel(logging.DEBUG) + + +async def test_security_feature(hass: HomeAssistant, skip_hass_states_is_state): + """Test the security feature and https://github.com/jmcollin78/versatile_thermostat/issues/49: + 1. creates a thermostat and check that security is off + 2. activate security feature when date is expired + 3. change the preset to boost + 4. check that security is still on + 5. resolve the date issue + 6. check that security is off and preset is changed to boost + """ + + entry = MockConfigEntry( + domain=DOMAIN, + title="TheOverSwitchMockName", + unique_id="uniqueId", + data={ + "name": "TheOverSwitchMockName", + "thermostat_type": "thermostat_over_switch", + "temperature_sensor_entity_id": "sensor.mock_temp_sensor", + "external_temperature_sensor_entity_id": "sensor.mock_ext_temp_sensor", + "cycle_min": 5, + "temp_min": 15, + "temp_max": 30, + "eco_temp": 17, + "comfort_temp": 18, + "boost_temp": 19, + "use_window_feature": False, + "use_motion_feature": False, + "use_power_feature": False, + "use_presence_feature": False, + "heater_entity_id": "switch.mock_switch", + "proportional_function": "tpi", + "tpi_coef_int": 0.3, + "tpi_coef_ext": 0.01, + "minimal_activation_delay": 30, + "security_delay_min": 5, # 5 minutes + "security_min_on_percent": 0.2, + "security_default_on_percent": 0.1, + }, + ) + + # 1. creates a thermostat and check that security is off + now: datetime = datetime.now() + entity: VersatileThermostat = await create_thermostat( + hass, entry, "climate.theoverswitchmockname" + ) + assert entity + + assert entity._security_state is False + assert entity.preset_mode is not PRESET_SECURITY + assert entity.preset_modes == [ + PRESET_NONE, + PRESET_ECO, + PRESET_COMFORT, + PRESET_BOOST, + ] + assert entity._last_ext_temperature_mesure is not None + assert entity._last_temperature_mesure is not None + assert (entity._last_temperature_mesure - now).total_seconds() < 1 + assert (entity._last_ext_temperature_mesure - now).total_seconds() < 1 + + # set a preset + assert entity.preset_mode is PRESET_NONE + await entity.async_set_preset_mode(PRESET_COMFORT) + assert entity.preset_mode is PRESET_COMFORT + + # Turn On the thermostat + assert entity.hvac_mode == HVACMode.OFF + await entity.async_set_hvac_mode(HVACMode.HEAT) + assert entity.hvac_mode == HVACMode.HEAT + + # 2. activate security feature when date is expired + with patch( + "custom_components.versatile_thermostat.climate.VersatileThermostat.send_event" + ) as mock_send_event, patch( + "custom_components.versatile_thermostat.climate.VersatileThermostat._async_heater_turn_on" + ) as mock_heater_on: + event_timestamp = now - timedelta(minutes=6) + + # set temperature to 15 so that on_percent will be > security_min_on_percent (0.2) + await send_temperature_change_event(entity, 15, event_timestamp) + assert entity._security_state is True + assert entity.preset_mode == PRESET_SECURITY + assert entity._saved_preset_mode == PRESET_COMFORT + assert entity._prop_algorithm.on_percent == 0.1 + assert entity._prop_algorithm.calculated_on_percent == 0.9 + + assert mock_send_event.call_count == 3 + mock_send_event.assert_has_calls( + [ + call.send_event(EventType.PRESET_EVENT, {"preset": PRESET_SECURITY}), + call.send_event( + EventType.TEMPERATURE_EVENT, + { + "last_temperature_mesure": event_timestamp.isoformat(), + "last_ext_temperature_mesure": entity._last_ext_temperature_mesure.isoformat(), + "current_temp": 15, + "current_ext_temp": None, + "target_temp": 18, + }, + ), + call.send_event( + EventType.SECURITY_EVENT, + { + "type": "start", + "last_temperature_mesure": event_timestamp.isoformat(), + "last_ext_temperature_mesure": entity._last_ext_temperature_mesure.isoformat(), + "current_temp": 15, + "current_ext_temp": None, + "target_temp": 18, + }, + ), + ], + any_order=True, + ) + + assert mock_heater_on.call_count == 1 + + # 3. Change the preset to Boost (we should stay in SECURITY) + with patch( + "custom_components.versatile_thermostat.climate.VersatileThermostat.send_event" + ) as mock_send_event, patch( + "custom_components.versatile_thermostat.climate.VersatileThermostat._async_heater_turn_on" + ) as mock_heater_on: + await entity.async_set_preset_mode(PRESET_BOOST) + + # 4. check that security is still on + assert entity._security_state is True + assert entity._prop_algorithm.on_percent == 0.1 + assert entity._prop_algorithm.calculated_on_percent == 0.9 + assert entity._saved_preset_mode == PRESET_BOOST + assert entity.preset_mode is PRESET_SECURITY + + # 5. resolve the datetime issue + with patch( + "custom_components.versatile_thermostat.climate.VersatileThermostat.send_event" + ) as mock_send_event, patch( + "custom_components.versatile_thermostat.climate.VersatileThermostat._async_heater_turn_on" + ) as mock_heater_on: + event_timestamp = datetime.now() + + # set temperature to 15 so that on_percent will be > security_min_on_percent (0.2) + await send_temperature_change_event(entity, 15.2, event_timestamp) + + assert entity._security_state is False + assert entity.preset_mode == PRESET_BOOST + assert entity._saved_preset_mode == PRESET_BOOST + assert entity._prop_algorithm.on_percent == 1.0 + assert entity._prop_algorithm.calculated_on_percent == 1.0 + + assert mock_send_event.call_count == 2 + mock_send_event.assert_has_calls( + [ + call.send_event(EventType.PRESET_EVENT, {"preset": PRESET_BOOST}), + call.send_event( + EventType.SECURITY_EVENT, + { + "type": "end", + "last_temperature_mesure": event_timestamp.isoformat(), + "last_ext_temperature_mesure": entity._last_ext_temperature_mesure.isoformat(), + "current_temp": 15.2, + "current_ext_temp": None, + "target_temp": 19, + }, + ), + ], + any_order=True, + ) + + assert mock_heater_on.call_count == 0 diff --git a/custom_components/versatile_thermostat/tests/test_start.py b/custom_components/versatile_thermostat/tests/test_start.py index 5eeaafec..c6ed17a8 100644 --- a/custom_components/versatile_thermostat/tests/test_start.py +++ b/custom_components/versatile_thermostat/tests/test_start.py @@ -1,6 +1,5 @@ """ Test the normal start of a Thermostat """ from unittest.mock import patch, call -import pytest from homeassistant.core import HomeAssistant from homeassistant.components.climate import HVACAction, HVACMode @@ -13,9 +12,7 @@ from ..climate import VersatileThermostat -from ..const import DOMAIN, EventType - -from .commons import MockClimate, FULL_SWITCH_CONFIG, PARTIAL_CLIMATE_CONFIG +from .commons import * async def test_over_switch_full_start(hass: HomeAssistant, skip_hass_states_is_state): @@ -51,7 +48,14 @@ def find_my_entity(entity_id) -> ClimateEntity: assert entity.hvac_action is HVACAction.OFF assert entity.hvac_mode is HVACMode.OFF assert entity.target_temperature == entity.min_temp - assert entity.preset_mode is None + assert entity.preset_modes == [ + PRESET_NONE, + PRESET_ECO, + PRESET_COMFORT, + PRESET_BOOST, + PRESET_ACTIVITY, + ] + assert entity.preset_mode is PRESET_NONE assert entity._security_state is False assert entity._window_state is None assert entity._motion_state is None @@ -61,16 +65,15 @@ def find_my_entity(entity_id) -> ClimateEntity: # should have been called with EventType.PRESET_EVENT and EventType.HVAC_MODE_EVENT assert mock_send_event.call_count == 2 - # Impossible to make this work, but it works... - # assert mock_send_event.assert_has_calls( - # [ - # call.send_event(EventType.PRESET_EVENT, {"preset": None}), - # call.send_event( - # EventType.HVAC_MODE_EVENT, - # {"hvac_mode": HVACMode.OFF}, - # ), - # ] - # ) + mock_send_event.assert_has_calls( + [ + call.send_event(EventType.PRESET_EVENT, {"preset": PRESET_NONE}), + call.send_event( + EventType.HVAC_MODE_EVENT, + {"hvac_mode": HVACMode.OFF}, + ), + ] + ) async def test_over_climate_full_start(hass: HomeAssistant, skip_hass_states_is_state): @@ -111,7 +114,13 @@ def find_my_entity(entity_id) -> ClimateEntity: assert entity.hvac_action is HVACAction.OFF assert entity.hvac_mode is HVACMode.OFF assert entity.target_temperature == entity.min_temp - assert entity.preset_mode is None + assert entity.preset_modes == [ + PRESET_NONE, + PRESET_ECO, + PRESET_COMFORT, + PRESET_BOOST, + ] + assert entity.preset_mode is PRESET_NONE assert entity._security_state is False assert entity._window_state is None assert entity._motion_state is None @@ -121,7 +130,7 @@ def find_my_entity(entity_id) -> ClimateEntity: assert mock_send_event.call_count == 2 mock_send_event.assert_has_calls( [ - call.send_event(EventType.PRESET_EVENT, {"preset": None}), + call.send_event(EventType.PRESET_EVENT, {"preset": PRESET_NONE}), call.send_event( EventType.HVAC_MODE_EVENT, {"hvac_mode": HVACMode.OFF},