Issue #467 - Always apply offset compensation (#567)

Co-authored-by: Jean-Marc Collin <jean-marc.collin-extern@renault.com>

custom_components/versatile_thermostat/base_thermostat.py

@@ -1654,9 +1654,28 @@ class BaseThermostat(ClimateEntity, RestoreEntity, Generic[T]):
 
             if not long_enough:
                 _LOGGER.debug(
-                    "Motion delay condition is not satisfied. Ignore motion event"
+                    "Motion delay condition is not satisfied (the sensor have change its state during the delay). Check motion sensor state"
                 )
-            else:
+                # Get sensor current state
+                motion_state = self.hass.states.get(self._motion_sensor_entity_id)
+                _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
+                    )
+                    long_enough = True
+                else:
+                    long_enough = False
+
+            if long_enough:
                 _LOGGER.debug("%s - Motion delay condition is satisfied", self)
                 self._motion_state = new_state.state
                 if self._attr_preset_mode == PRESET_ACTIVITY:
@@ -1679,6 +1698,11 @@ class BaseThermostat(ClimateEntity, RestoreEntity, Generic[T]):
                     )
                 self.recalculate()
                 await self.async_control_heating(force=True)
+            else:
+                self._motion_state = (
+                    STATE_ON if new_state.state == STATE_OFF else STATE_OFF
+                )
+
             self._motion_call_cancel = None
 
         im_on = self._motion_state == STATE_ON

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 (
+                # and (
-                    (
+                #     (
-                        self.hvac_mode == HVACMode.COOL
+                #         self.hvac_mode == HVACMode.COOL
-                        and self.target_temperature < self.current_temperature
+                #         and self.target_temperature < self.current_temperature
-                    )
+                #     )
-                    or (
+                #     or (
-                        self.hvac_mode == HVACMode.HEAT
+                #         self.hvac_mode == HVACMode.HEAT
-                        and self.target_temperature > self.current_temperature
+                #         and self.target_temperature > self.current_temperature
-                    )
+                #     )
-                )
+                # )
             ):
                 offset_temp = device_temp - self.current_temperature
 
@@ -692,6 +693,13 @@ class ThermostatOverClimate(BaseThermostat[UnderlyingClimate]):
             else None
         )
 
+        last_sent_temperature = under.last_sent_temperature or 0
+        under_temp_diff = (
+            (new_target_temp - last_sent_temperature) if new_target_temp else 0
+        )
+        if -1 < under_temp_diff < 1:
+            under_temp_diff = 0
+
         # 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:
@@ -702,7 +710,7 @@ class ThermostatOverClimate(BaseThermostat[UnderlyingClimate]):
         if (
             new_hvac_mode == self._hvac_mode
             and new_hvac_action == old_hvac_action
-            and new_target_temp is None
+            and under_temp_diff == 0
             and (new_fan_mode is None or new_fan_mode == self._attr_fan_mode)
         ):
             _LOGGER.debug(
@@ -834,11 +842,8 @@ class ThermostatOverClimate(BaseThermostat[UnderlyingClimate]):
                 under.last_sent_temperature,
                 new_target_temp,
             )
-            if (
+            # if the underlying have change its target temperature
-                # if the underlying have change its target temperature
+            if under_temp_diff != 0:
-                new_target_temp is not None
-                and new_target_temp != under.last_sent_temperature
-            ):
                 _LOGGER.info(
                     "%s - Target temp in underlying have change to %s (vs %s)",
                     self,
@@ -849,7 +854,7 @@ class ThermostatOverClimate(BaseThermostat[UnderlyingClimate]):
                 changes = True
             else:
                 _LOGGER.debug(
-                    "%s - Forget the eventual underlying temperature change because VTherm is regulated",
+                    "%s - Forget the eventual underlying temperature change there is no real change",
                     self,
                 )
 

custom_components/versatile_thermostat/underlyings.py

@@ -550,14 +550,11 @@ class UnderlyingClimate(UnderlyingEntity):
     def is_device_active(self):
         """If the toggleable device is currently active."""
         if self.is_initialized:
-            return (
+            return self.hvac_mode != HVACMode.OFF and self.hvac_action not in [
-                self._underlying_climate.hvac_mode != HVACMode.OFF
+                HVACAction.IDLE,
-                and self._underlying_climate.hvac_action
+                HVACAction.OFF,
-                not in [
+                None,
-                    HVACAction.IDLE,
+            ]
-                    HVACAction.OFF,
-                ]
-            )
         else:
             return None
 
@@ -650,7 +647,36 @@ class UnderlyingClimate(UnderlyingEntity):
         """Get the hvac action of the underlying"""
         if not self.is_initialized:
             return None
-        return self._underlying_climate.hvac_action
+
+        hvac_action = self._underlying_climate.hvac_action
+        if hvac_action is None:
+            target = (
+                self.underlying_target_temperature
+                or self._thermostat.target_temperature
+            )
+            current = (
+                self.underlying_current_temperature
+                or self._thermostat.current_temperature
+            )
+            hvac_mode = self.hvac_mode
+
+            _LOGGER.debug(
+                "%s - hvac_action simulation target=%s, current=%s, hvac_mode=%s",
+                self,
+                target,
+                current,
+                hvac_mode,
+            )
+            hvac_action = HVACAction.IDLE
+            if target is not None and current is not None:
+                dtemp = target - current
+
+                if hvac_mode == HVACMode.COOL and dtemp < 0:
+                    hvac_action = HVACAction.COOLING
+                elif hvac_mode in [HVACMode.HEAT, HVACMode.HEAT_COOL] and dtemp > 0:
+                    hvac_action = HVACAction.HEATING
+
+        return hvac_action
 
     @property
     def hvac_mode(self) -> HVACMode | None:
@@ -730,18 +756,19 @@ class UnderlyingClimate(UnderlyingEntity):
         return self._underlying_climate.target_temperature_low
 
     @property
-    def target_temperature(self) -> float:
+    def underlying_target_temperature(self) -> float:
         """Get the target_temperature"""
         if not self.is_initialized:
             return None
-        return self._underlying_climate.target_temperature
 
-    @property
+        if not hasattr(self._underlying_climate, "target_temperature"):
-    def is_aux_heat(self) -> bool:
+            return None
-        """Get the is_aux_heat"""
+        else:
-        if not self.is_initialized:
+            return self._underlying_climate.target_temperature
-            return False
+
-        return self._underlying_climate.is_aux_heat
+        # return self._hass.states.get(self._entity_id).attributes.get(
+        #    "target_temperature"
+        # )
 
     @property
     def underlying_current_temperature(self) -> float | None:
@@ -752,8 +779,17 @@ class UnderlyingClimate(UnderlyingEntity):
 
         if not hasattr(self._underlying_climate, "current_temperature"):
             return None
+        else:
+            return self._underlying_climate.current_temperature
 
-        return self._hass.states.get(self._entity_id).attributes.get("current_temperature")
+        # return self._hass.states.get(self._entity_id).attributes.get("current_temperature")
+
+    @property
+    def is_aux_heat(self) -> bool:
+        """Get the is_aux_heat"""
+        if not self.is_initialized:
+            return False
+        return self._underlying_climate.is_aux_heat
 
     def turn_aux_heat_on(self) -> None:
         """Turn auxiliary heater on."""

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)
 
@@ -109,8 +114,8 @@ async def test_movement_management_time_not_enough(
         # because no motion is detected yet
         assert entity.target_temperature == 18
         # state is not changed if time is not enough
-        assert entity.motion_state is None
+        assert entity.motion_state is STATE_OFF
-        assert entity.presence_state == "on"
+        assert entity.presence_state == STATE_ON
 
         assert mock_send_event.call_count == 0
         # Change is not confirmed
@@ -141,8 +146,8 @@ async def test_movement_management_time_not_enough(
         assert entity.preset_mode is PRESET_ACTIVITY
         # because motion is detected yet
         assert entity.target_temperature == 19
-        assert entity.motion_state == "on"
+        assert entity.motion_state == STATE_ON
-        assert entity.presence_state == "on"
+        assert entity.presence_state == STATE_ON
 
     # stop detecting motion with off delay too low
     with patch(
@@ -156,19 +161,24 @@ 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)
 
-         # Will return False -> we will stay to movement On
+        # Will return False -> we will stay to movement On
         await try_condition(None)
 
         assert entity.hvac_mode is HVACMode.HEAT
         assert entity.preset_mode is PRESET_ACTIVITY
         # because no motion is detected yet
         assert entity.target_temperature == 19
-        assert entity.motion_state == "on"
+        assert entity.motion_state == STATE_ON
-        assert entity.presence_state == "on"
+        assert entity.presence_state == STATE_ON
 
         assert mock_send_event.call_count == 0
         # The heater must heat now
@@ -192,15 +202,15 @@ async def test_movement_management_time_not_enough(
         event_timestamp = now - timedelta(minutes=1)
         try_condition = await send_motion_change_event(entity, False, True, event_timestamp)
 
-         # Will return True -> we will switch to movement Off
+        # Will return True -> we will switch to movement Off
         await try_condition(None)
 
         assert entity.hvac_mode is HVACMode.HEAT
         assert entity.preset_mode is PRESET_ACTIVITY
         # because no motion is detected yet
         assert entity.target_temperature == 18
-        assert entity.motion_state == "off"
+        assert entity.motion_state == STATE_OFF
-        assert entity.presence_state == "on"
+        assert entity.presence_state == STATE_ON
 
         assert mock_send_event.call_count == 0
         # The heater must stop heating now
@@ -214,7 +224,7 @@ async def test_movement_management_time_not_enough(
 async def test_movement_management_time_enough_and_presence(
     hass: HomeAssistant, skip_hass_states_is_state
 ):
-    """Test the Presence management when time is not enough"""
+    """Test the Motion management when time is not enough"""
 
     entry = MockConfigEntry(
         domain=DOMAIN,
@@ -479,7 +489,7 @@ async def test_movement_management_time_enoughand_not_presence(
 async def test_movement_management_with_stop_during_condition(
     hass: HomeAssistant, skip_hass_states_is_state
 ):
-    """Test the Presence management when the movement sensor switch to off and then to on during the test condition"""
+    """Test the Motion management when the movement sensor switch to off and then to on during the test condition"""
 
     entry = MockConfigEntry(
         domain=DOMAIN,
@@ -558,9 +568,13 @@ async def test_movement_management_with_stop_during_condition(
     ) as mock_heater_off, patch(
         "custom_components.versatile_thermostat.underlyings.UnderlyingSwitch.is_device_active",
         return_value=True,
-    ), patch("homeassistant.helpers.condition.state", return_value=True): # Not needed for this test
+    ), patch(
+        "homeassistant.helpers.condition.state", return_value=True
+    ):  # Not needed for this test
         event_timestamp = now - timedelta(minutes=5)
-        try_condition1 = await send_motion_change_event(entity, True, False, event_timestamp)
+        try_condition1 = await send_motion_change_event(
+            entity, True, False, event_timestamp
+        )
 
         assert try_condition1 is not None
 
@@ -573,8 +587,10 @@ async def test_movement_management_with_stop_during_condition(
 
         # Send a stop detection
         event_timestamp = now - timedelta(minutes=4)
-        try_condition = await send_motion_change_event(entity, False, True, event_timestamp)
+        try_condition = await send_motion_change_event(
-        assert try_condition is None    # The timer should not have been stopped
+            entity, False, True, event_timestamp
+        )
+        assert try_condition is None  # The timer should not have been stopped
 
         assert entity.hvac_mode is HVACMode.HEAT
         assert entity.preset_mode is PRESET_ACTIVITY
@@ -584,8 +600,12 @@ async def test_movement_management_with_stop_during_condition(
 
         # Resend a start detection
         event_timestamp = now - timedelta(minutes=3)
-        try_condition = await send_motion_change_event(entity, True, False, event_timestamp)
+        try_condition = await send_motion_change_event(
-        assert try_condition is None  # The timer should not have been restarted (we keep the first one)
+            entity, True, False, event_timestamp
+        )
+        assert (
+            try_condition is None
+        )  # The timer should not have been restarted (we keep the first one)
 
         assert entity.hvac_mode is HVACMode.HEAT
         assert entity.preset_mode is PRESET_ACTIVITY
@@ -596,6 +616,122 @@ async def test_movement_management_with_stop_during_condition(
 
         await try_condition1(None)
         # We should have switch this time
-        assert entity.target_temperature == 19 # Boost
+        assert entity.target_temperature == 19  # Boost
-        assert entity.motion_state == "on"     # switch to movement on
+        assert entity.motion_state == "on"  # switch to movement on
         assert entity.presence_state == "off"  # Non change
+
+
+@pytest.mark.parametrize("expected_lingering_tasks", [True])
+@pytest.mark.parametrize("expected_lingering_timers", [True])
+async def test_movement_management_with_stop_during_condition_last_state_on(
+    hass: HomeAssistant, skip_hass_states_is_state
+):
+    """Test the Motion management when the movement sensor switch to off and then to on during the test condition"""
+
+    entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="TheOverSwitchMockName",
+        unique_id="uniqueId",
+        data={
+            CONF_NAME: "TheOverSwitchMockName",
+            CONF_THERMOSTAT_TYPE: CONF_THERMOSTAT_SWITCH,
+            CONF_TEMP_SENSOR: "sensor.mock_temp_sensor",
+            CONF_EXTERNAL_TEMP_SENSOR: "sensor.mock_ext_temp_sensor",
+            CONF_CYCLE_MIN: 5,
+            CONF_TEMP_MIN: 15,
+            CONF_TEMP_MAX: 30,
+            "eco_temp": 17,
+            "comfort_temp": 18,
+            "boost_temp": 19,
+            "eco_away_temp": 17,
+            "comfort_away_temp": 18,
+            "boost_away_temp": 19,
+            CONF_USE_WINDOW_FEATURE: False,
+            CONF_USE_MOTION_FEATURE: True,
+            CONF_USE_POWER_FEATURE: False,
+            CONF_USE_PRESENCE_FEATURE: False,
+            CONF_HEATER: "switch.mock_switch",
+            CONF_PROP_FUNCTION: PROPORTIONAL_FUNCTION_TPI,
+            CONF_TPI_COEF_INT: 0.3,
+            CONF_TPI_COEF_EXT: 0.01,
+            CONF_MINIMAL_ACTIVATION_DELAY: 30,
+            CONF_SECURITY_DELAY_MIN: 5,
+            CONF_SECURITY_MIN_ON_PERCENT: 0.3,
+            CONF_MOTION_SENSOR: "binary_sensor.mock_motion_sensor",
+            CONF_MOTION_DELAY: 10,
+            CONF_MOTION_OFF_DELAY: 30,
+            CONF_MOTION_PRESET: "boost",
+            CONF_NO_MOTION_PRESET: "comfort",
+        },
+    )
+
+    entity: BaseThermostat = await create_thermostat(
+        hass, entry, "climate.theoverswitchmockname"
+    )
+    assert entity
+
+    tz = get_tz(hass)  # pylint: disable=invalid-name
+    now: datetime = datetime.now(tz=tz)
+
+    # 0. 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"
+    ):
+        await entity.async_set_hvac_mode(HVACMode.HEAT)
+        await entity.async_set_preset_mode(PRESET_ACTIVITY)
+
+        assert entity.hvac_mode is HVACMode.HEAT
+        assert entity.preset_mode is PRESET_ACTIVITY
+        # because no motion is detected yet
+        assert entity.target_temperature == 18
+        assert entity.motion_state is None
+
+        event_timestamp = now - timedelta(minutes=6)
+        await send_temperature_change_event(entity, 18, event_timestamp)
+        await send_ext_temperature_change_event(entity, 10, event_timestamp)
+
+    # 1. starts detecting motion but the sensor is off
+    with patch(
+        "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(
+            entity_id="binary_sensor.mock_motion_sensor", state=STATE_OFF
+        ),
+    ):
+        event_timestamp = now - timedelta(minutes=5)
+        try_condition1 = await send_motion_change_event(
+            entity, True, False, event_timestamp
+        )
+
+        assert try_condition1 is not None
+
+        await try_condition1(None)
+
+        # because no motion is detected yet -> condition.state is False and sensor is not active
+        assert entity.target_temperature == 18
+        assert entity.motion_state is STATE_OFF
+
+    # 2. starts detecting motion but the sensor is on
+    with patch(
+        "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(
+            entity_id="binary_sensor.mock_motion_sensor", state=STATE_ON
+        ),
+    ):
+        event_timestamp = now - timedelta(minutes=5)
+        try_condition1 = await send_motion_change_event(
+            entity, True, False, event_timestamp
+        )
+
+        assert try_condition1 is not None
+
+        await try_condition1(None)
+
+        # because no motion is detected yet -> condition.state is False and sensor is not active
+        assert entity.target_temperature == 19
+        assert entity.motion_state is STATE_ON

tests/test_overclimate.py

@@ -302,6 +302,23 @@ async def test_bug_101(
         assert entity.target_temperature == 12.75
         assert entity.preset_mode is PRESET_NONE
 
+        # 4. Change the target temp with < 1 value. The value should not 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.5,  # 12.75 means 13 in vtherm
+            True,
+            "climate.mock_climate",  # the underlying climate entity id
+        )
+        assert entity.target_temperature == 12.75
+        assert entity.preset_mode is PRESET_NONE
+
 
 @pytest.mark.parametrize("expected_lingering_timers", [True])
 async def test_bug_508(