Skip to content

Commit 70d15a5

Browse files
authored
Make update_attribute manufacturer code aware (zigpy#1760)
* Make `update_attribute` manufacturer code aware * Add test for manufacturer code `update_attribute` handling * Remove legacy cache fallback in `_legacy_apply_quirk_attribute_update` * Adjust comments * Use `mock_attribute_report` in test * Mark attribute as unsupported initially
1 parent 0ceeb47 commit 70d15a5

File tree

2 files changed

+143
-13
lines changed

2 files changed

+143
-13
lines changed

tests/test_zcl.py

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2490,3 +2490,101 @@ class ServerCommandDefs(Basic.ServerCommandDefs):
24902490
(TestCluster.ServerCommandDefs.reset_fact_default, None),
24912491
]:
24922492
assert cluster._get_effective_manufacturer_code(definition) is expected
2493+
2494+
2495+
async def test_quirk_manufacturer_code_context_isolation(app_mock) -> None:
2496+
"""Test that manufacturer code context is properly handled in _update_attribute.
2497+
2498+
When a manufacturer-specific attribute is reported and the cluster has multiple
2499+
attributes sharing the same ID (with different manufacturer codes), the
2500+
_update_attribute call must use the correct manufacturer code. This tests that:
2501+
1. The value is stored directly in the typed cache (not via legacy cache fallback)
2502+
2. Other attributes updated by quirks don't inherit the manufacturer code context
2503+
"""
2504+
2505+
class TestCluster(zcl.Cluster):
2506+
cluster_id = 0xABCD
2507+
ep_attribute = "test_cluster"
2508+
_skip_registry = True
2509+
2510+
class AttributeDefs(zcl.foundation.BaseAttributeDefs):
2511+
# Two attributes sharing the same ID with different manufacturer codes
2512+
manuf_attr = foundation.ZCLAttributeDef(
2513+
id=0x0001,
2514+
type=t.uint8_t,
2515+
manufacturer_code=0x1234,
2516+
)
2517+
standard_attr = foundation.ZCLAttributeDef(
2518+
id=0x0001,
2519+
type=t.uint8_t,
2520+
manufacturer_code=None,
2521+
)
2522+
# A different attribute that the quirk will also update
2523+
other_attr = foundation.ZCLAttributeDef(
2524+
id=0x0002,
2525+
type=t.uint8_t,
2526+
)
2527+
2528+
def _update_attribute(self, attrid, value):
2529+
super()._update_attribute(attrid, value)
2530+
2531+
# When updating the manufacturer-specific attribute, also update other_attr
2532+
if attrid == self.AttributeDefs.manuf_attr.id:
2533+
super()._update_attribute(self.AttributeDefs.other_attr.id, 99)
2534+
2535+
dev = add_initialized_device(app_mock, nwk=0x1234, ieee=make_ieee(1))
2536+
cluster = TestCluster(dev.endpoints[1])
2537+
dev.endpoints[1].add_input_cluster(TestCluster.cluster_id, cluster)
2538+
2539+
events = []
2540+
cluster.on_event(AttributeReportedEvent.event_type, events.append)
2541+
cluster.on_event(AttributeUpdatedEvent.event_type, events.append)
2542+
2543+
# The attribute is currently marked as unsupported
2544+
cluster.add_unsupported_attribute(TestCluster.AttributeDefs.manuf_attr)
2545+
2546+
# Report the manufacturer-specific attribute
2547+
await mock_attribute_report(
2548+
cluster, {TestCluster.AttributeDefs.manuf_attr: t.uint8_t(42)}
2549+
)
2550+
2551+
# The legacy cache should not contain the attribute, as the typed cache was used
2552+
assert 0x0001 not in cluster._attr_cache._legacy_cache
2553+
2554+
# Verify that the manufacturer-specific attribute was stored correctly
2555+
assert cluster._attr_cache.get_value(TestCluster.AttributeDefs.manuf_attr) == 42
2556+
2557+
# Verify that the standard attribute (same ID, no manufacturer code) was NOT updated
2558+
with pytest.raises(KeyError):
2559+
cluster._attr_cache.get_value(TestCluster.AttributeDefs.standard_attr)
2560+
2561+
# Verify that other_attr was updated (by the quirk) without manufacturer code context
2562+
assert cluster._attr_cache.get_value(TestCluster.AttributeDefs.other_attr) == 99
2563+
2564+
# Verify the events have the correct manufacturer codes
2565+
assert len(events) == 2
2566+
2567+
# First event: other_attr updated by quirk (should have no manufacturer code)
2568+
assert events[0] == AttributeUpdatedEvent(
2569+
device_ieee=str(dev.ieee),
2570+
endpoint_id=1,
2571+
cluster_type=zcl.ClusterType.Server,
2572+
cluster_id=TestCluster.cluster_id,
2573+
attribute_name="other_attr",
2574+
attribute_id=TestCluster.AttributeDefs.other_attr.id,
2575+
manufacturer_code=None,
2576+
value=99,
2577+
)
2578+
2579+
# Second event: manuf_attr reported (should have the manufacturer code)
2580+
assert events[1] == AttributeReportedEvent(
2581+
device_ieee=str(dev.ieee),
2582+
endpoint_id=1,
2583+
cluster_type=zcl.ClusterType.Server,
2584+
cluster_id=TestCluster.cluster_id,
2585+
attribute_name="manuf_attr",
2586+
attribute_id=TestCluster.AttributeDefs.manuf_attr.id,
2587+
manufacturer_code=0x1234,
2588+
raw_value=42,
2589+
value=42,
2590+
)

zigpy/zcl/__init__.py

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@
3838
"_suppressed_attribute_updates", default=frozenset()
3939
)
4040

41+
# Tracks the (attribute_id, manufacturer_code) for the current attribute update operation.
42+
# Used to preserve manufacturer code context when calling _update_attribute,
43+
# so that manufacturer-specific attributes with conflicting IDs are stored correctly.
44+
# The manufacturer code is only applied when the attribute ID matches the original.
45+
_attribute_update_context: ContextVar[tuple[int, int | None] | None] = ContextVar(
46+
"_attribute_update_context", default=None
47+
)
48+
4149

4250
@contextlib.contextmanager
4351
def _suppress_attribute_update_event(
@@ -53,6 +61,25 @@ def _suppress_attribute_update_event(
5361
_suppressed_attribute_updates.reset(token)
5462

5563

64+
@contextlib.contextmanager
65+
def _set_attribute_update_context(
66+
attrid: int,
67+
manufacturer_code: int | None,
68+
) -> Generator[None, None, None]:
69+
"""Set the attribute update context for preserving manufacturer code.
70+
71+
This allows quirks that call _update_attribute to preserve the manufacturer code,
72+
ensuring manufacturer-specific attributes are stored with the correct cache key.
73+
The manufacturer code is only applied when the attribute ID matches.
74+
"""
75+
token = _attribute_update_context.set((attrid, manufacturer_code))
76+
77+
try:
78+
yield
79+
finally:
80+
_attribute_update_context.reset(token)
81+
82+
5683
class ClusterType(enum.IntEnum):
5784
Server = 0
5885
Client = 1
@@ -443,23 +470,19 @@ def _legacy_apply_quirk_attribute_update(
443470
444471
Returns None if the quirk swallowed the attribute (no super() call).
445472
"""
446-
with _suppress_attribute_update_event(self.cluster_id, attr_def.id):
473+
manufacturer_code = self._get_effective_manufacturer_code(attr_def)
474+
475+
with (
476+
_suppress_attribute_update_event(self.cluster_id, attr_def.id),
477+
_set_attribute_update_context(attr_def.id, manufacturer_code),
478+
):
447479
self._update_attribute(attr_def.id, value)
448480

449481
try:
450482
return self._attr_cache.get_value(attr_def)
451483
except KeyError:
452-
pass
453-
454-
# When multiple attrs share an ID (different manufacturer codes),
455-
# `_update_attribute` stores in legacy cache. Move it to typed cache.
456-
if attr_def.id in self._attr_cache._legacy_cache:
457-
cached_value = self._attr_cache._legacy_cache.pop(attr_def.id).value
458-
self._attr_cache.set_value(attr_def, cached_value)
459-
return cached_value
460-
461-
# Quirk swallowed the attribute
462-
return None
484+
# Quirk swallowed the attribute
485+
return None
463486

464487
@classmethod
465488
def find_attributes(
@@ -1138,8 +1161,17 @@ def _update_attribute(
11381161
# other clusters or attributes to emit their own events.
11391162
suppressed = (self.cluster_id, attrid) in _suppressed_attribute_updates.get()
11401163

1164+
# Get the manufacturer code from context if set (set by
1165+
# _legacy_apply_quirk_attribute_update to preserve manufacturer code).
1166+
# Only apply when the attribute ID matches the original to avoid affecting
1167+
# other attributes that quirks may update.
1168+
ctx = _attribute_update_context.get()
1169+
11411170
try:
1142-
attr_def = self.find_attribute(attrid)
1171+
if ctx is not None and ctx[0] == attrid:
1172+
attr_def = self.find_attribute(attrid, manufacturer_code=ctx[1])
1173+
else:
1174+
attr_def = self.find_attribute(attrid)
11431175
except KeyError:
11441176
if value is not None:
11451177
self._attr_cache.set_legacy_value(attrid, value)

0 commit comments

Comments
 (0)