Filter out-of-range target temperature sent from underlying climate devices (#581)

* Filter out-of-range temperature from underlying climate

* Fixed broken test case, added new test case for range filtering

README.md

@@ -468,21 +468,16 @@ and of course, configure the VTherm's self-regulation mode in **Expert** mode. A
 For the changes to be taken into account, you must either **completely restart Home Assistant** or just the **Versatile Thermostat integration** (Dev tools / Yaml / reloading the configuration / Versatile Thermostat).
 
 #### Internal temperature compensation
-Sometimes, it happens that the internal thermometer of the underlying (TRV, air conditioning, etc.) is so wrong that self-regulation is not enough to regulate.
-This happens when the internal thermometer is too close to the heat source. The internal temperature then rises much faster than the room temperature, which generates faults in the regulation.
-Example :
-1. the room temperature is 18°, the setpoint is 20°,
-2. the internal temperature of the equipment is 22°,
-3. if VTherm sends 21° as setpoint (= 20° + 1° auto-regulation), then the equipment will not heat because its internal temperature (22°) is above the setpoint (21°)
+Sometimes, a device’s internal temperature sensor (like in a TRV or AC) can give inaccurate readings, especially if it’s too close to a heat source. This can cause the device to stop heating too soon.
+For example:
+1. target temperature: 20 °C, room temperature: 18 °C,
+2. device’s internal sensor: 22 °C
+3. If the target temperature is increased to 21 °C, the device won’t heat because it thinks it’s already warm (internal temperature is 22°C).
 
-To overcome this, a new optional option was added in version 5.4: ![Use of internal temperature](images/config-use-internal-temp.png)
+The Adjust Setpoint for Room vs. TRV Temperature feature fixes this by adding the temperature difference between the room and the device’s internal reading to the target. In this case, VTherm would adjust the target to 25°C (21°C + 4°C difference), forcing the device to continue heating.
 
-When enabled, this function will add the difference between the internal temperature and the room temperature to the setpoint to force heating.
-In the example above, the difference is +4° (22° - 18°), so VTherm will send 25° (21°+4°) to the equipment forcing it to heat up.
-
-This difference is calculated for each underlying because each has its own internal temperature. Think of a VTherm which would be connected to 3 TRVs each with its internal temperature for example.
-
-We then obtain much more effective self-regulation which avoids the pitfall of large variations in faulty internal temperature.
+This adjustment is specific to each device, making the heating system more accurate and avoiding issues from faulty sensor readings.
+See ![Use of internal temperature](images/config-use-internal-temp.png)
 
 #### synthesis of the self-regulation algorithm
 The self-regulation algorithm can be summarized as follows:

custom_components/versatile_thermostat/base_thermostat.py

@@ -1654,11 +1654,20 @@ class BaseThermostat(ClimateEntity, RestoreEntity, Generic[T]):
 
             if not long_enough:
                 _LOGGER.debug(
-                    "Motion delay condition is not satisfied. Check motion sensor state"
+                    "Motion delay condition is not satisfied (the sensor have change its state during the delay). Check motion sensor state"
                 )
                 # Get sensor current state
                 motion_state = self.hass.states.get(self._motion_sensor_entity_id)
-                if motion_state == new_state.state and new_state.state == STATE_ON:
+                _LOGGER.debug(
+                    "%s - motion_state=%s, new_state.state=%s",
+                    self,
+                    motion_state.state,
+                    new_state.state,
+                )
+                if (
+                    motion_state.state == new_state.state
+                    and new_state.state == STATE_ON
+                ):
                     _LOGGER.debug(
                         "%s - the motion sensor is finally 'on' after the delay", self
                     )

custom_components/versatile_thermostat/thermostat_climate.py

@@ -228,17 +228,18 @@ class ThermostatOverClimate(BaseThermostat[UnderlyingClimate]):
                 and self.auto_regulation_use_device_temp
                 # and we have access to the device temp
                 and (device_temp := under.underlying_current_temperature) is not None
+                # issue 467 - always apply offset. TODO removes this if ok
                 # 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
-                    )
-                )
+                # 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
 
@@ -718,6 +719,22 @@ class ThermostatOverClimate(BaseThermostat[UnderlyingClimate]):
             )
             return
 
+        # Forget event when the new target temperature is out of range
+        if (
+            not new_target_temp is None
+            and not self._attr_min_temp is None
+            and not self._attr_max_temp is None
+            and not (self._attr_min_temp <= new_target_temp <= self._attr_max_temp)
+        ):
+            _LOGGER.debug(
+                "%s - underlying sent a target temperature (%s) which is out of configured min/max range (%s / %s). The value will be ignored",
+                self,
+                new_target_temp,
+                self._attr_min_temp,
+                self._attr_max_temp,
+            )
+            return
+
         # A real changes have to be managed
         _LOGGER.info(
             "%s - Underlying climate %s have changed. new_hvac_mode is %s (vs %s), new_hvac_action=%s (vs %s), new_target_temp=%s (vs %s), new_fan_mode=%s (vs %s)",

tests/test_movement.py

@@ -97,7 +97,12 @@ async def test_movement_management_time_not_enough(
         return_value=False,
     ), patch(
         "homeassistant.helpers.condition.state", return_value=False
-    ) as mock_condition:
+    ) as mock_condition, patch(
+        "homeassistant.core.StateMachine.get",
+        return_value=State(
+            entity_id="binary_sensor.mock_motion_sensor", state=STATE_OFF
+        ),
+    ):
         event_timestamp = now - timedelta(minutes=4)
         try_condition = await send_motion_change_event(entity, True, False, event_timestamp)
 
@@ -156,7 +161,12 @@ async def test_movement_management_time_not_enough(
         return_value=True,
     ) as mock_device_active, patch(
         "homeassistant.helpers.condition.state", return_value=False
-    ) as mock_condition:
+    ) as mock_condition, patch(
+        "homeassistant.core.StateMachine.get",
+        return_value=State(
+            entity_id="binary_sensor.mock_motion_sensor", state=STATE_OFF
+        ),
+    ):
         event_timestamp = now - timedelta(minutes=2)
         try_condition = await send_motion_change_event(entity, False, True, event_timestamp)
 
@@ -685,7 +695,10 @@ async def test_movement_management_with_stop_during_condition_last_state_on(
         "custom_components.versatile_thermostat.underlyings.UnderlyingSwitch.is_device_active",
         return_value=True,
     ), patch("homeassistant.helpers.condition.state", return_value=False), patch(
-        "homeassistant.core.StateMachine.get", return_value=STATE_OFF
+        "homeassistant.core.StateMachine.get",
+        return_value=State(
+            entity_id="binary_sensor.mock_motion_sensor", state=STATE_OFF
+        ),
     ):
         event_timestamp = now - timedelta(minutes=5)
         try_condition1 = await send_motion_change_event(
@@ -705,7 +718,10 @@ async def test_movement_management_with_stop_during_condition_last_state_on(
         "custom_components.versatile_thermostat.underlyings.UnderlyingSwitch.is_device_active",
         return_value=True,
     ), patch("homeassistant.helpers.condition.state", return_value=False), patch(
-        "homeassistant.core.StateMachine.get", return_value=STATE_ON
+        "homeassistant.core.StateMachine.get",
+        return_value=State(
+            entity_id="binary_sensor.mock_motion_sensor", state=STATE_ON
+        ),
     ):
         event_timestamp = now - timedelta(minutes=5)
         try_condition1 = await send_motion_change_event(

tests/test_overclimate.py

@@ -276,7 +276,7 @@ async def test_bug_101(
             HVACAction.OFF,
             HVACAction.OFF,
             now,
-            12.75,
+            entity.min_temp + 1,
             True,
             "climate.mock_climate",  # the underlying climate entity id
         )
@@ -295,11 +295,11 @@ async def test_bug_101(
             HVACAction.OFF,
             HVACAction.OFF,
             event_timestamp,
-            12.75,
+            entity.min_temp + 1,
             True,
             "climate.mock_climate",  # the underlying climate entity id
         )
-        assert entity.target_temperature == 12.75
+        assert entity.target_temperature == entity.min_temp + 1
         assert entity.preset_mode is PRESET_NONE
 
         # 4. Change the target temp with < 1 value. The value should not be taken
@@ -312,11 +312,11 @@ async def test_bug_101(
             HVACAction.OFF,
             HVACAction.OFF,
             event_timestamp,
-            12.5,  # 12.75 means 13 in vtherm
+            entity.min_temp + 1.5,
             True,
             "climate.mock_climate",  # the underlying climate entity id
         )
-        assert entity.target_temperature == 12.75
+        assert entity.target_temperature == entity.min_temp + 1
         assert entity.preset_mode is PRESET_NONE
 
 
@@ -523,3 +523,111 @@ async def test_bug_524(hass: HomeAssistant, skip_hass_states_is_state):
     await send_presence_change_event(vtherm, True, False, datetime.now())
     await hass.async_block_till_done()
     assert vtherm.target_temperature == 25
+
+@pytest.mark.parametrize("expected_lingering_tasks", [True])
+@pytest.mark.parametrize("expected_lingering_timers", [True])
+async def test_ignore_temp_outside_minmax_range(
+    hass: HomeAssistant,
+    skip_hass_states_is_state,
+    skip_turn_on_off_heater,
+    skip_send_event,
+):
+    """Test that when a underlying climate target temp is changed, the VTherm ignores the target temp if it is outside the min/max range"""
+
+    tz = get_tz(hass)  # pylint: disable=invalid-name
+    now: datetime = datetime.now(tz=tz)
+
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="TheOverClimateMockName",
+        unique_id="uniqueId",
+        data=PARTIAL_CLIMATE_NOT_REGULATED_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, HVACAction.HEATING
+    )
+
+    with patch(
+        "custom_components.versatile_thermostat.base_thermostat.BaseThermostat.send_event"
+    ) as mock_send_event, patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.find_underlying_climate",
+        return_value=fake_underlying_climate,
+    ) as mock_find_climate, patch(
+        "custom_components.versatile_thermostat.underlyings.UnderlyingClimate.set_hvac_mode"
+    ) as mock_underlying_set_hvac_mode:
+        entity = await create_thermostat(hass, entry, "climate.theoverclimatemockname")
+
+        assert entity
+
+        assert entity.name == "TheOverClimateMockName"
+        assert entity.is_over_climate is True
+        assert entity.hvac_mode is HVACMode.OFF
+        # because in MockClimate HVACAction is HEATING if hvac_mode is not set
+        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
+
+        # should have been called with EventType.PRESET_EVENT and EventType.HVAC_MODE_EVENT
+        assert mock_send_event.call_count == 2
+        mock_send_event.assert_has_calls(
+            [
+                call.send_event(EventType.PRESET_EVENT, {"preset": PRESET_NONE}),
+                call.send_event(
+                    EventType.HVAC_MODE_EVENT,
+                    {"hvac_mode": HVACMode.OFF},
+                ),
+            ]
+        )
+
+        assert mock_find_climate.call_count == 1
+        assert mock_find_climate.mock_calls[0] == call()
+        mock_find_climate.assert_has_calls([call.find_underlying_entity()])
+
+        # 1. Force preset mode
+        await entity.async_set_hvac_mode(HVACMode.HEAT)
+        assert entity.hvac_mode == HVACMode.HEAT
+        await entity.async_set_preset_mode(PRESET_COMFORT)
+        assert entity.preset_mode == PRESET_COMFORT
+
+        # 1. Try to set the target temperature to a below min_temp -> should be ignored
+        # Wait 11 sec
+        event_timestamp = now + timedelta(seconds=11)
+        assert entity.is_regulated is False
+        await send_climate_change_event_with_temperature(
+            entity,
+            HVACMode.HEAT,
+            HVACMode.HEAT,
+            HVACAction.OFF,
+            HVACAction.OFF,
+            event_timestamp,
+            entity.min_temp - 1,
+            True,
+            "climate.mock_climate",  # the underlying climate entity id
+        )
+        assert entity.target_temperature == 17
+
+        # 2. Try to set the target temperature to a above max_temp -> should be ignored
+        event_timestamp = event_timestamp + timedelta(seconds=11)
+        assert entity.is_regulated is False
+        await send_climate_change_event_with_temperature(
+            entity,
+            HVACMode.HEAT,
+            HVACMode.HEAT,
+            HVACAction.OFF,
+            HVACAction.OFF,
+            event_timestamp,
+            entity.max_temp + 1,
+            True,
+            "climate.mock_climate",  # the underlying climate entity id
+        )
+        assert entity.target_temperature == 17