Issue #121 - loop when underlying is slow

custom_components/versatile_thermostat/climate.py

@@ -220,6 +220,7 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
     _presence_state: bool
     _window_auto_state: bool
     _underlyings: list[UnderlyingEntity]
+    _last_change_time: datetime
 
     def __init__(self, hass: HomeAssistant, unique_id, name, entry_infos) -> None:
         """Initialize the thermostat."""
@@ -284,6 +285,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
 
         self._current_tz = dt_util.get_time_zone(self._hass.config.time_zone)
 
+        self._last_change_time = None
+
         self._underlyings = []
 
         self.post_init(entry_infos)
@@ -778,6 +781,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             else:
                 self.hass.create_task(self._async_control_heating())
 
+            self.reset_last_change_time()
+
         await self.get_my_previous_state()
 
         if self.hass.state == CoreState.running:
@@ -1233,6 +1238,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         # Ensure we update the current operation after changing the mode
         self.reset_last_temperature_time()
 
+        self.reset_last_change_time()
+
         self.update_custom_attributes()
         self.async_write_ha_state()
         self.send_event(EventType.HVAC_MODE_EVENT, {"hvac_mode": self._hvac_mode})
@@ -1288,6 +1295,11 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         self.recalculate()
         self.send_event(EventType.PRESET_EVENT, {"preset": self._attr_preset_mode})
 
+    def reset_last_change_time(self, old_preset_mode=None):
+        """Reset to now the last change time"""
+        self._last_change_time = datetime.now(tz=self._current_tz)
+        _LOGGER.warning("%s - last_change_time is now %s", self, self._last_change_time)
+
     def reset_last_temperature_time(self, old_preset_mode=None):
         """Reset to now the last temperature time if conditions are satisfied"""
         if (
@@ -1363,6 +1375,7 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         await self._async_internal_set_temperature(temperature)
         self._attr_preset_mode = PRESET_NONE
         self.recalculate()
+        self.reset_last_change_time()
         await self._async_control_heating(force=True)
 
     async def _async_internal_set_temperature(self, temperature):
@@ -1566,8 +1579,9 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
     async def _check_switch_initial_state(self):
         """Prevent the device from keep running if HVAC_MODE_OFF."""
         _LOGGER.debug("%s - Calling _check_switch_initial_state", self)
-        if self.is_over_climate:
+        # We need to do the same check for over_climate underlyings
-            return
+        #if self.is_over_climate:
+        #    return
         for under in self._underlyings:
             await under.check_initial_state(self._hvac_mode)
 
@@ -1605,6 +1619,11 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             else None
         )
 
+        old_state_date_changed = old_state.last_changed
+        old_state_date_updated = old_state.last_updated
+        new_state_date_changed = new_state.last_changed
+        new_state_date_updated = new_state.last_updated
+
         # Issue 99 - some AC turn hvac_mode=cool and hvac_action=idle when sending a HVACMode_OFF command
         # Issue 114 - Remove this because hvac_mode is now managed by local _hvac_mode and use idle action as is
         #if self._hvac_mode == HVACMode.OFF and new_hvac_action == HVACAction.IDLE:
@@ -1620,6 +1639,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             old_hvac_action,
         )
 
+        _LOGGER.warning("%s - last_change_time=%s old_state_date_changed=%s old_state_date_updated=%s new_state_date_changed=%s new_state_date_updated=%s", self, self._last_change_time, old_state_date_changed, old_state_date_updated, new_state_date_changed, new_state_date_updated)
+
         if new_hvac_mode in [
             HVACMode.OFF,
             HVACMode.HEAT,
@@ -1632,7 +1653,7 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         ] and self._hvac_mode != new_hvac_mode:
             changes = True
             self._hvac_mode = new_hvac_mode
-            # Do not try to update all underlying state, else we will have a loop
+            # Update all underlyings state
             if self._is_over_climate:
                 for under in self._underlyings:
                     await under.set_hvac_mode(new_hvac_mode)
@@ -1680,8 +1701,9 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             # try to manage new target temperature set if state
             _LOGGER.debug("Do temperature check. temperature is %s, new_state.attributes is %s", self.target_temperature, new_state.attributes)
             if self._is_over_climate and new_state.attributes and (new_target_temp := new_state.attributes.get("temperature")) and new_target_temp != self.target_temperature:
-                _LOGGER.info("%s - Target temp have change to %s", self, new_target_temp)
+                _LOGGER.warning("%s - Target temp have change to %s", self, new_target_temp)
-                await self.async_set_temperature(temperature = new_target_temp)
+                # TODO temporary removes the temperature changes for test
+                # await self.async_set_temperature(temperature = new_target_temp)
                 changes = True
 
         if changes:

custom_components/versatile_thermostat/tests/commons.py

@@ -94,7 +94,7 @@ class MockClimate(ClimateEntity):
         self._attr_extra_state_attributes = {}
         self._unique_id = unique_id
         self._name = name
-        self._attr_hvac_action = HVACAction.OFF
+        self._attr_hvac_action = HVACAction.OFF if hvac_mode == HVACMode.OFF else HVACAction.HEATING
         self._attr_hvac_mode = hvac_mode
         self._attr_hvac_modes = [HVACMode.OFF, HVACMode.COOL, HVACMode.HEAT]
         self._attr_temperature_unit = UnitOfTemperature.CELSIUS
@@ -106,6 +106,11 @@ class MockClimate(ClimateEntity):
         self._attr_target_temperature = temperature
         self.async_write_ha_state()
 
+    def async_set_hvac_mode(self, hvac_mode):
+        """ The hvac mode"""
+        self._attr_hvac_mode = hvac_mode
+        self.async_write_ha_state()
+
 class MockUnavailableClimate(ClimateEntity):
     """A Mock Climate class used for Underlying climate mode"""
 

custom_components/versatile_thermostat/tests/test_bugs.py

@@ -391,8 +391,8 @@ async def test_bug_82(
         assert entity.name == "TheOverClimateMockName"
         assert entity._is_over_climate is True
         # assert entity.hvac_action is HVACAction.OFF
-        # assert entity.hvac_mode is HVACMode.OFF
+        assert entity.hvac_mode is HVACMode.OFF
-        assert entity.hvac_mode is None
+        # assert entity.hvac_mode is None
         assert entity.target_temperature == entity.min_temp
         assert entity.preset_modes == [
             PRESET_NONE,
@@ -429,7 +429,7 @@ async def test_bug_82(
 
         # Tries to turns on the Thermostat
         await entity.async_set_hvac_mode(HVACMode.HEAT)
-        assert entity.hvac_mode == None
+        assert entity.hvac_mode == HVACMode.HEAT
 
         # 2. activate security feature when date is expired
         with patch(
@@ -466,6 +466,7 @@ async def test_bug_101(
         data=PARTIAL_CLIMATE_CONFIG, # 5 minutes security delay
     )
 
+    # Underlying is in HEAT mode but should be shutdown at startup
     fake_underlying_climate = MockClimate(hass, "mockUniqueId", "MockClimateName", {}, HVACMode.HEAT)
 
     with patch(
@@ -473,7 +474,9 @@ async def test_bug_101(
     ) as mock_send_event, patch(
         "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.find_underlying_climate",
         return_value=fake_underlying_climate,
-    ) as mock_find_climate:
+    ) as mock_find_climate, patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode:
         entry.add_to_hass(hass)
         await hass.config_entries.async_setup(entry.entry_id)
         assert entry.state is ConfigEntryState.LOADED
@@ -491,8 +494,17 @@ async def test_bug_101(
 
         assert entity.name == "TheOverClimateMockName"
         assert entity._is_over_climate is True
-        assert entity.hvac_action is HVACAction.OFF
+        assert entity.hvac_mode is HVACMode.OFF
-        assert entity.hvac_mode is HVACMode.HEAT
+        # because the underlying is heating. In real life the underlying should be shut-off
+        assert entity.hvac_action is HVACAction.HEATING
+        # Underlying should have been shutdown
+        assert mock_underlying_set_hvac_mode.call_count == 1
+        mock_underlying_set_hvac_mode.assert_has_calls(
+            [
+                call.set_hvac_mode(HVACMode.OFF),
+            ]
+        )
+
         assert entity.target_temperature == entity.min_temp
         assert entity.preset_mode is PRESET_NONE
 

custom_components/versatile_thermostat/tests/test_multiple_switch.py

@@ -257,11 +257,14 @@ async def test_multiple_switchs(
     )
     assert entity
     assert entity.is_over_climate is False
+    assert entity.nb_underlying_entities == 4
 
     # start heating, in boost mode. We block the control_heating to avoid running a cycle
     with patch(
         "custom_components.versatile_thermostat.climate.VersatileThermostat._async_control_heating"
-    ):
+    ), patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingSwitch.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode:
         await entity.async_set_hvac_mode(HVACMode.HEAT)
         await entity.async_set_preset_mode(PRESET_BOOST)
 
@@ -273,14 +276,16 @@ async def test_multiple_switchs(
         event_timestamp = now - timedelta(minutes=4)
         await send_temperature_change_event(entity, 15, event_timestamp)
 
-    # Checks that all heaters are off
+        # Checks that all climates are off
-    with patch(
-        "homeassistant.core.StateMachine.is_state", return_value=False
-    ) as mock_is_state:
         assert entity._is_device_active is False
 
         # Should be call for all Switch
-        assert mock_is_state.call_count == 4
+        assert mock_underlying_set_hvac_mode.call_count == 4
+        mock_underlying_set_hvac_mode.assert_has_calls(
+            [
+                call.set_hvac_mode(HVACMode.HEAT),
+            ]
+        )
 
     # Set temperature to a low level
     with patch(
@@ -339,3 +344,215 @@ async def test_multiple_switchs(
 
         # The first heater should be turned on but is already on but because call_later is mocked, it is only turned on here
         assert mock_heater_on.call_count == 1
+
+@pytest.mark.parametrize("expected_lingering_tasks", [True])
+@pytest.mark.parametrize("expected_lingering_timers", [True])
+async def test_multiple_climates(
+    hass: HomeAssistant,
+    skip_hass_states_is_state,
+    skip_send_event,
+):
+    """Test that when multiple climates are configured the activation and deactivation is propagated to all climates"""
+
+    tz = get_tz(hass)  # pylint: disable=invalid-name
+    now: datetime = datetime.now(tz=tz)
+
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="TheOver4ClimateMockName",
+        unique_id="uniqueId",
+        data={
+            CONF_NAME: "TheOver4ClimateMockName",
+            CONF_THERMOSTAT_TYPE: CONF_THERMOSTAT_CLIMATE,
+            CONF_TEMP_SENSOR: "sensor.mock_temp_sensor",
+            CONF_EXTERNAL_TEMP_SENSOR: "sensor.mock_ext_temp_sensor",
+            CONF_TEMP_MIN: 15,
+            CONF_TEMP_MAX: 30,
+            "eco_temp": 17,
+            "comfort_temp": 18,
+            "boost_temp": 19,
+            CONF_USE_WINDOW_FEATURE: False,
+            CONF_USE_MOTION_FEATURE: False,
+            CONF_USE_POWER_FEATURE: False,
+            CONF_USE_PRESENCE_FEATURE: False,
+            CONF_CLIMATE: "switch.mock_climate1",
+            CONF_CLIMATE_2: "switch.mock_climate2",
+            CONF_CLIMATE_3: "switch.mock_climate3",
+            CONF_CLIMATE_4: "switch.mock_climate4",
+            CONF_MINIMAL_ACTIVATION_DELAY: 30,
+            CONF_SECURITY_DELAY_MIN: 5,
+            CONF_SECURITY_MIN_ON_PERCENT: 0.3,
+        },
+    )
+
+    entity: VersatileThermostat = await create_thermostat(
+        hass, entry, "climate.theover4climatemockname"
+    )
+    assert entity
+    assert entity.is_over_climate is True
+    assert entity.nb_underlying_entities == 4
+
+    # start heating, in boost mode. We block the control_heating to avoid running a cycle
+    with patch(
+        "custom_components.versatile_thermostat.climate.VersatileThermostat._async_control_heating"
+    ), patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode:
+        await entity.async_set_hvac_mode(HVACMode.HEAT)
+        await entity.async_set_preset_mode(PRESET_BOOST)
+
+        assert entity.hvac_mode is HVACMode.HEAT
+        assert entity.preset_mode is PRESET_BOOST
+        assert entity.target_temperature == 19
+        assert entity.window_state is None
+
+        event_timestamp = now - timedelta(minutes=4)
+        await send_temperature_change_event(entity, 15, event_timestamp)
+
+        # Should be call for all Switch
+        assert mock_underlying_set_hvac_mode.call_count == 4
+        mock_underlying_set_hvac_mode.assert_has_calls(
+            [
+                call.set_hvac_mode(HVACMode.HEAT),
+            ]
+        )
+        assert entity._is_device_active is False
+
+    # Stop heating, in boost mode. We block the control_heating to avoid running a cycle
+    with patch(
+        "custom_components.versatile_thermostat.climate.VersatileThermostat._async_control_heating"
+    ), patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode:
+        await entity.async_set_hvac_mode(HVACMode.OFF)
+
+        assert entity.hvac_mode is HVACMode.OFF
+        assert entity.preset_mode is PRESET_BOOST
+        assert entity.target_temperature == 19
+        assert entity.window_state is None
+
+        event_timestamp = now - timedelta(minutes=4)
+        await send_temperature_change_event(entity, 15, event_timestamp)
+
+        # Should be call for all Switch
+        assert mock_underlying_set_hvac_mode.call_count == 4
+        mock_underlying_set_hvac_mode.assert_has_calls(
+            [
+                call.set_hvac_mode(HVACMode.OFF),
+            ]
+        )
+        assert entity._is_device_active is False
+
+@pytest.mark.parametrize("expected_lingering_tasks", [True])
+@pytest.mark.parametrize("expected_lingering_timers", [True])
+async def test_multiple_climates_underlying_changes(
+    hass: HomeAssistant,
+    skip_hass_states_is_state,
+    skip_send_event,
+):
+    """Test that when multiple switch are configured the activation of one underlying climate activate the others"""
+
+    tz = get_tz(hass)  # pylint: disable=invalid-name
+    now: datetime = datetime.now(tz=tz)
+
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="TheOver4ClimateMockName",
+        unique_id="uniqueId",
+        data={
+            CONF_NAME: "TheOver4ClimateMockName",
+            CONF_THERMOSTAT_TYPE: CONF_THERMOSTAT_CLIMATE,
+            CONF_TEMP_SENSOR: "sensor.mock_temp_sensor",
+            CONF_EXTERNAL_TEMP_SENSOR: "sensor.mock_ext_temp_sensor",
+            CONF_TEMP_MIN: 15,
+            CONF_TEMP_MAX: 30,
+            "eco_temp": 17,
+            "comfort_temp": 18,
+            "boost_temp": 19,
+            CONF_USE_WINDOW_FEATURE: False,
+            CONF_USE_MOTION_FEATURE: False,
+            CONF_USE_POWER_FEATURE: False,
+            CONF_USE_PRESENCE_FEATURE: False,
+            CONF_CLIMATE: "switch.mock_climate1",
+            CONF_CLIMATE_2: "switch.mock_climate2",
+            CONF_CLIMATE_3: "switch.mock_climate3",
+            CONF_CLIMATE_4: "switch.mock_climate4",
+            CONF_MINIMAL_ACTIVATION_DELAY: 30,
+            CONF_SECURITY_DELAY_MIN: 5,
+            CONF_SECURITY_MIN_ON_PERCENT: 0.3,
+        },
+    )
+
+    entity: VersatileThermostat = await create_thermostat(
+        hass, entry, "climate.theover4climatemockname"
+    )
+    assert entity
+    assert entity.is_over_climate is True
+    assert entity.nb_underlying_entities == 4
+
+    # start heating, in boost mode. We block the control_heating to avoid running a cycle
+    with patch(
+        "custom_components.versatile_thermostat.climate.VersatileThermostat._async_control_heating"
+    ), patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode:
+        await entity.async_set_hvac_mode(HVACMode.HEAT)
+        await entity.async_set_preset_mode(PRESET_BOOST)
+
+        assert entity.hvac_mode is HVACMode.HEAT
+        assert entity.preset_mode is PRESET_BOOST
+        assert entity.target_temperature == 19
+        assert entity.window_state is None
+
+        event_timestamp = now - timedelta(minutes=4)
+        await send_temperature_change_event(entity, 15, event_timestamp)
+
+        # Should be call for all Switch
+        assert mock_underlying_set_hvac_mode.call_count == 4
+        mock_underlying_set_hvac_mode.assert_has_calls(
+            [
+                call.set_hvac_mode(HVACMode.HEAT),
+            ]
+        )
+        assert entity._is_device_active is False
+
+    # Stop heating on one underlying climate
+    with patch(
+        "custom_components.versatile_thermostat.climate.VersatileThermostat._async_control_heating"
+    ), patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode:
+        await send_climate_change_event(entity, HVACMode.OFF, HVACMode.HEAT, HVACAction.OFF, HVACAction.HEATING, now)
+
+        # Should be call for all Switch
+        assert mock_underlying_set_hvac_mode.call_count == 4
+        mock_underlying_set_hvac_mode.assert_has_calls(
+            [
+                call.set_hvac_mode(HVACMode.OFF),
+            ]
+        )
+        assert entity.hvac_mode == HVACMode.OFF
+        assert entity._is_device_active is False
+
+    # Start heating on one underlying climate
+    with patch(
+        "custom_components.versatile_thermostat.climate.VersatileThermostat._async_control_heating"
+    ), patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode, patch(
+        # notice that there is no need of return_value=HVACAction.IDLE because this is not a function but a property
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.hvac_action", HVACAction.IDLE
+    ) as mock_underlying_get_hvac_action:
+        await send_climate_change_event(entity, HVACMode.HEAT, HVACMode.OFF, HVACAction.IDLE, HVACAction.OFF, now)
+
+        # Should be call for all Switch
+        assert mock_underlying_set_hvac_mode.call_count == 4
+        mock_underlying_set_hvac_mode.assert_has_calls(
+            [
+                call.set_hvac_mode(HVACMode.HEAT),
+            ]
+        )
+        assert entity.hvac_mode == HVACMode.HEAT
+        assert entity.hvac_action == HVACAction.IDLE
+        assert entity._is_device_active is False
+

custom_components/versatile_thermostat/tests/test_security.py

@@ -236,8 +236,10 @@ async def test_security_over_climate(
 
         assert entity.name == "TheOverClimateMockName"
         assert entity._is_over_climate is True
-        assert entity.hvac_action is HVACAction.OFF
+
-        assert entity.hvac_mode is HVACMode.HEAT
+        # Because the underlying is HEATING. In real life the underlying will be shut-off
+        assert entity.hvac_action is HVACAction.HEATING
+        assert entity.hvac_mode is HVACMode.OFF
         assert entity.target_temperature == entity.min_temp
         assert entity.preset_modes == [
             PRESET_NONE,
@@ -252,6 +254,7 @@ async def test_security_over_climate(
         assert mock_send_event.call_count == 2
         mock_send_event.assert_has_calls(
             [
+                # At startup
                 call.send_event(EventType.PRESET_EVENT, {"preset": PRESET_NONE}),
                 call.send_event(
                     EventType.HVAC_MODE_EVENT,
@@ -276,6 +279,17 @@ async def test_security_over_climate(
         await entity.async_set_hvac_mode(HVACMode.HEAT)
         assert entity.hvac_mode == HVACMode.HEAT
 
+        # One call more
+        assert mock_send_event.call_count == 3
+        mock_send_event.assert_has_calls(
+            [
+                call.send_event(
+                    EventType.HVAC_MODE_EVENT,
+                    {"hvac_mode": HVACMode.HEAT},
+                ),
+            ]
+        )
+
         # 2. activate security feature when date is expired
         with patch(
             "custom_components.versatile_thermostat.climate.VersatileThermostat.send_event"

custom_components/versatile_thermostat/underlyings.py

@@ -6,7 +6,7 @@ from homeassistant.const import ATTR_ENTITY_ID, STATE_ON, UnitOfTemperature
 
 from homeassistant.exceptions import ServiceNotFound
 
-from homeassistant.backports.enum import StrEnum
+from enum import StrEnum
 from homeassistant.core import HomeAssistant, DOMAIN as HA_DOMAIN, CALLBACK_TYPE
 from homeassistant.components.climate import (
     ClimateEntity,
@@ -131,6 +131,23 @@ class UnderlyingEntity:
         """Remove the underlying entity"""
         return
 
+    async def check_initial_state(self, hvac_mode: HVACMode):
+        """Prevent the underlying to be on but thermostat is off"""
+        if hvac_mode == HVACMode.OFF and self.is_device_active:
+            _LOGGER.warning(
+                "%s - The hvac mode is OFF, but the underlying device is ON. Turning off device %s",
+                self,
+                self._entity_id,
+            )
+            await self.set_hvac_mode(hvac_mode)
+        elif hvac_mode != HVACMode.OFF and self.is_device_active:
+            _LOGGER.warning(
+                "%s - The hvac mode is ON, but the underlying device is not ON. Turning on device %s",
+                self,
+                self._entity_id,
+            )
+            await self.set_hvac_mode(hvac_mode)
+
     # override to be able to mock the call
     def call_later(
         self, hass: HomeAssistant, delay_sec: int, called_method
@@ -193,16 +210,6 @@ class UnderlyingSwitch(UnderlyingEntity):
         """If the toggleable device is currently active."""
         return self._hass.states.is_state(self._entity_id, STATE_ON)
 
-    async def check_initial_state(self, hvac_mode: HVACMode):
-        """Prevent the heater to be on but thermostat is off"""
-        if hvac_mode == HVACMode.OFF and self.is_device_active:
-            _LOGGER.warning(
-                "%s - The hvac mode is OFF, but the switch device is ON. Turning off device %s",
-                self,
-                self._entity_id,
-            )
-            await self.turn_off()
-
     async def start_cycle(
         self,
         hvac_mode: HVACMode,