diff --git a/.github/workflows/pull.yml b/.github/workflows/pull.yml index 0912fd6..8a55856 100644 --- a/.github/workflows/pull.yml +++ b/.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 \ diff --git a/custom_components/versatile_thermostat/climate.py b/custom_components/versatile_thermostat/climate.py index 249ddc5..403cf4d 100644 --- a/custom_components/versatile_thermostat/climate.py +++ b/custom_components/versatile_thermostat/climate.py @@ -220,6 +220,7 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): _presence_state: bool _window_auto_state: bool _underlyings: list[UnderlyingEntity] + _last_change_time: datetime def __init__(self, hass: HomeAssistant, unique_id, name, entry_infos) -> None: """Initialize the thermostat.""" @@ -284,6 +285,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): self._current_tz = dt_util.get_time_zone(self._hass.config.time_zone) + self._last_change_time = None + self._underlyings = [] self.post_init(entry_infos) @@ -778,6 +781,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): else: self.hass.create_task(self._async_control_heating()) + self.reset_last_change_time() + await self.get_my_previous_state() if self.hass.state == CoreState.running: @@ -951,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 @@ -1233,6 +1237,8 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): # Ensure we update the current operation after changing the mode self.reset_last_temperature_time() + self.reset_last_change_time() + self.update_custom_attributes() self.async_write_ha_state() self.send_event(EventType.HVAC_MODE_EVENT, {"hvac_mode": self._hvac_mode}) @@ -1288,6 +1294,11 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): self.recalculate() self.send_event(EventType.PRESET_EVENT, {"preset": self._attr_preset_mode}) + def reset_last_change_time(self, old_preset_mode=None): + """Reset to now the last change time""" + self._last_change_time = datetime.now(tz=self._current_tz) + _LOGGER.debug("%s - last_change_time is now %s", self, self._last_change_time) + def reset_last_temperature_time(self, old_preset_mode=None): """Reset to now the last temperature time if conditions are satisfied""" if ( @@ -1363,6 +1374,7 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): await self._async_internal_set_temperature(temperature) self._attr_preset_mode = PRESET_NONE self.recalculate() + self.reset_last_change_time() await self._async_control_heating(force=True) async def _async_internal_set_temperature(self, temperature): @@ -1585,7 +1597,22 @@ 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""" + if changes: + self.async_write_ha_state() + self.update_custom_attributes() + await self._async_control_heating() + new_state = event.data.get("new_state") _LOGGER.debug("%s - _async_climate_changed new_state is %s", self, new_state) if not new_state: @@ -1606,6 +1633,11 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): else None ) + old_state_date_changed = old_state.last_changed if old_state and old_state.last_changed else None + old_state_date_updated = old_state.last_updated if old_state and old_state.last_updated else None + new_state_date_changed = new_state.last_changed if new_state and new_state.last_changed else None + new_state_date_updated = new_state.last_updated if new_state and new_state.last_updated else None + # 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: @@ -1621,24 +1653,9 @@ class VersatileThermostat(ClimateEntity, RestoreEntity): old_hvac_action, ) - if new_hvac_mode in [ - HVACMode.OFF, - HVACMode.HEAT, - HVACMode.COOL, - HVACMode.HEAT_COOL, - HVACMode.DRY, - HVACMode.AUTO, - HVACMode.FAN_ONLY, - None - ] and self._hvac_mode != new_hvac_mode: - changes = True - self._hvac_mode = new_hvac_mode - # Do not try to update all underlying state, else we will have a loop - if self._is_over_climate: - for under in self._underlyings: - await under.set_hvac_mode(new_hvac_mode) + _LOGGER.debug("%s - last_change_time=%s old_state_date_changed=%s old_state_date_updated=%s new_state_date_changed=%s new_state_date_updated=%s", self, self._last_change_time, old_state_date_changed, old_state_date_updated, new_state_date_changed, new_state_date_updated) - # Interpretation of hvac + # Interpretation of hvac action HVAC_ACTION_ON = [ # pylint: disable=invalid-name HVACAction.COOLING, HVACAction.DRYING, @@ -1677,18 +1694,43 @@ 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 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: + _LOGGER.info("%s - underlying event is received less than 10 sec after command. Forget it to avoid loop", self + ) + await end_climate_changed(changes) + return + + if new_hvac_mode in [ + HVACMode.OFF, + HVACMode.HEAT, + HVACMode.COOL, + HVACMode.HEAT_COOL, + HVACMode.DRY, + HVACMode.AUTO, + HVACMode.FAN_ONLY, + None + ] and self._hvac_mode != new_hvac_mode: + changes = True + self._hvac_mode = new_hvac_mode + # Update all underlyings state + if self._is_over_climate: + for under in self._underlyings: + await under.set_hvac_mode(new_hvac_mode) + if not changes: # try to manage new target temperature set if state _LOGGER.debug("Do temperature check. temperature is %s, new_state.attributes is %s", self.target_temperature, new_state.attributes) if self._is_over_climate and new_state.attributes and (new_target_temp := new_state.attributes.get("temperature")) and new_target_temp != self.target_temperature: - _LOGGER.info("%s - Target temp have change to %s", self, new_target_temp) + _LOGGER.info("%s - Target temp in underlying have change to %s", self, new_target_temp) await self.async_set_temperature(temperature = new_target_temp) changes = True - if changes: - self.async_write_ha_state() - self.update_custom_attributes() - await self._async_control_heating() + await end_climate_changed(changes) + @callback async def _async_update_temp(self, state: State): @@ -2103,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 = ( @@ -2387,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[ diff --git a/custom_components/versatile_thermostat/manifest.json b/custom_components/versatile_thermostat/manifest.json index 24fa9a6..b6753bd 100644 --- a/custom_components/versatile_thermostat/manifest.json +++ b/custom_components/versatile_thermostat/manifest.json @@ -14,6 +14,6 @@ "quality_scale": "silver", "requirements": [], "ssdp": [], - "version": "3.0.0", + "version": "3.5.3", "zeroconf": [] } \ No newline at end of file diff --git a/custom_components/versatile_thermostat/requirements_dev.txt b/custom_components/versatile_thermostat/requirements_dev.txt index ed73233..16ea588 100644 --- a/custom_components/versatile_thermostat/requirements_dev.txt +++ b/custom_components/versatile_thermostat/requirements_dev.txt @@ -1,2 +1,2 @@ -homeassistant==2023.10.1 +homeassistant==2023.10.3 ffmpeg \ No newline at end of file diff --git a/custom_components/versatile_thermostat/requirements_test.txt b/custom_components/versatile_thermostat/requirements_test.txt index 5babf02..4af38e4 100644 --- a/custom_components/versatile_thermostat/requirements_test.txt +++ b/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 \ No newline at end of file diff --git a/custom_components/versatile_thermostat/tests/test_bugs.py b/custom_components/versatile_thermostat/tests/test_bugs.py index e082355..b6a3a7c 100644 --- a/custom_components/versatile_thermostat/tests/test_bugs.py +++ b/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 diff --git a/custom_components/versatile_thermostat/tests/test_multiple_switch.py b/custom_components/versatile_thermostat/tests/test_multiple_switch.py index a5f21c4..28a2351 100644 --- a/custom_components/versatile_thermostat/tests/test_multiple_switch.py +++ b/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 diff --git a/hacs.json b/hacs.json index e56181a..38d8b72 100644 --- a/hacs.json +++ b/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" } \ No newline at end of file