FIX A thermostat stays with security_default_on_percent when the preset change during security mode #49

custom_components/versatile_thermostat/climate.py

@@ -13,6 +13,8 @@ from homeassistant.core import (
     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 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             self._presets_away,
         )
         # Will be restored if possible
-        self._attr_preset_mode = None
+        self._attr_preset_mode = PRESET_NONE
-        self._saved_preset_mode = None
+        self._saved_preset_mode = PRESET_NONE
 
         # Power management
         self._device_power = entry_infos.get(CONF_DEVICE_POWER)
@@ -1076,6 +1080,16 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         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 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         _LOGGER.info("%s - Change entry with the values: %s", self, config_entry.data)
 
     @callback
-    async def _async_temperature_changed(self, event):
+    async def _async_temperature_changed(self, event: Event):
-        """Handle temperature changes."""
+        """Handle temperature of the temperature sensor changes."""
-        new_state = event.data.get("new_state")
+        new_state: State = event.data.get("new_state")
         _LOGGER.debug(
             "%s - Temperature changed. Event.new_state is %s",
             self,
@@ -1228,9 +1242,9 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         self.recalculate()
         await self._async_control_heating(force=False)
 
-    async def _async_ext_temperature_changed(self, event):
+    async def _async_ext_temperature_changed(self, event: Event):
-        """Handle external temperature changes."""
+        """Handle external temperature opf the sensor changes."""
-        new_state = event.data.get("new_state")
+        new_state: State = event.data.get("new_state")
         _LOGGER.debug(
             "%s - external Temperature changed. Event.new_state is %s",
             self,
@@ -1409,14 +1423,16 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         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 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             _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 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
     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 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             >= 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 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
                 )
             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(

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 pytest_homeassistant_custom_component.common import MockConfigEntry
 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 @@ from .const import (
     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 @@ async def create_thermostat(
         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)

custom_components/versatile_thermostat/tests/const.py

@@ -2,6 +2,8 @@ from homeassistant.components.climate.const import (
     PRESET_BOOST,
     PRESET_COMFORT,
     PRESET_ECO,
+    PRESET_NONE,
+    PRESET_ACTIVITY,
 )
 from custom_components.versatile_thermostat.const import (
     CONF_NAME,

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

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 pytest_homeassistant_custom_component.common import MockConfigEntry
 
 from ..climate import VersatileThermostat
 
-from ..const import DOMAIN, EventType
+from .commons import *
-
-from .commons import MockClimate, FULL_SWITCH_CONFIG, PARTIAL_CLIMATE_CONFIG
 
 
 async def test_over_switch_full_start(hass: HomeAssistant, skip_hass_states_is_state):
@@ -51,7 +48,14 @@ async def test_over_switch_full_start(hass: HomeAssistant, skip_hass_states_is_s
         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 @@ async def test_over_switch_full_start(hass: HomeAssistant, skip_hass_states_is_s
         # 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...
+        mock_send_event.assert_has_calls(
-        # assert mock_send_event.assert_has_calls(
+            [
-        #     [
+                call.send_event(EventType.PRESET_EVENT, {"preset": PRESET_NONE}),
-        #         call.send_event(EventType.PRESET_EVENT, {"preset": None}),
+                call.send_event(
-        #         call.send_event(
+                    EventType.HVAC_MODE_EVENT,
-        #             EventType.HVAC_MODE_EVENT,
+                    {"hvac_mode": HVACMode.OFF},
-        #             {"hvac_mode": HVACMode.OFF},
+                ),
-        #         ),
+            ]
-        #     ]
+        )
-        # )
 
 
 async def test_over_climate_full_start(hass: HomeAssistant, skip_hass_states_is_state):
@@ -111,7 +114,13 @@ async def test_over_climate_full_start(hass: HomeAssistant, skip_hass_states_is_
         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 @@ async def test_over_climate_full_start(hass: HomeAssistant, skip_hass_states_is_
         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},