Change reset cumulated_error formula

README-fr.md

@@ -128,7 +128,7 @@ En conséquence toute la phase de paramètrage d'un VTherm a été profondemment
 **Note :** les copies d'écran de la configuration d'un VTherm n'ont pas été mises à jour.
 
 # Merci pour la bière [buymecoffee](https://www.buymeacoffee.com/jmcollin78)
-Un grand merci à @salabur, @pvince83, @bergoglio, @EPicLURcher, @ecolorado66, @Kriss1670, @maia, @f.maymil, @moutte69, @Jerome, @Gunnar M, @Greg.o, @John Burgess, @abyssmal, @capinfo26, @Helge, @MattG @Mexx62, @Someone, @Lajull, @giopeco pour les bières. Ca fait très plaisir et ça m'encourage à continuer !
+Un grand merci à @salabur, @pvince83, @bergoglio, @EPicLURcher, @ecolorado66, @Kriss1670, @maia, @f.maymil, @moutte69, @Jerome, @Gunnar M, @Greg.o, @John Burgess, @abyssmal, @capinfo26, @Helge, @MattG @Mexx62, @Someone, @Lajull, @giopeco, @fredericselier, @philpagan pour les bières. Ca fait très plaisir et ça m'encourage à continuer !
 
 
 # Quand l'utiliser et ne pas l'utiliser

README.md

@@ -128,7 +128,7 @@ Consequently, the entire configuration phase of a VTherm has been profoundly mod
 **Note:** the VTherm configuration screenshots have not been updated.
 
 # Thanks for the beer [buymecoffee](https://www.buymeacoffee.com/jmcollin78)
-Many thanks to @salabur, @pvince83, @bergoglio, @EPicLURcher, @ecolorado66, @Kriss1670, @maia, @f.maymil, @moutte69, @Jerome, @Gunnar M, @Greg.o, @John Burgess, @abyssmal, @capinfo26, @Helge, @MattG, @MattG, @Mexx62, @Someone, @Lajull, @giopeco for the beers. It's very nice and encourages me to continue!
+Many thanks to @salabur, @pvince83, @bergoglio, @EPicLURcher, @ecolorado66, @Kriss1670, @maia, @f.maymil, @moutte69, @Jerome, @Gunnar M, @Greg.o, @John Burgess, @abyssmal, @capinfo26, @Helge, @MattG, @MattG, @Mexx62, @Someone, @Lajull, @giopeco, @fredericselier, @philpagan for the beers. It's very nice and encourages me to continue!
 
 # When to use / not use
 This thermostat can control 3 types of equipment:

copy-to-forum.txt

@@ -0,0 +1,7 @@
+Before copying to forum you need to replace relative images by this command into VSCode:
+
+Search :
+\(images/(.*).png\)
+
+Replace with:
+(https://github.com/jmcollin78/versatile_thermostat/blob/main/images/$1.png?raw=true)

custom_components/versatile_thermostat/base_thermostat.py

@@ -1233,9 +1233,9 @@ class BaseThermostat(ClimateEntity, RestoreEntity):
             )
 
         # If AC is on maybe we have to change the temperature in force mode, but not in frost mode (there is no Frost protection possible in AC mode)
-        if self._hvac_mode == HVACMode.COOL:
+        if self._hvac_mode == HVACMode.COOL and self.preset_mode != PRESET_NONE:
             if self.preset_mode != PRESET_FROST_PROTECTION:
-                await self._async_set_preset_mode_internal(self._attr_preset_mode, True)
+                await self._async_set_preset_mode_internal(self.preset_mode, True)
             else:
                 await self._async_set_preset_mode_internal(PRESET_ECO, True, False)
 

custom_components/versatile_thermostat/pi_algorithm.py

@@ -47,10 +47,10 @@ class PITemperatureRegulator:
     def set_target_temp(self, target_temp):
         """Set the new target_temp"""
         self.target_temp = target_temp
-        # Do not reset the accumulated error
         # Discussion #191. After a target change we should reset the accumulated error which is certainly wrong now.
-        if self.accumulated_error < 0:
+        # Discussion #384. Finally don't reset the accumulated error but smoothly reset it if the sign is inversed
-            self.accumulated_error = 0
+        # if self.accumulated_error < 0:
+        #     self.accumulated_error = 0
 
     def calculate_regulated_temperature(
         self, room_temp: float, external_temp: float
@@ -71,6 +71,11 @@ class PITemperatureRegulator:
         error = self.target_temp - room_temp
 
         # Calculate the sum of error (I)
+        # Discussion #384. Finally don't reset the accumulated error but smoothly reset it if the sign is inversed
+        # If the error have change its sign, reset smoothly the accumulated error
+        if error * self.accumulated_error < 0:
+            self.accumulated_error = self.accumulated_error / 2.0
+
         self.accumulated_error += error
 
         # Capping of the error
@@ -91,8 +96,7 @@ class PITemperatureRegulator:
 
         result = round(self.target_temp + total_offset, 1)
 
-        # TODO Change to debug after experimental
+        _LOGGER.debug(
-        _LOGGER.info(
             "PITemperatureRegulator - Error: %.2f accumulated_error: %.2f offset: %.2f offset_ext: %.2f target_tem: %.1f regulatedTemp: %.1f",
             error,
             self.accumulated_error,

custom_components/versatile_thermostat/thermostat_climate.py

@@ -194,8 +194,42 @@ class ThermostatOverClimate(BaseThermostat):
 
         self._last_regulation_change = now
         for under in self._underlyings:
+            # issue 348 - use device temperature if configured as offset
+            offset_temp = 0
+            device_temp = 0
+            if (
+                # regulation can use the device_temp
+                self.auto_regulation_use_device_temp
+                # and we have access to the device temp
+                and (device_temp := under.underlying_current_temperature) is not None
+                # and target is not reach (ie we need regulation)
+                and (
+                    (
+                        self.hvac_mode == HVACMode.COOL
+                        and self.target_temperature < self.current_temperature
+                    )
+                    or (
+                        self.hvac_mode == HVACMode.HEAT
+                        and self.target_temperature > self.current_temperature
+                    )
+                )
+            ):
+                offset_temp = device_temp - self.current_temperature
+
+            target_temp = self.regulated_target_temp + offset_temp
+
+            _LOGGER.debug(
+                "%s - The device offset temp for regulation is %.2f - internal temp is %.2f. New target is %.2f",
+                self,
+                offset_temp,
+                device_temp,
+                target_temp,
+            )
+
             await under.set_temperature(
-                self.regulated_target_temp, self._attr_max_temp, self._attr_min_temp
+                target_temp,
+                self._attr_max_temp,
+                self._attr_min_temp,
             )
 
     async def _send_auto_fan_mode(self):

custom_components/versatile_thermostat/underlyings.py

@@ -568,23 +568,9 @@ class UnderlyingClimate(UnderlyingEntity):
         if not self.is_initialized:
             return
 
-        # issue 348 - use device temperature if configured as offset
-        offset_temp = 0
-        if self._thermostat.auto_regulation_use_device_temp and hasattr(
-            self._underlying_climate, "current_temperature"
-        ):
-            device_temp = self._underlying_climate.current_temperature
-            offset_temp = device_temp - self._thermostat.current_temperature
-            _LOGGER.debug(
-                "%s - the device offset temp for regulation is %.2f - internal temp is %.2f",
-                self,
-                offset_temp,
-                device_temp,
-            )
-
         data = {
             ATTR_ENTITY_ID: self._entity_id,
-            "temperature": self.cap_sent_value(temperature + offset_temp),
+            "temperature": self.cap_sent_value(temperature),
             "target_temp_high": max_temp,
             "target_temp_low": min_temp,
         }
@@ -686,6 +672,18 @@ class UnderlyingClimate(UnderlyingEntity):
             return False
         return self._underlying_climate.is_aux_heat
 
+    @property
+    def underlying_current_temperature(self) -> float | None:
+        """Get the underlying current_temperature if it exists
+        and if initialized"""
+        if not self.is_initialized:
+            return None
+
+        if not hasattr(self._underlying_climate, "current_temperature"):
+            return None
+
+        return self._underlying_climate.current_temperature
+
     def turn_aux_heat_on(self) -> None:
         """Turn auxiliary heater on."""
         if not self.is_initialized:

tests/test_auto_regulation.py

@@ -127,9 +127,7 @@ async def test_over_climate_regulation(
 
             # the regulated temperature should be under
             assert entity.regulated_target_temp < entity.target_temperature
-            assert (
+            assert entity.regulated_target_temp == 18 - 2.5
-                entity.regulated_target_temp == 18 - 2
-            )  # normally 0.6 but round_to_nearest gives 0.5
 
 
 @pytest.mark.parametrize("expected_lingering_tasks", [True])
@@ -421,18 +419,6 @@ async def test_over_climate_regulation_use_device_temp(
         assert entity.is_regulated is True
         assert entity.auto_regulation_use_device_temp is True
 
-        assert entity.hvac_mode is HVACMode.OFF
-        assert entity.hvac_action is HVACAction.OFF
-        assert entity.target_temperature == entity.min_temp
-        assert entity.preset_modes == [
-            PRESET_NONE,
-            PRESET_FROST_PROTECTION,
-            PRESET_ECO,
-            PRESET_COMFORT,
-            PRESET_BOOST,
-        ]
-        assert entity.preset_mode is PRESET_NONE
-
         # 1.  Activate the heating by changing HVACMode and temperature
         # Select a hvacmode, presence and preset
         await entity.async_set_hvac_mode(HVACMode.HEAT)
@@ -442,7 +428,11 @@ async def test_over_climate_regulation_use_device_temp(
         await send_temperature_change_event(entity, 18, event_timestamp)
         await send_ext_temperature_change_event(entity, 10, event_timestamp)
 
-        # 2. set manual target temp (at now - 7) -> the regulation should occurs
+        # 2. set manual target temp (at now - 7) -> no regulation should occurs
+        # room temp is 18
+        # target is 16
+        # internal heater temp is 15
+        fake_underlying_climate.set_current_temperature(15)
         event_timestamp = now - timedelta(minutes=7)
         with patch(
             "custom_components.versatile_thermostat.commons.NowClass.get_now",
@@ -456,7 +446,7 @@ async def test_over_climate_regulation_use_device_temp(
             assert entity.hvac_action == HVACAction.HEATING
             assert entity.preset_mode == PRESET_NONE  # Manual mode
 
-            # the regulated temperature should be lower
+            # the regulated temperature should be higher
             assert entity.regulated_target_temp < entity.target_temperature
             # The calcul is the following: 16 + (16 - 18) x 0.4 (strong) + 0 x ki - 1 (device offset)
             assert (
@@ -471,7 +461,8 @@ async def test_over_climate_regulation_use_device_temp(
                         "set_temperature",
                         {
                             "entity_id": "climate.mock_climate",
-                            "temperature": 12.0,  # because device offset is -3 (15 - 18)
+                            # because device offset is -3 but not used because target is reach
+                            "temperature": 15.0,
                             "target_temp_high": 30,
                             "target_temp_low": 15,
                         },
@@ -480,19 +471,24 @@ async def test_over_climate_regulation_use_device_temp(
             )
 
         # 3. change temperature so that the regulated temperature should slow down
-        fake_underlying_climate.set_current_temperature(27)
+        # HVACMODE.HEAT
+        # room temp is 15
+        # target is 18
+        # internal heater temp is 20
+        fake_underlying_climate.set_current_temperature(20)
+        await entity.async_set_temperature(temperature=18)
+        await send_ext_temperature_change_event(entity, 9, event_timestamp)
 
         event_timestamp = now - timedelta(minutes=5)
         with patch(
             "custom_components.versatile_thermostat.commons.NowClass.get_now",
             return_value=event_timestamp,
         ), patch("homeassistant.core.ServiceRegistry.async_call") as mock_service_call:
-            await send_temperature_change_event(entity, 23, event_timestamp)
+            await send_temperature_change_event(entity, 15, event_timestamp)
-            await send_ext_temperature_change_event(entity, 19, event_timestamp)
 
-            # the regulated temperature should be under (device offset is -3)
+            # the regulated temperature should be under (device offset is -2)
-            assert entity.regulated_target_temp < entity.target_temperature
+            assert entity.regulated_target_temp > entity.target_temperature
-            assert entity.regulated_target_temp == 12.5
+            assert entity.regulated_target_temp == 19.4  # 18 + 1.4
 
             mock_service_call.assert_has_calls(
                 [
@@ -501,7 +497,42 @@ async def test_over_climate_regulation_use_device_temp(
                         "set_temperature",
                         {
                             "entity_id": "climate.mock_climate",
-                            "temperature": 16.5,  # because device offset is +4 (27 - 23)
+                            "temperature": 24.4,  # 19.4 + 5
+                            "target_temp_high": 30,
+                            "target_temp_low": 15,
+                        },
+                    ),
+                ]
+            )
+
+        # 4. In cool mode
+        # room temp is 25
+        # target is 23
+        # internal heater temp is 27
+        await entity.async_set_hvac_mode(HVACMode.COOL)
+        await entity.async_set_temperature(temperature=23)
+        fake_underlying_climate.set_current_temperature(27)
+        await send_ext_temperature_change_event(entity, 30, event_timestamp)
+
+        event_timestamp = now - timedelta(minutes=3)
+        with patch(
+            "custom_components.versatile_thermostat.commons.NowClass.get_now",
+            return_value=event_timestamp,
+        ), patch("homeassistant.core.ServiceRegistry.async_call") as mock_service_call:
+            await send_temperature_change_event(entity, 25, event_timestamp)
+
+            # the regulated temperature should be upper (device offset is +2)
+            assert entity.regulated_target_temp < entity.target_temperature
+            assert entity.regulated_target_temp == 22.4
+
+            mock_service_call.assert_has_calls(
+                [
+                    call.service_call(
+                        "climate",
+                        "set_temperature",
+                        {
+                            "entity_id": "climate.mock_climate",
+                            "temperature": 24.4,  # 22.4 + 2° of offset
                             "target_temp_high": 30,
                             "target_temp_low": 15,
                         },