From 062f8a617d099d924f758ca80cb94d9e7014d716 Mon Sep 17 00:00:00 2001 From: Jean-Marc Collin Date: Sat, 12 Oct 2024 10:46:13 +0200 Subject: [PATCH] Fix #533 (#542) Clean some pylint hints Avoid 2 times open percentage send at startup Co-authored-by: Jean-Marc Collin --- .../versatile_thermostat/number.py | 5 +- .../versatile_thermostat/prop_algorithm.py | 4 +- .../versatile_thermostat/select.py | 12 +- .../versatile_thermostat/thermostat_valve.py | 48 ++--- .../versatile_thermostat/underlyings.py | 6 +- tests/commons.py | 36 +++- tests/test_bugs.py | 165 +++++++++++++++++- 7 files changed, 240 insertions(+), 36 deletions(-) diff --git a/custom_components/versatile_thermostat/number.py b/custom_components/versatile_thermostat/number.py index b6236a3..af9b169 100644 --- a/custom_components/versatile_thermostat/number.py +++ b/custom_components/versatile_thermostat/number.py @@ -20,7 +20,6 @@ from homeassistant.components.climate import ( PRESET_COMFORT, PRESET_ECO, ) -from homeassistant.components.sensor import UnitOfTemperature from homeassistant.helpers.device_registry import DeviceInfo, DeviceEntryType from homeassistant.config_entries import ConfigEntry @@ -487,8 +486,8 @@ class TemperatureNumber( # pylint: disable=abstract-method ) ) - # We set the min, max and step from central config if relevant because it is possible that central config - # was not loaded at startup + # We set the min, max and step from central config if relevant because it is possible + # that central config was not loaded at startup self.init_min_max_step() def __str__(self): diff --git a/custom_components/versatile_thermostat/prop_algorithm.py b/custom_components/versatile_thermostat/prop_algorithm.py index 12b3b1e..77fe427 100644 --- a/custom_components/versatile_thermostat/prop_algorithm.py +++ b/custom_components/versatile_thermostat/prop_algorithm.py @@ -1,4 +1,5 @@ """ The TPI calculation module """ +# pylint: disable='line-too-long' import logging from homeassistant.components.climate import HVACMode @@ -15,6 +16,7 @@ FUNCTION_TYPE = [PROPORTIONAL_FUNCTION_ATAN, PROPORTIONAL_FUNCTION_LINEAR] def is_number(value): + """check if value is a number""" return isinstance(value, (int, float)) @@ -61,7 +63,7 @@ class PropAlgorithm: minimal_activation_delay, ) raise TypeError( - f"TPI parameters are not set correctly. VTherm will not work as expected. Please reconfigure it correctly. See previous log for values" + "TPI parameters are not set correctly. VTherm will not work as expected. Please reconfigure it correctly. See previous log for values" ) self._vtherm_entity_id = vtherm_entity_id diff --git a/custom_components/versatile_thermostat/select.py b/custom_components/versatile_thermostat/select.py index 1f17ec3..dd1df35 100644 --- a/custom_components/versatile_thermostat/select.py +++ b/custom_components/versatile_thermostat/select.py @@ -3,19 +3,15 @@ """ Implements the VersatileThermostat select component """ import logging -from homeassistant.const import EVENT_HOMEASSISTANT_START -from homeassistant.core import HomeAssistant, CoreState, callback +from homeassistant.core import HomeAssistant -from homeassistant.components.climate import ClimateEntity, DOMAIN as CLIMATE_DOMAIN from homeassistant.components.select import SelectEntity from homeassistant.helpers.device_registry import DeviceInfo, DeviceEntryType from homeassistant.config_entries import ConfigEntry from homeassistant.helpers.restore_state import RestoreEntity from homeassistant.helpers.entity_platform import AddEntitiesCallback -from homeassistant.helpers.entity_component import EntityComponent from custom_components.versatile_thermostat.base_thermostat import ( - BaseThermostat, ConfigData, ) @@ -126,6 +122,12 @@ class CentralModeSelect(SelectEntity, RestoreEntity): self._attr_current_option = option await self.notify_central_mode_change(old_central_mode=old_option) + @overrides + def select_option(self, option: str) -> None: + """Change the selected option""" + # Update the VTherms which have temperature in central config + self.hass.create_task(self.async_select_option(option)) + async def notify_central_mode_change(self, old_central_mode: str | None = None): """Notify all VTherm that the central_mode have change""" api: VersatileThermostatAPI = VersatileThermostatAPI.get_vtherm_api(self.hass) diff --git a/custom_components/versatile_thermostat/thermostat_valve.py b/custom_components/versatile_thermostat/thermostat_valve.py index 1af53db..3865851 100644 --- a/custom_components/versatile_thermostat/thermostat_valve.py +++ b/custom_components/versatile_thermostat/thermostat_valve.py @@ -33,26 +33,24 @@ _LOGGER = logging.getLogger(__name__) class ThermostatOverValve(BaseThermostat[UnderlyingValve]): # pylint: disable=abstract-method """Representation of a class for a Versatile Thermostat over a Valve""" - _entity_component_unrecorded_attributes = ( - BaseThermostat._entity_component_unrecorded_attributes.union( - frozenset( - { - "is_over_valve", - "underlying_valve_0", - "underlying_valve_1", - "underlying_valve_2", - "underlying_valve_3", - "on_time_sec", - "off_time_sec", - "cycle_min", - "function", - "tpi_coef_int", - "tpi_coef_ext", - "auto_regulation_dpercent", - "auto_regulation_period_min", - "last_calculation_timestamp", - } - ) + _entity_component_unrecorded_attributes = BaseThermostat._entity_component_unrecorded_attributes.union( # pylint: disable=protected-access + frozenset( + { + "is_over_valve", + "underlying_valve_0", + "underlying_valve_1", + "underlying_valve_2", + "underlying_valve_3", + "on_time_sec", + "off_time_sec", + "cycle_min", + "function", + "tpi_coef_int", + "tpi_coef_ext", + "auto_regulation_dpercent", + "auto_regulation_period_min", + "last_calculation_timestamp", + } ) ) @@ -241,10 +239,16 @@ class ThermostatOverValve(BaseThermostat[UnderlyingValve]): # pylint: disable=a max(0, min(self.proportional_algorithm.on_percent, 1)) * 100 ) + # Issue 533 - don't filter with dtemp if valve should be close. Else it will never close + if new_valve_percent < self._auto_regulation_dpercent: + new_valve_percent = 0 + dpercent = new_valve_percent - self.valve_open_percent if ( - dpercent >= -1 * self._auto_regulation_dpercent - and dpercent < self._auto_regulation_dpercent + new_valve_percent > 0 + and -1 * self._auto_regulation_dpercent + <= dpercent + < self._auto_regulation_dpercent ): _LOGGER.debug( "%s - do not calculate TPI because regulation_dpercent (%.1f) is not exceeded", diff --git a/custom_components/versatile_thermostat/underlyings.py b/custom_components/versatile_thermostat/underlyings.py index a245079..2d92669 100644 --- a/custom_components/versatile_thermostat/underlyings.py +++ b/custom_components/versatile_thermostat/underlyings.py @@ -880,8 +880,10 @@ class UnderlyingValve(UnderlyingEntity): ): """We use this function to change the on_percent""" if force: - self._percent_open = self.cap_sent_value(self._percent_open) - await self.send_percent_open() + # self._percent_open = self.cap_sent_value(self._percent_open) + # await self.send_percent_open() + # avoid to send 2 times the same value at startup + self.set_valve_open_percent() @overrides def cap_sent_value(self, value) -> float: diff --git a/tests/commons.py b/tests/commons.py index 7071f9f..1443812 100644 --- a/tests/commons.py +++ b/tests/commons.py @@ -552,7 +552,14 @@ class MockNumber(NumberEntity): """A fake switch to be used instead real switch""" def __init__( # pylint: disable=unused-argument, dangerous-default-value - self, hass: HomeAssistant, unique_id, name, entry_infos={} + self, + hass: HomeAssistant, + unique_id, + name, + min=0, + max=100, + step=1, + entry_infos={}, ): """Init the switch""" super().__init__() @@ -562,7 +569,9 @@ class MockNumber(NumberEntity): self.entity_id = self.platform + "." + unique_id self._name = name self._attr_native_value = 0 - self._attr_native_min_value = 0 + self._attr_native_min_value = min + self._attr_native_max_value = max + self._attr_step = step @property def name(self) -> str: @@ -992,3 +1001,26 @@ async def set_climate_preset_temp( "commons tests set_cliamte_preset_temp: cannot find number entity with entity_id '%s'", number_entity_id, ) + + +async def set_all_climate_preset_temp( + hass, vtherm: BaseThermostat, temps: dict, number_entity_base_name: str +): + """Initialize all temp of preset for a VTherm entity""" + # We initialize + for preset_name, value in temps.items(): + + await set_climate_preset_temp(vtherm, preset_name, value) + + # Search the number entity to control it is correctly set + number_entity_name = ( + f"number.{number_entity_base_name}_preset_{preset_name}{PRESET_TEMP_SUFFIX}" + ) + temp_entity: NumberEntity = search_entity( + hass, + number_entity_name, + NUMBER_DOMAIN, + ) + assert temp_entity + # Because set_value is not implemented in Number class (really don't understand why...) + assert temp_entity.state == value diff --git a/tests/test_bugs.py b/tests/test_bugs.py index 963c76e..74c2020 100644 --- a/tests/test_bugs.py +++ b/tests/test_bugs.py @@ -8,11 +8,11 @@ from datetime import datetime, timedelta import logging +from homeassistant.core import HomeAssistant, State from homeassistant.components.climate import ( SERVICE_SET_TEMPERATURE, ) - from custom_components.versatile_thermostat.config_flow import ( VersatileThermostatBaseConfigFlow, ) @@ -1211,3 +1211,166 @@ 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_bug_533(hass: HomeAssistant, skip_hass_states_is_state): + """Test that with an over_valve and _auto_regulation_dpercent is set that the valve could close totally""" + + vtherm_api: VersatileThermostatAPI = VersatileThermostatAPI.get_vtherm_api(hass) + + # The temperatures to set + temps = { + "frost": 7.0, + "eco": 17.0, + "comfort": 19.0, + "boost": 21.0, + } + + config_entry = MockConfigEntry( + domain=DOMAIN, + title="TheOverValveMockName", + unique_id="overValveUniqueId", + data={ + CONF_NAME: "overValve", + CONF_THERMOSTAT_TYPE: CONF_THERMOSTAT_VALVE, + CONF_PROP_FUNCTION: PROPORTIONAL_FUNCTION_TPI, + CONF_TPI_COEF_INT: 0.5, + CONF_TPI_COEF_EXT: 0, + 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, + CONF_USE_WINDOW_FEATURE: False, + CONF_USE_MOTION_FEATURE: False, + CONF_USE_POWER_FEATURE: False, + CONF_USE_PRESENCE_FEATURE: False, + CONF_VALVE: "number.mock_valve", + CONF_AUTO_REGULATION_DTEMP: 10, # This parameter makes the bug + CONF_MINIMAL_ACTIVATION_DELAY: 30, + CONF_SECURITY_DELAY_MIN: 60, + }, + # | temps, + ) + + # Not used because number is not registred so we can use directly the underlying number + # fake_underlying_number = MockNumber( + # hass=hass, unique_id="mock_number", name="mock_number" + # ) + + vtherm: ThermostatOverClimate = await create_thermostat( + hass, config_entry, "climate.overvalve" + ) + + assert vtherm is not None + + tz = get_tz(hass) # pylint: disable=invalid-name + now: datetime = datetime.now(tz=tz) + + # Set all temps and check they are correctly initialized + await set_all_climate_preset_temp(hass, vtherm, temps, "overvalve") + await send_temperature_change_event(vtherm, 15, now) + await send_ext_temperature_change_event(vtherm, 15, now) + + # 1. Set mode to Heat and preset to Comfort + await vtherm.async_set_hvac_mode(HVACMode.HEAT) + with patch( + "homeassistant.core.StateMachine.get", + return_value=State( + entity_id="number.mock_valve", + state="100", + attributes={"min": 0, "max": 100}, + ), + ), patch("homeassistant.core.ServiceRegistry.async_call") as mock_service_call: + await vtherm.async_set_preset_mode(PRESET_COMFORT) + await hass.async_block_till_done() + + assert vtherm.target_temperature == 19.0 + assert mock_service_call.call_count == 1 + mock_service_call.assert_has_calls( + [ + call.async_call( + domain="number", + service="set_value", + service_data={"value": 100}, + target={"entity_id": "number.mock_valve"}, + ), + ] + ) + + # 2. set current temperature to 18 -> still 50% open, so there is a call + now = now + timedelta(minutes=1) + with patch( + "homeassistant.core.StateMachine.get", + return_value=State( + entity_id="number.mock_valve", + state="100", + attributes={"min": 0, "max": 100}, + ), + ), patch("homeassistant.core.ServiceRegistry.async_call") as mock_service_call: + await send_temperature_change_event(vtherm, 18, now) + await hass.async_block_till_done() + + assert mock_service_call.call_count == 1 + mock_service_call.assert_has_calls( + [ + call.async_call( + domain="number", + service="set_value", + service_data={"value": 50}, + target={"entity_id": "number.mock_valve"}, + ), + ] + ) + + # 3. set current temperature to 18.8 -> still 10% open, so there is one call + now = now + timedelta(minutes=1) + with patch( + "homeassistant.core.StateMachine.get", + return_value=State( + entity_id="number.mock_valve", + state="50", + attributes={"min": 0, "max": 100}, + ), + ), patch("homeassistant.core.ServiceRegistry.async_call") as mock_service_call: + await send_temperature_change_event(vtherm, 18.8, now) + await hass.async_block_till_done() + + assert mock_service_call.call_count == 1 + mock_service_call.assert_has_calls( + [ + call.async_call( + domain="number", + service="set_value", + service_data={"value": 10}, + target={"entity_id": "number.mock_valve"}, + ), + ] + ) + + # 4. set current temperature to 19 -> should have 0% open and one call to set the 0 + now = now + timedelta(minutes=1) + with patch( + "homeassistant.core.StateMachine.get", + return_value=State( + entity_id="number.mock_valve", + state="10", # the previous value + attributes={"min": 0, "max": 100}, + ), + ), patch("homeassistant.core.ServiceRegistry.async_call") as mock_service_call: + await send_temperature_change_event(vtherm, 19, now) + await hass.async_block_till_done() + + assert mock_service_call.call_count == 1 + mock_service_call.assert_has_calls( + [ + call.async_call( + domain="number", + service="set_value", + service_data={"value": 0}, + target={"entity_id": "number.mock_valve"}, + ), + ] + )