From 92b246fda9a0e676d2c756dd8563d6fbc4d08238 Mon Sep 17 00:00:00 2001 From: tizianodeg <65893913+tizianodeg@users.noreply.github.com> Date: Wed, 8 May 2024 21:02:43 +0200 Subject: [PATCH] Fix nibe_heatpump climate for models without cooling support (#114599) * fix nibe_heatpump climate for models without cooling support * add test for set temperature with no cooling support * fixup use self._coil_setpoint_cool None * fixup add new test to explicitly test unsupported cooling --- .../components/nibe_heatpump/climate.py | 27 ++- .../nibe_heatpump/snapshots/test_climate.ambr | 208 ++++++++++++++++++ .../components/nibe_heatpump/test_climate.py | 73 +++++- 3 files changed, 294 insertions(+), 14 deletions(-) diff --git a/homeassistant/components/nibe_heatpump/climate.py b/homeassistant/components/nibe_heatpump/climate.py index 746ed26687d..3a0a405d5b8 100644 --- a/homeassistant/components/nibe_heatpump/climate.py +++ b/homeassistant/components/nibe_heatpump/climate.py @@ -112,7 +112,12 @@ class NibeClimateEntity(CoordinatorEntity[Coordinator], ClimateEntity): self._coil_current = _get(climate.current) self._coil_setpoint_heat = _get(climate.setpoint_heat) - self._coil_setpoint_cool = _get(climate.setpoint_cool) + self._coil_setpoint_cool: None | Coil + try: + self._coil_setpoint_cool = _get(climate.setpoint_cool) + except CoilNotFoundException: + self._coil_setpoint_cool = None + self._attr_hvac_modes = [HVACMode.AUTO, HVACMode.HEAT] self._coil_prio = _get(unit.prio) self._coil_mixing_valve_state = _get(climate.mixing_valve_state) if climate.active_accessory is None: @@ -147,8 +152,10 @@ class NibeClimateEntity(CoordinatorEntity[Coordinator], ClimateEntity): self._attr_hvac_mode = mode setpoint_heat = _get_float(self._coil_setpoint_heat) - setpoint_cool = _get_float(self._coil_setpoint_cool) - + if self._coil_setpoint_cool: + setpoint_cool = _get_float(self._coil_setpoint_cool) + else: + setpoint_cool = None if mode == HVACMode.HEAT_COOL: self._attr_target_temperature = None self._attr_target_temperature_low = setpoint_heat @@ -207,9 +214,12 @@ class NibeClimateEntity(CoordinatorEntity[Coordinator], ClimateEntity): self._coil_setpoint_heat, temperature ) elif hvac_mode == HVACMode.COOL: - await coordinator.async_write_coil( - self._coil_setpoint_cool, temperature - ) + if self._coil_setpoint_cool: + await coordinator.async_write_coil( + self._coil_setpoint_cool, temperature + ) + else: + raise ValueError(f"{hvac_mode} mode not supported for {self.name}") else: raise ValueError( "'set_temperature' requires 'hvac_mode' when passing" @@ -220,7 +230,10 @@ class NibeClimateEntity(CoordinatorEntity[Coordinator], ClimateEntity): if (temperature := kwargs.get(ATTR_TARGET_TEMP_LOW)) is not None: await coordinator.async_write_coil(self._coil_setpoint_heat, temperature) - if (temperature := kwargs.get(ATTR_TARGET_TEMP_HIGH)) is not None: + if ( + self._coil_setpoint_cool + and (temperature := kwargs.get(ATTR_TARGET_TEMP_HIGH)) is not None + ): await coordinator.async_write_coil(self._coil_setpoint_cool, temperature) async def async_set_hvac_mode(self, hvac_mode: HVACMode) -> None: diff --git a/tests/components/nibe_heatpump/snapshots/test_climate.ambr b/tests/components/nibe_heatpump/snapshots/test_climate.ambr index 0c5cd46f5db..fb3e2d1003b 100644 --- a/tests/components/nibe_heatpump/snapshots/test_climate.ambr +++ b/tests/components/nibe_heatpump/snapshots/test_climate.ambr @@ -319,6 +319,214 @@ 'state': 'auto', }) # --- +# name: test_basic[Model.F730-s1-climate.climate_system_s1][cooling] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'current_temperature': 20.5, + 'friendly_name': 'Climate System S1', + 'hvac_action': , + 'hvac_modes': list([ + , + , + ]), + 'max_temp': 35.0, + 'min_temp': 5.0, + 'supported_features': , + 'target_temp_high': None, + 'target_temp_low': 21.0, + 'target_temp_step': 0.5, + 'temperature': None, + }), + 'context': , + 'entity_id': 'climate.climate_system_s1', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': 'heat_cool', + }) +# --- +# name: test_basic[Model.F730-s1-climate.climate_system_s1][heating (auto)] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'current_temperature': 20.5, + 'friendly_name': 'Climate System S1', + 'hvac_action': , + 'hvac_modes': list([ + , + , + ]), + 'max_temp': 35.0, + 'min_temp': 5.0, + 'supported_features': , + 'target_temp_high': None, + 'target_temp_low': None, + 'target_temp_step': 0.5, + 'temperature': None, + }), + 'context': , + 'entity_id': 'climate.climate_system_s1', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': 'auto', + }) +# --- +# name: test_basic[Model.F730-s1-climate.climate_system_s1][heating (only)] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'current_temperature': 20.5, + 'friendly_name': 'Climate System S1', + 'hvac_action': , + 'hvac_modes': list([ + , + , + ]), + 'max_temp': 35.0, + 'min_temp': 5.0, + 'supported_features': , + 'target_temp_high': None, + 'target_temp_low': None, + 'target_temp_step': 0.5, + 'temperature': 21.0, + }), + 'context': , + 'entity_id': 'climate.climate_system_s1', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': 'heat', + }) +# --- +# name: test_basic[Model.F730-s1-climate.climate_system_s1][heating] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'current_temperature': 20.5, + 'friendly_name': 'Climate System S1', + 'hvac_action': , + 'hvac_modes': list([ + , + , + ]), + 'max_temp': 35.0, + 'min_temp': 5.0, + 'supported_features': , + 'target_temp_high': None, + 'target_temp_low': 21.0, + 'target_temp_step': 0.5, + 'temperature': None, + }), + 'context': , + 'entity_id': 'climate.climate_system_s1', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': 'heat_cool', + }) +# --- +# name: test_basic[Model.F730-s1-climate.climate_system_s1][idle (mixing valve)] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'current_temperature': 20.5, + 'friendly_name': 'Climate System S1', + 'hvac_action': , + 'hvac_modes': list([ + , + , + ]), + 'max_temp': 35.0, + 'min_temp': 5.0, + 'supported_features': , + 'target_temp_high': None, + 'target_temp_low': 21.0, + 'target_temp_step': 0.5, + 'temperature': None, + }), + 'context': , + 'entity_id': 'climate.climate_system_s1', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': 'heat_cool', + }) +# --- +# name: test_basic[Model.F730-s1-climate.climate_system_s1][initial] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'current_temperature': 20.5, + 'friendly_name': 'Climate System S1', + 'hvac_action': , + 'hvac_modes': list([ + , + , + ]), + 'max_temp': 35.0, + 'min_temp': 5.0, + 'supported_features': , + 'target_temp_high': None, + 'target_temp_low': 21.0, + 'target_temp_step': 0.5, + 'temperature': None, + }), + 'context': , + 'entity_id': 'climate.climate_system_s1', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': 'heat_cool', + }) +# --- +# name: test_basic[Model.F730-s1-climate.climate_system_s1][off (auto)] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'current_temperature': 20.5, + 'friendly_name': 'Climate System S1', + 'hvac_action': , + 'hvac_modes': list([ + , + , + ]), + 'max_temp': 35.0, + 'min_temp': 5.0, + 'supported_features': , + 'target_temp_high': None, + 'target_temp_low': None, + 'target_temp_step': 0.5, + 'temperature': None, + }), + 'context': , + 'entity_id': 'climate.climate_system_s1', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': 'auto', + }) +# --- +# name: test_basic[Model.F730-s1-climate.climate_system_s1][unavailable] + StateSnapshot({ + 'attributes': ReadOnlyDict({ + 'current_temperature': 20.5, + 'friendly_name': 'Climate System S1', + 'hvac_action': , + 'hvac_modes': list([ + , + , + ]), + 'max_temp': 35.0, + 'min_temp': 5.0, + 'supported_features': , + 'target_temp_high': None, + 'target_temp_low': None, + 'target_temp_step': 0.5, + 'temperature': None, + }), + 'context': , + 'entity_id': 'climate.climate_system_s1', + 'last_changed': , + 'last_reported': , + 'last_updated': , + 'state': 'auto', + }) +# --- # name: test_basic[Model.S320-s1-climate.climate_system_s1][cooling] StateSnapshot({ 'attributes': ReadOnlyDict({ diff --git a/tests/components/nibe_heatpump/test_climate.py b/tests/components/nibe_heatpump/test_climate.py index 3a468e51e83..c845f0eac4b 100644 --- a/tests/components/nibe_heatpump/test_climate.py +++ b/tests/components/nibe_heatpump/test_climate.py @@ -62,6 +62,7 @@ def _setup_climate_group( [ (Model.S320, "s1", "climate.climate_system_s1"), (Model.F1155, "s2", "climate.climate_system_s2"), + (Model.F730, "s1", "climate.climate_system_s1"), ], ) async def test_basic( @@ -139,7 +140,7 @@ async def test_active_accessory( (Model.F1155, "s2", "climate.climate_system_s2"), ], ) -async def test_set_temperature( +async def test_set_temperature_supported_cooling( hass: HomeAssistant, mock_connection: MockConnection, model: Model, @@ -149,7 +150,7 @@ async def test_set_temperature( entity_registry_enabled_by_default: None, snapshot: SnapshotAssertion, ) -> None: - """Test setting temperature.""" + """Test setting temperature for models with cooling support.""" climate, _ = _setup_climate_group(coils, model, climate_id) await async_add_model(hass, model) @@ -226,6 +227,62 @@ async def test_set_temperature( mock_connection.write_coil.reset_mock() +@pytest.mark.parametrize( + ("model", "climate_id", "entity_id"), + [ + (Model.F730, "s1", "climate.climate_system_s1"), + ], +) +async def test_set_temperature_unsupported_cooling( + hass: HomeAssistant, + mock_connection: MockConnection, + model: Model, + climate_id: str, + entity_id: str, + coils: dict[int, Any], + entity_registry_enabled_by_default: None, + snapshot: SnapshotAssertion, +) -> None: + """Test setting temperature for models that do not support cooling.""" + climate, _ = _setup_climate_group(coils, model, climate_id) + + await async_add_model(hass, model) + + coil_setpoint_heat = mock_connection.heatpump.get_coil_by_address( + climate.setpoint_heat + ) + + # Set temperature to heat + await hass.services.async_call( + PLATFORM_DOMAIN, + SERVICE_SET_TEMPERATURE, + { + ATTR_ENTITY_ID: entity_id, + ATTR_TEMPERATURE: 22, + ATTR_HVAC_MODE: HVACMode.HEAT, + }, + blocking=True, + ) + await hass.async_block_till_done() + assert mock_connection.write_coil.mock_calls == [ + call(CoilData(coil_setpoint_heat, 22)) + ] + + # Attempt to set temperature to cool should raise ValueError + with pytest.raises(ValueError): + await hass.services.async_call( + PLATFORM_DOMAIN, + SERVICE_SET_TEMPERATURE, + { + ATTR_ENTITY_ID: entity_id, + ATTR_TEMPERATURE: 22, + ATTR_HVAC_MODE: HVACMode.COOL, + }, + blocking=True, + ) + mock_connection.write_coil.reset_mock() + + @pytest.mark.parametrize( ("hvac_mode", "cooling_with_room_sensor", "use_room_sensor"), [ @@ -239,6 +296,7 @@ async def test_set_temperature( [ (Model.S320, "s1", "climate.climate_system_s1"), (Model.F1155, "s2", "climate.climate_system_s2"), + (Model.F730, "s1", "climate.climate_system_s1"), ], ) async def test_set_hvac_mode( @@ -283,10 +341,11 @@ async def test_set_hvac_mode( @pytest.mark.parametrize( - ("model", "climate_id", "entity_id"), + ("model", "climate_id", "entity_id", "unsupported_mode"), [ - (Model.S320, "s1", "climate.climate_system_s1"), - (Model.F1155, "s2", "climate.climate_system_s2"), + (Model.S320, "s1", "climate.climate_system_s1", HVACMode.DRY), + (Model.F1155, "s2", "climate.climate_system_s2", HVACMode.DRY), + (Model.F730, "s1", "climate.climate_system_s1", HVACMode.COOL), ], ) async def test_set_invalid_hvac_mode( @@ -295,6 +354,7 @@ async def test_set_invalid_hvac_mode( model: Model, climate_id: str, entity_id: str, + unsupported_mode: str, coils: dict[int, Any], entity_registry_enabled_by_default: None, ) -> None: @@ -302,14 +362,13 @@ async def test_set_invalid_hvac_mode( _setup_climate_group(coils, model, climate_id) await async_add_model(hass, model) - with pytest.raises(ValueError): await hass.services.async_call( PLATFORM_DOMAIN, SERVICE_SET_HVAC_MODE, { ATTR_ENTITY_ID: entity_id, - ATTR_HVAC_MODE: HVACMode.DRY, + ATTR_HVAC_MODE: unsupported_mode, }, blocking=True, )