From e8bb465b43f9543601f61cfbecbd49482400ba6d Mon Sep 17 00:00:00 2001 From: Jean-Marc Collin Date: Sat, 13 Jan 2024 11:28:26 +0000 Subject: [PATCH] Try to fix issue #334 - loop when underlying is late to update --- .../versatile_thermostat/base_thermostat.py | 5 +- .../thermostat_climate.py | 28 ++++- .../versatile_thermostat/underlyings.py | 8 ++ tests/commons.py | 14 ++- tests/test_bugs.py | 14 ++- tests/test_central_config.py | 5 +- tests/test_multiple_switch.py | 119 +++++++++++++++++- tests/test_window.py | 21 ++-- 8 files changed, 186 insertions(+), 28 deletions(-) diff --git a/custom_components/versatile_thermostat/base_thermostat.py b/custom_components/versatile_thermostat/base_thermostat.py index 2444890..753f986 100644 --- a/custom_components/versatile_thermostat/base_thermostat.py +++ b/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) diff --git a/custom_components/versatile_thermostat/thermostat_climate.py b/custom_components/versatile_thermostat/thermostat_climate.py index 446fb01..b0f09bb 100644 --- a/custom_components/versatile_thermostat/thermostat_climate.py +++ b/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: diff --git a/custom_components/versatile_thermostat/underlyings.py b/custom_components/versatile_thermostat/underlyings.py index b82e6a8..5c10af9 100644 --- a/custom_components/versatile_thermostat/underlyings.py +++ b/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, diff --git a/tests/commons.py b/tests/commons.py index 6a30a56..265a711 100644 --- a/tests/commons.py +++ b/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, diff --git a/tests/test_bugs.py b/tests/test_bugs.py index 4bc9715..acdf6e4 100644 --- a/tests/test_bugs.py +++ b/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( - "climate", - SERVICE_SET_HVAC_MODE, - {"entity_id": "climate.mock_climate", "hvac_mode": HVACMode.HEAT}, - ), + # call.async_call( + # "climate", + # SERVICE_SET_HVAC_MODE, + # {"entity_id": "climate.mock_climate", "hvac_mode": HVACMode.HEAT}, + # ), call.async_call( "climate", SERVICE_SET_TEMPERATURE, diff --git a/tests/test_central_config.py b/tests/test_central_config.py index 9421182..8fb9ebf 100644 --- a/tests/test_central_config.py +++ b/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 diff --git a/tests/test_multiple_switch.py b/tests/test_multiple_switch.py index b9ff945..e8d9963 100644 --- a/tests/test_multiple_switch.py +++ b/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( diff --git a/tests/test_window.py b/tests/test_window.py index 9bea20d..441d470 100644 --- a/tests/test_window.py +++ b/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