Port IKEA STARKVIND air purifier to quirks v2 - #5248
Conversation
Replaces the two v1 quirks with a single v2 quirk and moves the entities that ZHA hardcoded for the manufacturer specific cluster into the quirk. The virtual `PM25` cluster the quirk used to add is dropped: PM2.5 is only reported through the `air_quality_25pm` attribute of the manufacturer specific cluster, but adding a server side `PM25` cluster made ZHA bind it and configure reporting for `measured_value` on the device, which answers UNSUPPORTED_ATTRIBUTE. The sensor is now created directly from the manufacturer specific attribute, keeping its unique_id. Fixes zigpy/zha#407
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #5248 +/- ##
==========================================
+ Coverage 92.60% 92.64% +0.03%
==========================================
Files 424 424
Lines 14667 14636 -31
==========================================
- Hits 13582 13559 -23
+ Misses 1085 1077 -8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Verdict: ready — approval left to a maintainer, self-approval is blocked on one's own PR. No blockers, no must-address items; one optional nit below.
I regenerated both committed STARKVIND snapshots in zigpy/zha against this PR's quirk (unmodified ZHA at 438760cc) and diffed every entity. The PR body's central claim holds exactly:
- Zero entities added, zero removed on either model (
ikea-of-sweden-starkvind-air-purifier-0x00011001.json,ikea-of-sweden-starkvind-air-purifier-table-0x00011001.json). - Every
unique_idis byte-identical before and after — including the PM2.5 sensor at{ieee}-1-1066, whichunique_id_suffix=str(PM25.cluster_id)preserves even though the entity now lives on0xFC7D. - The only entity-level deltas are
class_name(PM25→Sensor) andfallback_name(null→PM2.5) on the PM2.5 sensor, plusnative_value0→nullon the table model — which is the #4625 fix landing, since that snapshot hasair_quality_25pm: 65535. - The six duplicated entities keep their ZHA-native classes today, as the body predicts:
Device._add_pending_entitiesdrops a pending entity whose(platform, unique_id)is already claimed, so the quirk's declarations are inert untilzigpy/zha#865lands.
I also checked the quirk's entity metadata against the ZHA-native classes it will take over from once zigpy/zha#865 merges, since that is the point where a mismatch would silently surface. Everything lines up: entity_type defaults land on the right categories (.binary_sensor() → DIAGNOSTIC matches ReplaceFilter, .switch()/.number() → CONFIG match ConfigurableAttributeSwitch/FilterLifeTime), device_class/state_class/translation_key/unit/min_value/max_value all match, and every reporting_config value matches the corresponding entry in IkeaFan._server_cluster_config. Because each entity passes a reporting_config, zhaquirks/builder/discovery.py gives them read_on_startup=False and bind=True, which is exactly what the ZHA-side config does — so the aggregated bind/reporting/startup-read set for 0xFC7D really is unchanged.
Coordination note (not a defect). The release ordering in the body is the load-bearing constraint: zigpy/zha#865 must not ship before a zha-quirks release containing this change, or the six entities disappear for everyone on the new ZHA. Keeping zigpy/zha#865 in draft until then handles it.
Narrow known consequence of dropping signature matching. Matching on manufacturer + model now applies the quirk to firmware variants that previously matched neither v1 signature. Those users gain the seven entities (ZHA's native classes need the named attributes, which a bare 0xFC7D cluster doesn't have, so they got none) and lose the non-functional ZHA Fan entity created from the unimplemented 0x0202 cluster, which .removes(Fan.cluster_id) now deletes for them too. That is the intended cleanup, but it is a unique_id disappearing for that narrow group and worth stating out loud.
Overlapping open PRs — all conflict textually with this file; none is a competing implementation worth blocking on:
- #4499 (bulk v1→v2 migration) also rewrites
zhaquirks/ikea/starkvind.py, but to a pre-2.0CustomDeviceV2shape that keeps the virtualPM25Cluster, theBusplumbing and theread_attributesredirect — i.e. it carries the exact defects this PR removes. Whichever lands second needs a rebase; this PR is the more complete treatment of the file. - #5193 (draft) removes the legacy
Busrouting from this file. This PR deletes theBususage outright, so it subsumes #5193's starkvind hunk. - #4303 and #4309 show
zhaquirks/ikea/starkvind.pyin their file lists only as stale-base noise — their "changes" revert the file to pre-#5113zigpy.quirksimports. Not real overlap.
Verified (11 checks)
- Regenerated both STARKVIND snapshots in a disposable
zigpy/zhaworktree (438760cc) with this PR editable-installed; diffed the full entity list per platform. No adds, no removes, no unique_id change. - Confirmed the six duplicate-suffix entities still resolve to ZHA's own classes today by reading
Device._add_pending_entities(zha/zigbee/device.py:1154) — dedup is on(PLATFORM, unique_id), first-registered wins. - Compared every quirk entity's metadata against
ReplaceFilter,FilterLifeTime,IkeaFilterRunTime,IkeaDeviceRunTime,ChildLock,DisableLedand the ZHAPM25sensor, and against the post- zigpy/zha#865 snapshots in that PR's diff: onlyclass_nameandfallback_namechange there too. - Checked
QuirkBuilderentity-type defaults inzhaquirks/builder/builder.py(.sensor()STANDARD,.binary_sensor()DIAGNOSTIC,.switch()/.number()CONFIG) — all match the classes being replaced. - Traced
reporting_config→AttrConfig(read_on_startup=False)+bind=Trueinzhaquirks/builder/discovery.py:200-235, and matched every interval againstIkeaFan._server_cluster_config. - Confirmed
zigpy/zha#865trimsIkeaFan._server_cluster_configto onlyfan_mode/fan_speedand keepsbind=True, with the quirk covering the remaining seven attributes. - Verified the fan round-trip:
IkeaFan._async_set_fan_modewrites{"fan_mode": ...}by string name (zha/application/platforms/fan/__init__.py:537), which the quirk'swrite_attributesscaling matches;speed_range(1, 10)still lines up with the 10-50 → 2-10 downscale in_update_attribute;write_attributesreturnssuper()'s result directly, sowrite_attributes_safe'sres[0]inspection stays correct. - Confirmed
write_attributesno longer drops sibling attributes whenfan_modeis present — the v1 version returnedsuper().write_attributes({"fan_mode": ...})and silently discarded everything else. The newtest_fan_mode_writecovers exactly that. - Cross-checked the "only
0xFFFFmeans no value" change against Z2M:src/lib/ikea.ts:301-307treatspm25 < 65535as a real (out-of-range) reading and only maps65535to unknown. Matches. tests/test_quirks.py: the two droppedcheck_for_duplicate_cluster_idsallow-list entries referenceIkeaSTARKVIND/IkeaSTARKVIND_v2, which no longer exist — leaving them would be an import-timeNameError. The invariant itself is untouched, and it only ever applied to v1 signature/replacement pairs.- Local run: full
pytest tests/4830 passed / 2 xfailed,pre-commit run --all-filesclean,zhaquirks/ikea/starkvind.pyat 100% line coverage. PR is level withorigin/dev. Copilot (GPT-5.5,--effort high, given both this PR and the coupledzigpy/zha#865diff) returned no findings.
| return await super().write_attributes({"fan_mode": fan_mode}, **kwargs) | ||
| """Scale a written fan mode back up to the device's 10-50 range.""" | ||
| fan_mode_name = self.AttributeDefs.fan_mode.name | ||
| fan_mode = attributes.get(fan_mode_name) |
There was a problem hiding this comment.
Optional: this only handles the str key form, but the signature advertises int | ZCLAttributeDef keys too — a caller writing {IkeaAirpurifier.AttributeDefs.fan_mode: 5} or {0x0006: 5} would skip the scaling and send 5 instead of 25. ZHA only ever writes by name (write_attributes_safe is typed dict[str, Any], and IkeaFan._async_set_fan_mode passes {"fan_mode": ...}), so nothing hits this today, and v1 had the same limitation with if "fan_mode" in attributes. Since the method is being rewritten anyway, normalising the lookup would make it match its own annotation.
There was a problem hiding this comment.
@zigpy-review-bot Create an issue to do this in the future. I believe this also happens in other places. Maybe we should even have something shared to remap attribute writes/reads (and updates?) like this? Think about this before.
There was a problem hiding this comment.
Opened #5283.
You were right that it happens elsewhere — I found four different hand-rolled answers to the same lookup across the repo, three of them incomplete, plus the same gap in the shared base class: LocalDataCluster.write_attributes handles name and id keys but drops a ZCLAttributeDef key with a "not a valid attribute id" error. zhaquirks/bosch/rbsh_trv0_zb_eu.py already grew a file-local get_attribute_id_or_name() helper for exactly this, which is the clearest sign it belongs somewhere shared.
It is also not purely theoretical: zha.set_zigbee_cluster_attribute accepts an attribute id or name and ZHA passes the key through verbatim to cluster.write_attributes(), so the same logical write behaves differently depending on which form the caller used — with an id, a name-keyed override does not fire at all.
On the "something shared to remap" part, the issue splits it into key normalization (small, and probably belongs in zigpy's CustomCluster since the three-form contract is zigpy's), declarative value conversion covering the update direction too (the forward and reverse conversions are currently duplicated across write_attributes and _update_attribute and can drift — this quirk, elko, eurotronic, bosch), and attribute redirection, which I'd argue deserves caution rather than a helper: this PR exists because redirecting reads and writes does not redirect binds and reporting configuration, so a general redirect API would make that pattern easier to reach for while leaving the half that actually breaks unsolved.
Affected sites
- Name only, misses id and definition keys: this quirk,
zhaquirks/elko/smart_super_thermostat.py,zhaquirks/eurotronic/__init__.py,zhaquirks/danfoss/thermostat.py - Name-or-id re-implemented per site, misses definition keys:
zhaquirks/xbee/__init__.py,zhaquirks/xiaomi/aqara/thermostat_agl001.py,zhaquirks/xiaomi/aqara/motion_ac02.py,zhaquirks/tuya/ts1201.py,zhaquirks/plaid/soil.py - File-local helper, also id and name only:
zhaquirks/bosch/rbsh_trv0_zb_eu.py - Correct, normalizes via
find_attribute():zhaquirks/legrand/cable_outlet.py,zhaquirks/sonoff/zbm5.py
Definition-keyed writes already exist in-repo (zhaquirks/ubisys/cover_j1.py, zhaquirks/sonoff/zbm5.py), so the definition form is reachable from quirk-to-quirk calls too.
I left this PR's write_attributes as is — happy to normalize it here if you'd rather not wait for the shared fix.
Proposed change
Ports the IKEA STARKVIND air purifier to quirks v2, drops the virtual
PM25cluster and moves the entities that ZHA hardcoded for the manufacturer specific cluster into the quirk.Why the virtual
PM25cluster has to go. PM2.5 is only exposed through theair_quality_25pmattribute (0x0004) of the IKEA manufacturer specific cluster0xFC7D. To get a PM2.5 sensor, the quirk added a server sidePM25cluster (0x042A) and mirrored the value onto it through aBus, plus aread_attributesoverride that redirectedmeasured_valuereads to the manufacturer cluster. But ZHA doesn't only read that cluster — it also binds it and configures reporting formeasured_value, and those requests go to the real device, which doesn't implement0x042Aas a server cluster and answersUNSUPPORTED_ATTRIBUTE. That is the error users see in the reconfigure dialog (zigpy/zha#407), and it is visible in the committed diagnostics for the table model, where everyPM25attribute except the mirrored one is markedunsupported.The sensor is now created directly from the manufacturer specific attribute instead, so nothing is bound or configured on a cluster the device doesn't have.
Entity changes
No entity is added or removed, and every
unique_idis preserved:unique_id{ieee}-1-1066PM25sensor on the virtual cluster.sensor()onair_quality_25pm{ieee}-1-64637-replace_filterReplaceFilter.binary_sensor(){ieee}-1-64637-filter_life_timeFilterLifeTime.number(){ieee}-1-64637-filter_run_timeIkeaFilterRunTime.sensor(){ieee}-1-64637-device_run_timeIkeaDeviceRunTime.sensor(){ieee}-1-64637-child_lockChildLock.switch(){ieee}-1-64637-disable_ledDisableLed.switch()The fan entity stays in ZHA (quirks v2 has no fan platform) and keeps its
unique_idand its bind/reporting config forfan_modeandfan_speed. Thereporting_configvalues on the quirk entities are copied from ZHA's, so the aggregated binds, reporting intervals and startup reads for0xFC7Dcome out byte-identical to today — only the requests against0x042Adisappear.Behavior changes
unknowninstead of a stale value when the device reports0xFFFF. The device sends0xFFFFwhen the value is unavailable (it needs the fan running for a while). ZHA already maps auint16attribute's0xFFFFtounknowngenerically, so no filtering code is needed in the quirk. Previously the mirror dropped such reports instead of passing them on, so the sensor kept whatever the virtual cluster last held — which is why [BUG] Starkvind PM 2.5 sensor not reading values #4625 sees a PM2.5 sensor stuck at0: the committed diagnostics for the table model hasair_quality_25pm: 65535and a PM2.5 state of0.>= 5500are no longer discarded. Only0xFFFFis treated as "no value", matching Z2M, which labels everything below65535as a real (if out-of-range) reading.0xFC7Cwas present; a firmware variant with any other cluster set got no quirk at all and therefore none of these entities (compare [BUG] Ikea Starkvind air purifier & air purifier table sensors missing #1846).PM25out-cluster is no longer removed. It never created an entity; removing it only hid what the device actually reports.Additional information
Coupled with zigpy/zha#865, which removes the now-duplicated entity classes from ZHA. This PR stands on its own and already fixes zigpy/zha#407 without it: I regenerated the diagnostics against an unmodified ZHA with this quirk installed, and the entity list and every
unique_idcome out unchanged.Device._add_pending_entitiesignores a pending entity whose(platform, unique_id)is already taken, so ZHA's own classes keep precedence for the six duplicated ones, while the PM2.5 sensor switches to the quirk's (ZHA'sPM25class no longer matches, since there is noPM25cluster to match on). The ZHA PR is a draft until azha-quirksrelease containing this change exists, since its CI needs it.Checklist
pre-commitchecks pass / the code has been formatted using Black