Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cluster.bind is not async #3704

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions tests/test_danfoss.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from zigpy.zcl.clusters.hvac import Thermostat
from zigpy.zcl.foundation import WriteAttributesStatusRecord, ZCLAttributeDef

from tests.common import wait_for_zigpy_tasks
import zhaquirks
from zhaquirks.danfoss.thermostat import CustomizedStandardCluster

Expand Down Expand Up @@ -46,7 +47,7 @@ def test_popp_signature(assert_signature_matches_quirk):
)


@mock.patch("zigpy.zcl.Cluster.bind", mock.AsyncMock())
@mock.patch("zigpy.zcl.Cluster.bind", mock.Mock())
async def test_danfoss_time_bind(zigpy_device_from_quirk):
"""Test the time being set when binding the Time cluster."""
device = zigpy_device_from_quirk(zhaquirks.danfoss.thermostat.DanfossThermostat)
Expand All @@ -67,7 +68,8 @@ def mock_write(attributes, manufacturer=None):
)

with patch_danfoss_trv_write:
await danfoss_thermostat_cluster.bind()
danfoss_thermostat_cluster.bind()
await wait_for_zigpy_tasks()

assert 0x0000 in danfoss_time_cluster._attr_cache
assert 0x0001 in danfoss_time_cluster._attr_cache
Expand Down
17 changes: 7 additions & 10 deletions tests/test_ikea.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from zigpy.zcl.clusters.general import Basic, PowerConfiguration
from zigpy.zcl.clusters.measurement import PM25

from tests.common import ClusterListener
from tests.common import ClusterListener, wait_for_zigpy_tasks
import zhaquirks
import zhaquirks.ikea.starkvind
from zhaquirks.ikea.starkvind import IkeaAirpurifier
Expand Down Expand Up @@ -159,7 +159,7 @@ def mock_read(attributes, manufacturer=None):
assert not fail


@mock.patch("zigpy.zcl.Cluster.bind", mock.AsyncMock())
@mock.patch("zigpy.zcl.Cluster.bind", mock.Mock())
@pytest.mark.parametrize(
"firmware, pct_device, pct_correct, expected_pct_updates, expect_log_warning",
(
Expand Down Expand Up @@ -204,20 +204,18 @@ def mock_read(attributes, manufacturer=None):
]
return (records,)

p1 = mock.patch.object(power_cluster, "create_catching_task")
p2 = mock.patch.object(
p1 = mock.patch.object(
basic_cluster, "_read_attributes", mock.AsyncMock(side_effect=mock_read)
)

with p1 as mock_task, p2 as request_mock:
with p1 as request_mock:
# update battery percentage with no firmware in attr cache, check pct not doubled for now
power_cluster.update_attribute(battery_pct_id, pct_device)
assert len(power_listener.attribute_updates) == 1
assert power_listener.attribute_updates[0] == (battery_pct_id, pct_device)

# but also check that sw_build_id read is requested in the background for next update
assert mock_task.call_count == 1
await mock_task.call_args[0][0] # await coroutine to read attribute
await wait_for_zigpy_tasks()
assert request_mock.call_count == 1 # verify request to read sw_build_id
assert request_mock.mock_calls[0][1][0][0] == sw_build_id

Expand All @@ -228,7 +226,6 @@ def mock_read(attributes, manufacturer=None):
assert power_listener.attribute_updates[1] == (battery_pct_id, pct_correct)

# reset mocks for testing when sw_build_id is known next
mock_task.reset_mock()
request_mock.reset_mock()
power_listener = ClusterListener(power_cluster)

Expand All @@ -239,11 +236,11 @@ def mock_read(attributes, manufacturer=None):
assert power_listener.attribute_updates[0] == (battery_pct_id, pct_correct)

# check no attribute reads were requested when sw_build_id is known
assert mock_task.call_count == 0
assert request_mock.call_count == 0

# make sure a call to bind() always reads sw_build_id (e.g. on join or to refresh when repaired/reconfigured)
await power_cluster.bind()
power_cluster.bind()
await wait_for_zigpy_tasks()
assert request_mock.call_count == 1
assert request_mock.mock_calls[0][1][0][0] == sw_build_id

Expand Down
9 changes: 4 additions & 5 deletions tests/test_sinope.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from zigpy.zcl.clusters.general import DeviceTemperature
from zigpy.zcl.clusters.measurement import FlowMeasurement

from tests.common import ClusterListener
from tests.common import ClusterListener, wait_for_zigpy_tasks
import zhaquirks
from zhaquirks.const import (
COMMAND_M_INITIAL_PRESS,
Expand Down Expand Up @@ -224,12 +224,12 @@ async def test_sinope_light_switch_reporting(zigpy_device_from_quirk, quirk):
manu_cluster = device.endpoints[1].in_clusters[SINOPE_MANUFACTURER_CLUSTER_ID]

request_patch = mock.patch("zigpy.zcl.Cluster.request", mock.AsyncMock())
bind_patch = mock.patch("zigpy.zcl.Cluster.bind", mock.AsyncMock())

with request_patch as request_mock, bind_patch as bind_mock:
with request_patch as request_mock:
request_mock.return_value = (foundation.Status.SUCCESS, "done")

await manu_cluster.bind()
manu_cluster.bind()
await wait_for_zigpy_tasks()
await manu_cluster.configure_reporting(
SinopeTechnologiesManufacturerCluster.AttributeDefs.action_report.id,
3600,
Expand All @@ -238,7 +238,6 @@ async def test_sinope_light_switch_reporting(zigpy_device_from_quirk, quirk):
)

assert len(request_mock.mock_calls) == 1
assert len(bind_mock.mock_calls) == 1


@pytest.mark.parametrize("quirk", (SinopeTechnologieslight,))
Expand Down
10 changes: 6 additions & 4 deletions tests/test_tuya.py
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ def test_multiple_attributes_report():
assert data.data.datapoints[3].dp == 9


@mock.patch("zigpy.zcl.Cluster.bind", mock.AsyncMock())
@mock.patch("zigpy.zcl.Cluster.bind", mock.Mock())
@pytest.mark.parametrize(
"quirk",
(zhaquirks.tuya.ts0501_fan_switch.TS0501FanSwitch,),
Expand All @@ -1655,7 +1655,8 @@ async def test_fan_switch_writes_attributes(zigpy_device_from_quirk, quirk):
with mock.patch.object(fan_cluster.endpoint, "request", mock.AsyncMock()) as m1:
m1.return_value = (foundation.Status.SUCCESS, "done")

await fan_cluster.bind()
fan_cluster.bind()
await wait_for_zigpy_tasks()

assert len(m1.mock_calls) == 1
assert m1.mock_calls[0].kwargs["cluster"] == 514
Expand Down Expand Up @@ -1693,12 +1694,13 @@ async def test_power_config_no_bind(zigpy_device_from_quirk, quirk):
power_cluster = device.endpoints[1].power

request_patch = mock.patch("zigpy.zcl.Cluster.request", mock.AsyncMock())
bind_patch = mock.patch("zigpy.zcl.Cluster.bind", mock.AsyncMock())
bind_patch = mock.patch("zigpy.zcl.Cluster.bind")

with request_patch as request_mock, bind_patch as bind_mock:
request_mock.return_value = (foundation.Status.SUCCESS, "done")

await power_cluster.bind()
power_cluster.bind()
await wait_for_zigpy_tasks()
await power_cluster.configure_reporting(
PowerConfiguration.attributes_by_name["battery_percentage_remaining"].id,
3600,
Expand Down
20 changes: 8 additions & 12 deletions tests/test_xiaomi.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
from zigpy.zcl.clusters.security import IasZone
from zigpy.zcl.clusters.smartenergy import Metering

from tests.common import ZCL_OCC_ATTR_RPT_OCC, ClusterListener
from tests.common import ZCL_OCC_ATTR_RPT_OCC, ClusterListener, wait_for_zigpy_tasks
import zhaquirks
from zhaquirks.const import (
BUTTON_1,
Expand Down Expand Up @@ -491,35 +491,31 @@ def test_attribute_parsing(raw_report):
assert len(raw_report) == 2 * len(reports[0])


@mock.patch("zigpy.zcl.Cluster.bind", mock.AsyncMock())
@mock.patch("zigpy.zcl.Cluster.bind", mock.Mock())
@pytest.mark.parametrize("quirk", (zhaquirks.xiaomi.aqara.plug_eu.PlugMAEU01,))
async def test_xiaomi_eu_plug_binding(zigpy_device_from_quirk, quirk):
"""Test binding Xiaomi EU plug sets OppleMode to True and removes the plug from group 0."""

device = zigpy_device_from_quirk(quirk)
opple_cluster = device.endpoints[1].opple_cluster

p1 = mock.patch.object(opple_cluster, "create_catching_task")
p2 = mock.patch.object(opple_cluster.endpoint, "request", mock.AsyncMock())

with p1 as mock_task, p2 as request_mock:
with p2 as request_mock:
request_mock.return_value = (foundation.Status.SUCCESS, "done")

await opple_cluster.bind()
opple_cluster.bind()
await wait_for_zigpy_tasks()

# Only removed the plug from group 0 so far
assert len(request_mock.mock_calls) == 1
assert mock_task.call_count == 1
assert len(request_mock.mock_calls) == 2

# Removed the plug from group 0
assert request_mock.mock_calls[0]
assert request_mock.mock_calls[0].kwargs["cluster"] == 4
assert request_mock.mock_calls[0].kwargs["sequence"] == 1
assert request_mock.mock_calls[0].kwargs["data"] == b"\x01\x01\x03\x00\x00"

# Await call writing OppleMode attribute
await mock_task.call_args[0][0]

assert len(request_mock.mock_calls) == 2
# Write the OppleMode attribute
assert request_mock.mock_calls[1].kwargs["cluster"] == 64704
assert request_mock.mock_calls[1].kwargs["sequence"] == 2
assert (
Expand Down
26 changes: 15 additions & 11 deletions zhaquirks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@
_CONSTANT_ATTRIBUTES: dict[int, typing.Any] = {}
_VALID_ATTRIBUTES: set[int] = set()

async def bind(self):
def bind(self):
"""Prevent bind."""
self.debug("binding LocalDataCluster")
return (foundation.Status.SUCCESS,)

async def unbind(self):
def unbind(self):
"""Prevent unbind."""
self.debug("unbinding LocalDataCluster")
return (foundation.Status.SUCCESS,)
Expand Down Expand Up @@ -172,26 +172,30 @@

COORDINATOR_GROUP_ID = 0x30 # Group id with only coordinator as a member

async def bind(self):
def bind(self):
"""Bind cluster to a group."""
# Ensure coordinator is a member of the group
application = self._endpoint.device.application
coordinator = application.get_device(application.state.node_info.ieee)
await coordinator.add_to_group(
self.COORDINATOR_GROUP_ID,
name="Coordinator Group - Created by ZHAQuirks",
self.create_catching_task(

Check warning on line 180 in zhaquirks/__init__.py

View check run for this annotation

Codecov / codecov/patch

zhaquirks/__init__.py#L180

Added line #L180 was not covered by tests
coordinator.add_to_group(
self.COORDINATOR_GROUP_ID,
name="Coordinator Group - Created by ZHAQuirks",
)
)

# Bind cluster to group
dstaddr = zdotypes.MultiAddress()
dstaddr.addrmode = 1
dstaddr.nwk = self.COORDINATOR_GROUP_ID
dstaddr.endpoint = self._endpoint.endpoint_id
return await self._endpoint.device.zdo.Bind_req(
self._endpoint.device.ieee,
self._endpoint.endpoint_id,
self.cluster_id,
dstaddr,
self.create_catching_task(

Check warning on line 192 in zhaquirks/__init__.py

View check run for this annotation

Codecov / codecov/patch

zhaquirks/__init__.py#L192

Added line #L192 was not covered by tests
self._endpoint.device.zdo.Bind_req(
self._endpoint.device.ieee,
self._endpoint.endpoint_id,
self.cluster_id,
dstaddr,
)
)


Expand Down
12 changes: 6 additions & 6 deletions zhaquirks/danfoss/thermostat.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,14 +375,14 @@

return write_res

async def bind(self):
def bind(self):
"""According to the documentation of Zigbee2MQTT there is a bug in the Danfoss firmware with the time.

It doesn't request it, so it has to be fed the correct time.
"""
await self.endpoint.time.write_time()
self.create_catching_task(self.endpoint.time.write_time())

return await super().bind()
return super().bind()


class DanfossUserInterfaceCluster(CustomizedStandardCluster, UserInterface):
Expand Down Expand Up @@ -470,13 +470,13 @@
}
)

async def bind(self):
def bind(self):
"""According to the documentation of Zigbee2MQTT there is a bug in the Danfoss firmware with the time.

It doesn't request it, so it has to be fed the correct time.
"""
result = await super().bind()
await self.write_time()
result = super().bind()
self.create_catching_task(self.write_time())

Check warning on line 479 in zhaquirks/danfoss/thermostat.py

View check run for this annotation

Codecov / codecov/patch

zhaquirks/danfoss/thermostat.py#L478-L479

Added lines #L478 - L479 were not covered by tests
return result


Expand Down
5 changes: 2 additions & 3 deletions zhaquirks/develco/air_quality.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,9 @@
super().__init__(*args, **kwargs)
self.endpoint.device.voc_bus.add_listener(self)

async def bind(self):
def bind(self):
"""Bind cluster."""
result = await self.endpoint.device.app_cluster.bind()
return result
return self.endpoint.device.app_cluster.bind()

Check warning on line 135 in zhaquirks/develco/air_quality.py

View check run for this annotation

Codecov / codecov/patch

zhaquirks/develco/air_quality.py#L135

Added line #L135 was not covered by tests

async def write_attributes(self, attributes, manufacturer=None):
"""Ignore write_attributes."""
Expand Down
8 changes: 5 additions & 3 deletions zhaquirks/ikea/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,12 @@ class DoublingPowerConfigClusterIKEA(CustomCluster, PowerConfiguration):
This implementation doubles battery pct remaining for IKEA devices with old firmware.
"""

async def bind(self):
def bind(self):
"""Bind cluster and read the sw_build_id for later use."""
result = await super().bind()
await self.endpoint.basic.read_attributes([Basic.AttributeDefs.sw_build_id.id])
result = super().bind()
self.create_catching_task(
self.endpoint.basic.read_attributes([Basic.AttributeDefs.sw_build_id.id])
)
return result

def _is_firmware_new(self):
Expand Down
8 changes: 5 additions & 3 deletions zhaquirks/osram/lightifyx4.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,12 @@
0x002F: 0xFFFF,
}

async def bind(self):
def bind(self):
"""Bind cluster."""
result = await super().bind()
await self.write_attributes(self.attr_config, manufacturer=OSRAM_MFG_CODE)
result = super().bind()
self.create_catching_task(

Check warning on line 91 in zhaquirks/osram/lightifyx4.py

View check run for this annotation

Codecov / codecov/patch

zhaquirks/osram/lightifyx4.py#L90-L91

Added lines #L90 - L91 were not covered by tests
self.write_attributes(self.attr_config, manufacturer=OSRAM_MFG_CODE)
)
return result


Expand Down
2 changes: 1 addition & 1 deletion zhaquirks/tuya/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ def update_attribute(self, attr_name: str, value: Any) -> None:
class TuyaNoBindPowerConfigurationCluster(CustomCluster, PowerConfiguration):
"""PowerConfiguration cluster that prevents setting up binding/attribute reports in order to stop battery drain."""

async def bind(self):
def bind(self):
"""Prevent bind."""
return (foundation.Status.SUCCESS,)

Expand Down
6 changes: 3 additions & 3 deletions zhaquirks/tuya/ts0501_fan_switch.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ class FanCluster(CustomCluster, Fan):
Fan.attributes_by_name["fan_mode_sequence"].id: Fan.FanModeSequence.Low_Med_High
}

async def bind(self):
def bind(self):
"""Bind fan cluster and write attributes."""
result = await super().bind()
await self.write_attributes(self.attr_config)
result = super().bind()
self.create_catching_task(self.write_attributes(self.attr_config))
return result


Expand Down
8 changes: 5 additions & 3 deletions zhaquirks/tuya/ts0601_valve.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,14 +255,16 @@
109: "_dp_2_attr_update",
}

async def bind(self):
def bind(self):
"""Bind cluster.

When adding this device tuya gateway issues factory reset,
we just need to reset the frost lock, because its state is unknown to us.
"""
result = await super().bind()
await self.write_attributes({self.attributes_by_name["frost_lock_reset"].id: 0})
result = super().bind()
self.create_catching_task(

Check warning on line 265 in zhaquirks/tuya/ts0601_valve.py

View check run for this annotation

Codecov / codecov/patch

zhaquirks/tuya/ts0601_valve.py#L264-L265

Added lines #L264 - L265 were not covered by tests
self.write_attributes({self.attributes_by_name["frost_lock_reset"].id: 0})
)
return result


Expand Down
Loading
Loading