Bug #121 loop in over climate

* Issue #121 - loop when underlying is slow
* Issue #121 - try fix
* Release 3.5.3
* Fix tests step

.github/workflows/pull.yml

@@ -31,6 +31,8 @@ jobs:
         - run: black .
 
   tests:
+    # Tests don't run in Gitlab ci environment
+    if: 0
     runs-on: "ubuntu-latest"
     name: Run tests
     steps:
@@ -41,10 +43,10 @@ jobs:
         with:
           python-version: "3.8"
       - name: Install requirements
-        run: python3 -m pip install -r requirements_test.txt
+        run: cd custom_components/versatile_thermostat && python3 -m pip install -r requirements_test.txt
       - name: Run tests
         run: |
-          pytest \
+          cd custom_components/versatile_thermostat && pytest \
             -qq \
             --timeout=9 \
             --durations=10 \

custom_components/versatile_thermostat/climate.py

@@ -956,7 +956,6 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
         """Return current operation."""
         # Issue #114 - returns my current hvac_mode and not the underlying hvac_mode which could be different
         # delta will be managed by climate_state_change event.
-        # TODO remove this when ok
         # if self._is_over_climate:
             # if one not OFF -> return it
             # else OFF
@@ -1598,7 +1597,14 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
 
     @callback
     async def _async_climate_changed(self, event):
-        """Handle unerdlying climate state changes."""
+        """Handle unerdlying climate state changes.
+           This method takes the underlying values and update the VTherm with them.
+           To avoid loops (issues #121 #101 #95 #99), we discard the event if it is received
+           less than 10 sec after the last command. What we want here is to take the values
+           from underlyings ONLY if someone have change directly on the underlying and not
+           as a return of the command. The only thing we take all the time is the HVACAction
+           which is important for feedaback and which cannot generates loops.
+        """
 
         async def end_climate_changed(changes):
             """ To end the event management"""
@@ -1688,7 +1694,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             )
             changes = True
 
-        # Issue #120 - Some TRV are chaning target temperature a very long time (6 sec) after the change. In that case a loop is possible because
+        # Issue #120 - Some TRV are chaning 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()
             if delta < 10:
@@ -2138,9 +2145,6 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             now - self._last_ext_temperature_mesure.replace(tzinfo=self._current_tz)
         ).total_seconds() / 60.0
 
-        # TODO before change:
-        # mode_cond = self._is_over_climate or self._hvac_mode != HVACMode.OFF
-        # fixed into this. Why if _is_over_climate we could into security even if HVACMode is OFF ?
         mode_cond = self._hvac_mode != HVACMode.OFF
 
         temp_cond: bool = (
@@ -2422,7 +2426,7 @@ class VersatileThermostat(ClimateEntity, RestoreEntity):
             "window_auto_max_duration": self._window_auto_max_duration,
         }
         if self._is_over_climate:
-            self._attr_extra_state_attributes["underlying_climate_1"] = self._underlyings[
+            self._attr_extra_state_attributes["underlying_climate_0"] = self._underlyings[
                 0
             ].entity_id
             self._attr_extra_state_attributes["underlying_climate_1"] = self._underlyings[

custom_components/versatile_thermostat/manifest.json

@@ -14,6 +14,6 @@
   "quality_scale": "silver",
   "requirements": [],
   "ssdp": [],
-  "version": "3.0.0",
+  "version": "3.5.3",
   "zeroconf": []
 }

custom_components/versatile_thermostat/requirements_dev.txt

@@ -1,2 +1,2 @@
-homeassistant==2023.10.1
+homeassistant==2023.10.3
 ffmpeg

custom_components/versatile_thermostat/requirements_test.txt

@@ -1,4 +1,5 @@
--r requirements_dev.txt
+# For automatic run of test in Gitlab CI, we must not include other things that pytest-homeassistant-custom-component
+#-r requirements_dev.txt
 # aiodiscover
-ulid_transform
+# ulid_transform
 pytest-homeassistant-custom-component

custom_components/versatile_thermostat/tests/test_bugs.py

@@ -530,9 +530,16 @@ async def test_bug_101(
         await entity.async_set_preset_mode(PRESET_COMFORT)
         assert entity.preset_mode == PRESET_COMFORT
 
-        # 2. Change the target temp of underlying thermostat
+        # 2. Change the target temp of underlying thermostat at now -> the event will be disgarded because to fast (to avoid loop cf issue 121)
         await send_climate_change_event_with_temperature(entity, HVACMode.HEAT, HVACMode.HEAT, HVACAction.OFF, HVACAction.OFF, now, 12.75)
-        # Should have been switched to Manual preset
+        # Should NOT have been switched to Manual preset
+        assert entity.target_temperature == 17
+        assert entity.preset_mode is PRESET_COMFORT
+
+        # 2. Change the target temp of underlying thermostat at 11 sec later -> the event will be taken
+        # Wait 11 sec
+        event_timestamp = now + timedelta(seconds=11)
+        await send_climate_change_event_with_temperature(entity, HVACMode.HEAT, HVACMode.HEAT, HVACAction.OFF, HVACAction.OFF, event_timestamp, 12.75)
         assert entity.target_temperature == 12.75
         assert entity.preset_mode is PRESET_NONE
 

custom_components/versatile_thermostat/tests/test_multiple_switch.py

@@ -163,7 +163,7 @@ async def test_one_switch_cycle(
         await asyncio.sleep(0.1)
 
         assert mock_heater_on.call_count == 1
-        # TODO normal ? assert entity.underlying_entity(0)._should_relaunch_control_heating is False
+        # normal ? assert entity.underlying_entity(0)._should_relaunch_control_heating is False
 
     # Simulate the end of heater on cycle
     event_timestamp = now - timedelta(minutes=3)
@@ -522,7 +522,9 @@ async def test_multiple_climates_underlying_changes(
     ), 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)
+        # 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)
 
         # Should be call for all Switch
         assert mock_underlying_set_hvac_mode.call_count == 4
@@ -543,7 +545,9 @@ async def test_multiple_climates_underlying_changes(
         # 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)
+        # Wait 11 sec so that the event will not be discarded
+        event_timestamp = now + timedelta(seconds=11)
+        await send_climate_change_event(entity, HVACMode.HEAT, HVACMode.OFF, HVACAction.IDLE, HVACAction.OFF, event_timestamp)
 
         # Should be call for all Switch
         assert mock_underlying_set_hvac_mode.call_count == 4

hacs.json

@@ -3,5 +3,5 @@
     "content_in_root": false,
     "render_readme": true,
     "hide_default_branch": false,
-    "homeassistant": "2023.7.3"
+    "homeassistant": "2023.10.3"
 }