From 5b0e00a74b70895a244b72a118c249ee5274eebe Mon Sep 17 00:00:00 2001 From: Alexei Chetroi Date: Fri, 17 Sep 2021 15:17:34 -0400 Subject: [PATCH] Refactor ZHA HVAC thermostat channel (#56238) * Refactor HVAC channel to use zigpy cached attributes * Allow named attributes in ZHA test attribute reports * Let attribute write to update cache * WIP Update tests * Cleanup --- .../components/zha/core/channels/hvac.py | 86 +++------ tests/components/zha/common.py | 11 +- tests/components/zha/conftest.py | 19 +- tests/components/zha/test_climate.py | 182 +++++++++--------- 4 files changed, 136 insertions(+), 162 deletions(-) diff --git a/homeassistant/components/zha/core/channels/hvac.py b/homeassistant/components/zha/core/channels/hvac.py index f4a3245bef8..31c75a0c794 100644 --- a/homeassistant/components/zha/core/channels/hvac.py +++ b/homeassistant/components/zha/core/channels/hvac.py @@ -125,134 +125,118 @@ class ThermostatChannel(ZigbeeChannel): "unoccupied_heating_setpoint": False, "unoccupied_cooling_setpoint": False, } - self._abs_max_cool_setpoint_limit = 3200 # 32C - self._abs_min_cool_setpoint_limit = 1600 # 16C - self._ctrl_seqe_of_oper = 0xFF - self._abs_max_heat_setpoint_limit = 3000 # 30C - self._abs_min_heat_setpoint_limit = 700 # 7C - self._running_mode = None - self._max_cool_setpoint_limit = None - self._max_heat_setpoint_limit = None - self._min_cool_setpoint_limit = None - self._min_heat_setpoint_limit = None - self._local_temp = None - self._occupancy = None - self._occupied_cooling_setpoint = None - self._occupied_heating_setpoint = None - self._pi_cooling_demand = None - self._pi_heating_demand = None - self._running_state = None - self._system_mode = None - self._unoccupied_cooling_setpoint = None - self._unoccupied_heating_setpoint = None @property def abs_max_cool_setpoint_limit(self) -> int: """Absolute maximum cooling setpoint.""" - return self._abs_max_cool_setpoint_limit + return self.cluster.get("abs_max_cool_setpoint_limit", 3200) @property def abs_min_cool_setpoint_limit(self) -> int: """Absolute minimum cooling setpoint.""" - return self._abs_min_cool_setpoint_limit + return self.cluster.get("abs_min_cool_setpoint_limit", 1600) @property def abs_max_heat_setpoint_limit(self) -> int: """Absolute maximum heating setpoint.""" - return self._abs_max_heat_setpoint_limit + return self.cluster.get("abs_max_heat_setpoint_limit", 3000) @property def abs_min_heat_setpoint_limit(self) -> int: """Absolute minimum heating setpoint.""" - return self._abs_min_heat_setpoint_limit + return self.cluster.get("abs_min_heat_setpoint_limit", 700) @property def ctrl_seqe_of_oper(self) -> int: """Control Sequence of operations attribute.""" - return self._ctrl_seqe_of_oper + return self.cluster.get("ctrl_seqe_of_oper", 0xFF) @property def max_cool_setpoint_limit(self) -> int: """Maximum cooling setpoint.""" - if self._max_cool_setpoint_limit is None: + sp_limit = self.cluster.get("max_cool_setpoint_limit") + if sp_limit is None: return self.abs_max_cool_setpoint_limit - return self._max_cool_setpoint_limit + return sp_limit @property def min_cool_setpoint_limit(self) -> int: """Minimum cooling setpoint.""" - if self._min_cool_setpoint_limit is None: + sp_limit = self.cluster.get("min_cool_setpoint_limit") + if sp_limit is None: return self.abs_min_cool_setpoint_limit - return self._min_cool_setpoint_limit + return sp_limit @property def max_heat_setpoint_limit(self) -> int: """Maximum heating setpoint.""" - if self._max_heat_setpoint_limit is None: + sp_limit = self.cluster.get("max_heat_setpoint_limit") + if sp_limit is None: return self.abs_max_heat_setpoint_limit - return self._max_heat_setpoint_limit + return sp_limit @property def min_heat_setpoint_limit(self) -> int: """Minimum heating setpoint.""" - if self._min_heat_setpoint_limit is None: + sp_limit = self.cluster.get("min_heat_setpoint_limit") + if sp_limit is None: return self.abs_min_heat_setpoint_limit - return self._min_heat_setpoint_limit + return sp_limit @property def local_temp(self) -> int | None: """Thermostat temperature.""" - return self._local_temp + return self.cluster.get("local_temp") @property def occupancy(self) -> int | None: """Is occupancy detected.""" - return self._occupancy + return self.cluster.get("occupancy") @property def occupied_cooling_setpoint(self) -> int | None: """Temperature when room is occupied.""" - return self._occupied_cooling_setpoint + return self.cluster.get("occupied_cooling_setpoint") @property def occupied_heating_setpoint(self) -> int | None: """Temperature when room is occupied.""" - return self._occupied_heating_setpoint + return self.cluster.get("occupied_heating_setpoint") @property def pi_cooling_demand(self) -> int: """Cooling demand.""" - return self._pi_cooling_demand + return self.cluster.get("pi_cooling_demand") @property def pi_heating_demand(self) -> int: """Heating demand.""" - return self._pi_heating_demand + return self.cluster.get("pi_heating_demand") @property def running_mode(self) -> int | None: """Thermostat running mode.""" - return self._running_mode + return self.cluster.get("running_mode") @property def running_state(self) -> int | None: """Thermostat running state, state of heat, cool, fan relays.""" - return self._running_state + return self.cluster.get("running_state") @property def system_mode(self) -> int | None: """System mode.""" - return self._system_mode + return self.cluster.get("system_mode") @property def unoccupied_cooling_setpoint(self) -> int | None: """Temperature when room is not occupied.""" - return self._unoccupied_cooling_setpoint + return self.cluster.get("unoccupied_cooling_setpoint") @property def unoccupied_heating_setpoint(self) -> int | None: """Temperature when room is not occupied.""" - return self._unoccupied_heating_setpoint + return self.cluster.get("unoccupied_heating_setpoint") @callback def attribute_updated(self, attrid, value): @@ -261,7 +245,6 @@ class ThermostatChannel(ZigbeeChannel): self.debug( "Attribute report '%s'[%s] = %s", self.cluster.name, attr_name, value ) - setattr(self, f"_{attr_name}", value) self.async_send_signal( f"{self.unique_id}_{SIGNAL_ATTR_UPDATED}", AttributeUpdateRecord(attrid, attr_name, value), @@ -276,8 +259,6 @@ class ThermostatChannel(ZigbeeChannel): self._init_attrs.pop(attr, None) if attr in fail: continue - if isinstance(attr, str): - setattr(self, f"_{attr}", res[attr]) self.async_send_signal( f"{self.unique_id}_{SIGNAL_ATTR_UPDATED}", AttributeUpdateRecord(None, attr, res[attr]), @@ -301,7 +282,6 @@ class ThermostatChannel(ZigbeeChannel): self.debug("couldn't set '%s' operation mode", mode) return False - self._system_mode = mode self.debug("set system to %s", mode) return True @@ -317,11 +297,6 @@ class ThermostatChannel(ZigbeeChannel): self.debug("couldn't set heating setpoint") return False - if is_away: - self._unoccupied_heating_setpoint = temperature - else: - self._occupied_heating_setpoint = temperature - self.debug("set heating setpoint to %s", temperature) return True async def async_set_cooling_setpoint( @@ -335,10 +310,6 @@ class ThermostatChannel(ZigbeeChannel): if not await self.write_attributes(data): self.debug("couldn't set cooling setpoint") return False - if is_away: - self._unoccupied_cooling_setpoint = temperature - else: - self._occupied_cooling_setpoint = temperature self.debug("set cooling setpoint to %s", temperature) return True @@ -349,7 +320,6 @@ class ThermostatChannel(ZigbeeChannel): self.debug("read 'occupancy' attr, success: %s, fail: %s", res, fail) if "occupancy" not in res: return None - self._occupancy = res["occupancy"] return bool(self.occupancy) except ZigbeeException as ex: self.debug("Couldn't read 'occupancy' attribute: %s", ex) diff --git a/tests/components/zha/common.py b/tests/components/zha/common.py index 0115838c70d..d8889a0208c 100644 --- a/tests/components/zha/common.py +++ b/tests/components/zha/common.py @@ -3,6 +3,7 @@ import asyncio import math from unittest.mock import AsyncMock, Mock +import zigpy.zcl import zigpy.zcl.foundation as zcl_f import homeassistant.components.zha.core.const as zha_const @@ -47,7 +48,8 @@ def patch_cluster(cluster): cluster.read_attributes = AsyncMock(wraps=cluster.read_attributes) cluster.read_attributes_raw = AsyncMock(side_effect=_read_attribute_raw) cluster.unbind = AsyncMock(return_value=[0]) - cluster.write_attributes = AsyncMock( + cluster.write_attributes = AsyncMock(wraps=cluster.write_attributes) + cluster._write_attributes = AsyncMock( return_value=[zcl_f.WriteAttributesResponse.deserialize(b"\x00")[0]] ) if cluster.cluster_id == 4: @@ -76,13 +78,16 @@ def send_attribute_report(hass, cluster, attrid, value): return send_attributes_report(hass, cluster, {attrid: value}) -async def send_attributes_report(hass, cluster: int, attributes: dict): +async def send_attributes_report(hass, cluster: zigpy.zcl.Cluster, attributes: dict): """Cause the sensor to receive an attribute report from the network. This is to simulate the normal device communication that happens when a device is paired to the zigbee network. """ - attrs = [make_attribute(attrid, value) for attrid, value in attributes.items()] + attrs = [ + make_attribute(cluster.attridx.get(attr, attr), value) + for attr, value in attributes.items() + ] hdr = make_zcl_header(zcl_f.Command.Report_Attributes) hdr.frame_control.disable_default_response = True cluster.handle_message(hdr, [attrs]) diff --git a/tests/components/zha/conftest.py b/tests/components/zha/conftest.py index da76b7015c9..fd138567367 100644 --- a/tests/components/zha/conftest.py +++ b/tests/components/zha/conftest.py @@ -1,4 +1,5 @@ """Test configuration for the ZHA component.""" +import itertools import time from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch @@ -116,6 +117,7 @@ def zigpy_device_mock(zigpy_app_controller): node_descriptor=b"\x02@\x807\x10\x7fd\x00\x00*d\x00\x00", nwk=0xB79C, patch_cluster=True, + quirk=None, ): """Make a fake device using the specified cluster classes.""" device = zigpy.device.Device( @@ -133,13 +135,20 @@ def zigpy_device_mock(zigpy_app_controller): endpoint.request = AsyncMock(return_value=[0]) for cluster_id in ep.get(SIG_EP_INPUT, []): - cluster = endpoint.add_input_cluster(cluster_id) - if patch_cluster: - common.patch_cluster(cluster) + endpoint.add_input_cluster(cluster_id) for cluster_id in ep.get(SIG_EP_OUTPUT, []): - cluster = endpoint.add_output_cluster(cluster_id) - if patch_cluster: + endpoint.add_output_cluster(cluster_id) + + if quirk: + device = quirk(zigpy_app_controller, device.ieee, device.nwk, device) + + if patch_cluster: + for endpoint in (ep for epid, ep in device.endpoints.items() if epid): + endpoint.request = AsyncMock(return_value=[0]) + for cluster in itertools.chain( + endpoint.in_clusters.values(), endpoint.out_clusters.values() + ): common.patch_cluster(cluster) return device diff --git a/tests/components/zha/test_climate.py b/tests/components/zha/test_climate.py index f3808fee78d..e452d90d60f 100644 --- a/tests/components/zha/test_climate.py +++ b/tests/components/zha/test_climate.py @@ -3,6 +3,8 @@ from unittest.mock import patch import pytest +import zhaquirks.sinope.thermostat +import zhaquirks.tuya.valve import zigpy.profiles import zigpy.zcl.clusters from zigpy.zcl.clusters.hvac import Thermostat @@ -96,6 +98,12 @@ CLIMATE_SINOPE = { ], SIG_EP_OUTPUT: [zigpy.zcl.clusters.general.Ota.cluster_id, 65281], }, + 196: { + SIG_EP_PROFILE: 0xC25D, + SIG_EP_TYPE: zigpy.profiles.zha.DeviceType.THERMOSTAT, + SIG_EP_INPUT: [zigpy.zcl.clusters.general.PowerConfiguration.cluster_id], + SIG_EP_OUTPUT: [], + }, } CLIMATE_ZEN = { @@ -159,13 +167,13 @@ ZCL_ATTR_PLUG = { def device_climate_mock(hass, zigpy_device_mock, zha_device_joined): """Test regular thermostat device.""" - async def _dev(clusters, plug=None, manuf=None): + async def _dev(clusters, plug=None, manuf=None, quirk=None): if plug is None: plugged_attrs = ZCL_ATTR_PLUG else: plugged_attrs = {**ZCL_ATTR_PLUG, **plug} - zigpy_device = zigpy_device_mock(clusters, manufacturer=manuf) + zigpy_device = zigpy_device_mock(clusters, manufacturer=manuf, quirk=quirk) zigpy_device.endpoints[1].thermostat.PLUGGED_ATTR_READS = plugged_attrs zha_device = await zha_device_joined(zigpy_device) await async_enable_traffic(hass, [zha_device]) @@ -198,7 +206,11 @@ async def device_climate_fan(device_climate_mock): async def device_climate_sinope(device_climate_mock): """Sinope thermostat.""" - return await device_climate_mock(CLIMATE_SINOPE, manuf=MANUF_SINOPE) + return await device_climate_mock( + CLIMATE_SINOPE, + manuf=MANUF_SINOPE, + quirk=zhaquirks.sinope.thermostat.SinopeTechnologiesThermostat, + ) @pytest.fixture @@ -212,7 +224,9 @@ async def device_climate_zen(device_climate_mock): async def device_climate_moes(device_climate_mock): """MOES thermostat.""" - return await device_climate_mock(CLIMATE_MOES, manuf=MANUF_MOES) + return await device_climate_mock( + CLIMATE_MOES, manuf=MANUF_MOES, quirk=zhaquirks.tuya.valve.MoesHY368_Type1 + ) def test_sequence_mappings(): @@ -456,22 +470,18 @@ async def test_target_temperature( ): """Test target temperature property.""" - with patch.object( - zigpy.zcl.clusters.manufacturer_specific.ManufacturerSpecificCluster, - "ep_attribute", - "sinope_manufacturer_specific", - ): - device_climate = await device_climate_mock( - CLIMATE_SINOPE, - { - "occupied_cooling_setpoint": 2500, - "occupied_heating_setpoint": 2200, - "system_mode": sys_mode, - "unoccupied_heating_setpoint": 1600, - "unoccupied_cooling_setpoint": 2700, - }, - manuf=MANUF_SINOPE, - ) + device_climate = await device_climate_mock( + CLIMATE_SINOPE, + { + "occupied_cooling_setpoint": 2500, + "occupied_heating_setpoint": 2200, + "system_mode": sys_mode, + "unoccupied_heating_setpoint": 1600, + "unoccupied_cooling_setpoint": 2700, + }, + manuf=MANUF_SINOPE, + quirk=zhaquirks.sinope.thermostat.SinopeTechnologiesThermostat, + ) entity_id = await find_entity_id(DOMAIN, device_climate, hass) if preset: await hass.services.async_call( @@ -498,20 +508,16 @@ async def test_target_temperature_high( ): """Test target temperature high property.""" - with patch.object( - zigpy.zcl.clusters.manufacturer_specific.ManufacturerSpecificCluster, - "ep_attribute", - "sinope_manufacturer_specific", - ): - device_climate = await device_climate_mock( - CLIMATE_SINOPE, - { - "occupied_cooling_setpoint": 1700, - "system_mode": Thermostat.SystemMode.Auto, - "unoccupied_cooling_setpoint": unoccupied, - }, - manuf=MANUF_SINOPE, - ) + device_climate = await device_climate_mock( + CLIMATE_SINOPE, + { + "occupied_cooling_setpoint": 1700, + "system_mode": Thermostat.SystemMode.Auto, + "unoccupied_cooling_setpoint": unoccupied, + }, + manuf=MANUF_SINOPE, + quirk=zhaquirks.sinope.thermostat.SinopeTechnologiesThermostat, + ) entity_id = await find_entity_id(DOMAIN, device_climate, hass) if preset: await hass.services.async_call( @@ -538,20 +544,16 @@ async def test_target_temperature_low( ): """Test target temperature low property.""" - with patch.object( - zigpy.zcl.clusters.manufacturer_specific.ManufacturerSpecificCluster, - "ep_attribute", - "sinope_manufacturer_specific", - ): - device_climate = await device_climate_mock( - CLIMATE_SINOPE, - { - "occupied_heating_setpoint": 2100, - "system_mode": Thermostat.SystemMode.Auto, - "unoccupied_heating_setpoint": unoccupied, - }, - manuf=MANUF_SINOPE, - ) + device_climate = await device_climate_mock( + CLIMATE_SINOPE, + { + "occupied_heating_setpoint": 2100, + "system_mode": Thermostat.SystemMode.Auto, + "unoccupied_heating_setpoint": unoccupied, + }, + manuf=MANUF_SINOPE, + quirk=zhaquirks.sinope.thermostat.SinopeTechnologiesThermostat, + ) entity_id = await find_entity_id(DOMAIN, device_climate, hass) if preset: await hass.services.async_call( @@ -748,22 +750,18 @@ async def test_set_temperature_hvac_mode(hass, device_climate): async def test_set_temperature_heat_cool(hass, device_climate_mock): """Test setting temperature service call in heating/cooling HVAC mode.""" - with patch.object( - zigpy.zcl.clusters.manufacturer_specific.ManufacturerSpecificCluster, - "ep_attribute", - "sinope_manufacturer_specific", - ): - device_climate = await device_climate_mock( - CLIMATE_SINOPE, - { - "occupied_cooling_setpoint": 2500, - "occupied_heating_setpoint": 2000, - "system_mode": Thermostat.SystemMode.Auto, - "unoccupied_heating_setpoint": 1600, - "unoccupied_cooling_setpoint": 2700, - }, - manuf=MANUF_SINOPE, - ) + device_climate = await device_climate_mock( + CLIMATE_SINOPE, + { + "occupied_cooling_setpoint": 2500, + "occupied_heating_setpoint": 2000, + "system_mode": Thermostat.SystemMode.Auto, + "unoccupied_heating_setpoint": 1600, + "unoccupied_cooling_setpoint": 2700, + }, + manuf=MANUF_SINOPE, + quirk=zhaquirks.sinope.thermostat.SinopeTechnologiesThermostat, + ) entity_id = await find_entity_id(DOMAIN, device_climate, hass) thrm_cluster = device_climate.device.endpoints[1].thermostat @@ -838,22 +836,18 @@ async def test_set_temperature_heat_cool(hass, device_climate_mock): async def test_set_temperature_heat(hass, device_climate_mock): """Test setting temperature service call in heating HVAC mode.""" - with patch.object( - zigpy.zcl.clusters.manufacturer_specific.ManufacturerSpecificCluster, - "ep_attribute", - "sinope_manufacturer_specific", - ): - device_climate = await device_climate_mock( - CLIMATE_SINOPE, - { - "occupied_cooling_setpoint": 2500, - "occupied_heating_setpoint": 2000, - "system_mode": Thermostat.SystemMode.Heat, - "unoccupied_heating_setpoint": 1600, - "unoccupied_cooling_setpoint": 2700, - }, - manuf=MANUF_SINOPE, - ) + device_climate = await device_climate_mock( + CLIMATE_SINOPE, + { + "occupied_cooling_setpoint": 2500, + "occupied_heating_setpoint": 2000, + "system_mode": Thermostat.SystemMode.Heat, + "unoccupied_heating_setpoint": 1600, + "unoccupied_cooling_setpoint": 2700, + }, + manuf=MANUF_SINOPE, + quirk=zhaquirks.sinope.thermostat.SinopeTechnologiesThermostat, + ) entity_id = await find_entity_id(DOMAIN, device_climate, hass) thrm_cluster = device_climate.device.endpoints[1].thermostat @@ -921,22 +915,18 @@ async def test_set_temperature_heat(hass, device_climate_mock): async def test_set_temperature_cool(hass, device_climate_mock): """Test setting temperature service call in cooling HVAC mode.""" - with patch.object( - zigpy.zcl.clusters.manufacturer_specific.ManufacturerSpecificCluster, - "ep_attribute", - "sinope_manufacturer_specific", - ): - device_climate = await device_climate_mock( - CLIMATE_SINOPE, - { - "occupied_cooling_setpoint": 2500, - "occupied_heating_setpoint": 2000, - "system_mode": Thermostat.SystemMode.Cool, - "unoccupied_cooling_setpoint": 1600, - "unoccupied_heating_setpoint": 2700, - }, - manuf=MANUF_SINOPE, - ) + device_climate = await device_climate_mock( + CLIMATE_SINOPE, + { + "occupied_cooling_setpoint": 2500, + "occupied_heating_setpoint": 2000, + "system_mode": Thermostat.SystemMode.Cool, + "unoccupied_cooling_setpoint": 1600, + "unoccupied_heating_setpoint": 2700, + }, + manuf=MANUF_SINOPE, + quirk=zhaquirks.sinope.thermostat.SinopeTechnologiesThermostat, + ) entity_id = await find_entity_id(DOMAIN, device_climate, hass) thrm_cluster = device_climate.device.endpoints[1].thermostat