Try to fix issue #334 - loop when underlying is late to update

custom_components/versatile_thermostat/base_thermostat.py

@@ -1406,9 +1406,10 @@ class BaseThermostat(ClimateEntity, RestoreEntity):
         if new_state is None or new_state.state in (STATE_UNAVAILABLE, STATE_UNKNOWN):
             return
 
-        await self._async_update_temp(new_state)
+        dearm_window_auto = await self._async_update_temp(new_state)
         self.recalculate()
         await self.async_control_heating(force=False)
+        return dearm_window_auto
 
     async def _async_ext_temperature_changed(self, event: Event):
         """Handle external temperature opf the sensor changes."""
@@ -1646,7 +1647,7 @@ class BaseThermostat(ClimateEntity, RestoreEntity):
                 await self.check_security()
 
             # check window_auto
-            await self._async_manage_window_auto()
+            return await self._async_manage_window_auto()
 
         except ValueError as ex:
             _LOGGER.error("Unable to update temperature from sensor: %s", ex)

custom_components/versatile_thermostat/thermostat_climate.py

@@ -601,8 +601,9 @@ class ThermostatOverClimate(BaseThermostat):
         #    new_hvac_mode = HVACMode.OFF
 
         _LOGGER.info(
-            "%s - Underlying climate changed. Event.new_hvac_mode is %s, current_hvac_mode=%s, new_hvac_action=%s, old_hvac_action=%s",
+            "%s - Underlying climate %s changed. Event.new_hvac_mode is %s, current_hvac_mode=%s, new_hvac_action=%s, old_hvac_action=%s",
             self,
+            new_state.entity_id,
             new_hvac_mode,
             self._hvac_mode,
             new_hvac_action,
@@ -658,7 +659,7 @@ class ThermostatOverClimate(BaseThermostat):
             )
             changes = True
 
-        # Issue #120 - Some TRV are chaning target temperature a very long time (6 sec) after the change.
+        # Issue #120 - Some TRV are changing target temperature a very long time (6 sec) after the change.
         # In that case a loop is possible if a user change multiple times during this 6 sec.
         if new_state_date_updated and self._last_change_time:
             delta = (new_state_date_updated - self._last_change_time).total_seconds()
@@ -684,12 +685,31 @@ class ThermostatOverClimate(BaseThermostat):
             ]
             and self._hvac_mode != new_hvac_mode
         ):
-            changes = True
-            self._hvac_mode = new_hvac_mode
             # Update all underlyings state
+            # Issue #334 - if all underlyings are not aligned with the same hvac_mode don't change the underlying and wait they are aligned
             if self.is_over_climate:
+                for under in self._underlyings:
+                    if (
+                        under.entity_id != new_state.entity_id
+                        and under.hvac_mode != self._hvac_mode
+                    ):
+                        _LOGGER.info(
+                            "%s - the underlying's hvac_mode %s is not aligned with VTherm hvac_mode %s. So we don't diffuse the change to all other underlyings to avoid loops",
+                            under,
+                            under.hvac_mode,
+                            self._hvac_mode,
+                        )
+                        return
+
+                _LOGGER.debug(
+                    "%s - All underlyings have the same hvac_mode, so VTherm will send the new hvac mode %s",
+                    self,
+                    new_hvac_mode,
+                )
                 for under in self._underlyings:
                     await under.set_hvac_mode(new_hvac_mode)
+            changes = True
+            self._hvac_mode = new_hvac_mode
 
         # A quick win to known if it has change by using the self._attr_fan_mode and not only underlying[0].fan_mode
         if new_fan_mode != self._attr_fan_mode:

custom_components/versatile_thermostat/underlyings.py

@@ -484,6 +484,14 @@ class UnderlyingClimate(UnderlyingEntity):
         if not self.is_initialized:
             return False
 
+        if self._underlying_climate.hvac_mode == hvac_mode:
+            _LOGGER.debug(
+                "%s - hvac_mode is already is requested state %s. Do not send any command",
+                self,
+                self._underlying_climate.hvac_mode,
+            )
+            return False
+
         data = {ATTR_ENTITY_ID: self._entity_id, "hvac_mode": hvac_mode}
         await self._hass.services.async_call(
             CLIMATE_DOMAIN,

tests/commons.py

@@ -428,10 +428,12 @@ async def send_temperature_change_event(
             )
         },
     )
-    await entity._async_temperature_changed(temp_event)
+    dearm_window_auto = await entity._async_temperature_changed(temp_event)
     if sleep:
         await asyncio.sleep(0.1)
 
+    return dearm_window_auto
+
 
 async def send_ext_temperature_change_event(
     entity: BaseThermostat, new_temp, date, sleep=True
@@ -619,6 +621,7 @@ async def send_climate_change_event(
     old_hvac_action: HVACAction,
     date,
     sleep=True,
+    underlying_entity_id: str = None,
 ):
     """Sending a new climate event simulating a change on the underlying climate state"""
     _LOGGER.info(
@@ -630,18 +633,23 @@ async def send_climate_change_event(
         date,
         entity,
     )
+
+    send_from_entity_id = (
+        underlying_entity_id if underlying_entity_id is not None else entity.entity_id
+    )
+
     climate_event = Event(
         EVENT_STATE_CHANGED,
         {
             "new_state": State(
-                entity_id=entity.entity_id,
+                entity_id=send_from_entity_id,
                 state=new_hvac_mode,
                 attributes={"hvac_action": new_hvac_action},
                 last_changed=date,
                 last_updated=date,
             ),
             "old_state": State(
-                entity_id=entity.entity_id,
+                entity_id=send_from_entity_id,
                 state=old_hvac_mode,
                 attributes={"hvac_action": old_hvac_action},
                 last_changed=date,

tests/test_bugs.py

@@ -633,14 +633,16 @@ async def test_bug_272(
 
         # In the accepted interval
         await entity.async_set_temperature(temperature=17.5)
-        assert mock_service_call.call_count == 2
+
+        # MagicMock climate is already HEAT by default. So there is no SET_HAVC_MODE call
+        assert mock_service_call.call_count == 1
         mock_service_call.assert_has_calls(
             [
-                call.async_call(
+                # call.async_call(
-                    "climate",
+                #     "climate",
-                    SERVICE_SET_HVAC_MODE,
+                #     SERVICE_SET_HVAC_MODE,
-                    {"entity_id": "climate.mock_climate", "hvac_mode": HVACMode.HEAT},
+                #     {"entity_id": "climate.mock_climate", "hvac_mode": HVACMode.HEAT},
-                ),
+                # ),
                 call.async_call(
                     "climate",
                     SERVICE_SET_TEMPERATURE,

tests/test_central_config.py

@@ -268,7 +268,7 @@ async def test_full_over_switch_wo_central_config(
         assert entity._security_default_on_percent == 0.1
         assert entity.is_inversed is False
 
-        assert entity.is_window_auto_enabled is True
+        assert entity.is_window_auto_enabled is False  # we have an entity_id
         assert entity._window_sensor_entity_id == "binary_sensor.mock_window_sensor"
         assert entity._window_delay_sec == 30
         assert entity._window_auto_close_threshold == 0.1
@@ -377,7 +377,8 @@ async def test_full_over_switch_with_central_config(
         assert entity._security_default_on_percent == 0.2
         assert entity.is_inversed is False
 
-        assert entity.is_window_auto_enabled is True
+        # We have an entity so window auto is not enabled
+        assert entity.is_window_auto_enabled is False
         assert entity._window_sensor_entity_id == "binary_sensor.mock_window_sensor"
         assert entity._window_delay_sec == 15
         assert entity._window_auto_close_threshold == 1

tests/test_multiple_switch.py

@@ -472,7 +472,7 @@ async def test_multiple_climates_underlying_changes(
     skip_hass_states_is_state,
     skip_send_event,
 ):  # pylint: disable=unused-argument
-    """Test that when multiple switch are configured the activation of one underlying
+    """Test that when multiple climate are configured the activation of one underlying
     climate activate the others"""
 
     tz = get_tz(hass)  # pylint: disable=invalid-name
@@ -541,11 +541,15 @@ async def test_multiple_climates_underlying_changes(
         assert entity.is_device_active is False  # pylint: disable=protected-access
 
     # Stop heating on one underlying climate
+    # All underlying supposed to be aligned with the hvac_mode now
     with patch(
         "custom_components.versatile_thermostat.base_thermostat.BaseThermostat.async_control_heating"
     ), patch(
         "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
-    ) as mock_underlying_set_hvac_mode:
+    ) as mock_underlying_set_hvac_mode, patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.hvac_mode",
+        HVACMode.HEAT,
+    ):
         # Wait 11 sec so that the event will not be discarded
         event_timestamp = now + timedelta(seconds=11)
         await send_climate_change_event(
@@ -555,6 +559,7 @@ async def test_multiple_climates_underlying_changes(
             HVACAction.OFF,
             HVACAction.HEATING,
             event_timestamp,
+            underlying_entity_id="switch.mock_climate3",
         )
 
         # Should be call for all Switch
@@ -577,6 +582,9 @@ async def test_multiple_climates_underlying_changes(
         # a function but a property
         "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.hvac_action",
         HVACAction.IDLE,
+    ), patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.hvac_mode",
+        HVACMode.OFF,
     ):
         # Wait 11 sec so that the event will not be discarded
         event_timestamp = now + timedelta(seconds=11)
@@ -601,6 +609,113 @@ async def test_multiple_climates_underlying_changes(
         assert entity.is_device_active is False  # pylint: disable=protected-access
 
 
+@pytest.mark.parametrize("expected_lingering_tasks", [True])
+@pytest.mark.parametrize("expected_lingering_timers", [True])
+async def test_multiple_climates_underlying_changes_not_aligned(
+    hass: HomeAssistant,
+    skip_hass_states_is_state,
+    skip_send_event,
+):  # pylint: disable=unused-argument
+    """Test that when multiple climate are configured the activation of one underlying
+    climate don't activate the others if their havc_mode are not aligned"""
+
+    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_CYCLE_MIN: 8,
+            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: BaseThermostat = 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.base_thermostat.BaseThermostat.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 STATE_OFF
+
+        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),
+            ]
+        )
+
+    # Stop heating on one underlying climate
+    # All underlying supposed to be aligned with the hvac_mode now
+    with patch(
+        "custom_components.versatile_thermostat.base_thermostat.BaseThermostat.async_control_heating"
+    ), patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode, patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.hvac_mode",
+        HVACMode.COOL,
+    ):
+        # Wait 11 sec so that the event will not be discarded
+        event_timestamp = now + timedelta(seconds=11)
+        await send_climate_change_event(
+            entity,
+            HVACMode.OFF,
+            HVACMode.HEAT,
+            HVACAction.OFF,
+            HVACAction.HEATING,
+            event_timestamp,
+            underlying_entity_id="switch.mock_climate3",
+        )
+
+        # Should be call for all Switch
+        assert mock_underlying_set_hvac_mode.call_count == 0
+        # mock_underlying_set_hvac_mode.assert_has_calls(
+        #     [
+        #         call.set_hvac_mode(HVACMode.OFF),
+        #     ]
+        # )
+        # No change
+        assert entity.hvac_mode == HVACMode.HEAT
+
+
 @pytest.mark.parametrize("expected_lingering_tasks", [True])
 @pytest.mark.parametrize("expected_lingering_timers", [True])
 async def test_multiple_switch_power_management(

tests/test_window.py

@@ -581,7 +581,7 @@ async def test_window_auto_auto_stop(hass: HomeAssistant, skip_hass_states_is_st
             CONF_SECURITY_MIN_ON_PERCENT: 0.3,
             CONF_WINDOW_AUTO_OPEN_THRESHOLD: 6,
             CONF_WINDOW_AUTO_CLOSE_THRESHOLD: 6,
-            CONF_WINDOW_AUTO_MAX_DURATION: 0,  # 0 will deactivate window auto detection
+            CONF_WINDOW_AUTO_MAX_DURATION: 1,  # 0 will deactivate window auto detection
         },
     )
 
@@ -604,9 +604,9 @@ async def test_window_auto_auto_stop(hass: HomeAssistant, skip_hass_states_is_st
     assert entity.target_temperature == 21
 
     assert entity.window_state is STATE_OFF
-    assert entity.is_window_auto_enabled is False
+    assert entity.is_window_auto_enabled is True
 
-    # Initialize the slope algo with 2 measurements
+    # 1. Initialize the slope algo with 2 measurements
     event_timestamp = now + timedelta(minutes=1)
     await send_temperature_change_event(entity, 19, event_timestamp)
     event_timestamp = event_timestamp + timedelta(minutes=1)
@@ -614,7 +614,7 @@ async def test_window_auto_auto_stop(hass: HomeAssistant, skip_hass_states_is_st
     event_timestamp = event_timestamp + timedelta(minutes=1)
     await send_temperature_change_event(entity, 19, event_timestamp)
 
-    # Make the temperature down
+    # 2. Make the temperature down
     with patch(
         "custom_components.versatile_thermostat.base_thermostat.BaseThermostat.send_event"
     ) as mock_send_event, patch(
@@ -634,7 +634,7 @@ async def test_window_auto_auto_stop(hass: HomeAssistant, skip_hass_states_is_st
         assert entity._window_auto_algo.is_window_close_detected() is False
         assert entity.hvac_mode is HVACMode.HEAT
 
-    # send one degre down in one minute
+    # 3. send one degre down in one minute
     with patch(
         "custom_components.versatile_thermostat.base_thermostat.BaseThermostat.send_event"
     ) as mock_send_event, patch(
@@ -670,12 +670,14 @@ async def test_window_auto_auto_stop(hass: HomeAssistant, skip_hass_states_is_st
         assert entity.window_auto_state == STATE_ON
         assert entity.hvac_mode is HVACMode.OFF
 
-    # This is to avoid that the slope stayx under 6, else we will reactivate the window immediatly
+    # 4. This is to avoid that the slope stay under 6, else we will reactivate the window immediatly
     event_timestamp = event_timestamp + timedelta(minutes=1)
-    await send_temperature_change_event(entity, 19, event_timestamp, sleep=False)
+    dearm_window_auto = await send_temperature_change_event(
+        entity, 19, event_timestamp, sleep=False
+    )
     assert entity.last_temperature_slope > -6.0
 
-    # Waits for automatic disable
+    # 5. Waits for automatic disable
     with patch(
         "custom_components.versatile_thermostat.base_thermostat.BaseThermostat.send_event"
     ) as mock_send_event, patch(
@@ -684,7 +686,8 @@ async def test_window_auto_auto_stop(hass: HomeAssistant, skip_hass_states_is_st
         "custom_components.versatile_thermostat.underlyings.UnderlyingSwitch.is_device_active",
         return_value=False,
     ):
-        await asyncio.sleep(0.3)
+        # simulate the expiration of the delay
+        await dearm_window_auto(None)
 
         assert entity.hvac_mode is HVACMode.HEAT
         assert entity.preset_mode is PRESET_BOOST