Skip to content

Port Inovelli VZM entities from ZHA to quirks v2 - #5122

Merged
TheJulianJES merged 3 commits into
devfrom
zigpy-bot/inovelli-v2-entities
Jul 29, 2026
Merged

Port Inovelli VZM entities from ZHA to quirks v2#5122
TheJulianJES merged 3 commits into
devfrom
zigpy-bot/inovelli-v2-entities

Conversation

@TheJulianJES

@TheJulianJES TheJulianJES commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Not sure of the InovelliQuirkBuilder approach yet. It does compact the actual Inovelli quirks a lot though, since many entities are shared.

Proposed change

Add InovelliQuirkBuilder with one method per Inovelli 0xFC31 config entity and wire the VZM30/31/32/35/36 device quirks to declare the entities that the ZHA library previously hard-coded.

Each entity is created with a unique_id_suffix of "64561-" (64561 == 0xFC31) so it keeps the unique_id ZHA produced for the old ZHA-native entity (which included the cluster id); the internal temperature sensor keeps ZHA's bare cluster-id suffix.

The hard-coded entities must be removed from the ZHA library in the same release that bumps the zha-quirks pin to include this change.

Additional information

Related ZHA PR:

Device diagnostics

TO BE ADDED.

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works
  • Device diagnostics data has been attached

Add InovelliQuirkBuilder with one method per Inovelli 0xFC31 config entity
and wire the VZM30/31/32/35/36 device quirks to declare the entities that
the ZHA library previously hard-coded.

Each entity is created with a unique_id_suffix of "64561-<attribute>"
(64561 == 0xFC31) so it keeps the unique_id ZHA produced for the old
ZHA-native entity (which included the cluster id); the internal temperature
sensor keeps ZHA's bare cluster-id suffix.

The hard-coded entities must be removed from the ZHA library in the same
release that bumps the zha-quirks pin to include this change.
@zigpy-review-bot
zigpy-review-bot force-pushed the zigpy-bot/inovelli-v2-entities branch from 1b4043d to 3d44273 Compare June 25, 2026 20:58
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.56%. Comparing base (e2cafc6) to head (0a323a8).
⚠️ Report is 18 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #5122      +/-   ##
==========================================
+ Coverage   92.40%   92.56%   +0.15%     
==========================================
  Files         416      424       +8     
  Lines       14314    14589     +275     
==========================================
+ Hits        13227    13504     +277     
+ Misses       1087     1085       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TheJulianJES

TheJulianJES commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Other branch to also add entities for Inovelli VZM32-SN mmWave: dev...zigpy-bot/inovelli-vzm32-entities (just the second commit matters for that device – the other is this branch/PR).

It's mostly based on Inovelli's official work:

Annotate the builder helpers and public entity methods with -> Self instead
of a forward-referenced string / no annotation, matching the base QuirkBuilder
and propagating correctly to any subclass.
@zigpy-review-bot zigpy-review-bot added the bot: unclear LLM is unsure about PR state label Jul 14, 2026
@zigpy-review-bot zigpy-review-bot added bot: nearly ready PR is good but needs a few minor tweaks per LLM bot: 2.0 migration done PR is on the ZHA/quirks 2.0.0 API (migrated or written for 2.0) per LLM code quality Improvement to code quality bot: needs maintainer PR does something questionable that needs a maintainer decision per LLM and removed bot: unclear LLM is unsure about PR state labels Jul 23, 2026

@zigpy-review-bot zigpy-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: comment (positive) — reviewed on its merits as if ready, per an explicit request to ignore the draft status. I found no code defects; the port is verifiably complete and unique_id-safe. Leaving this as a comment rather than an approval since the head branch originates from the review bot's own earlier session — the final sign-off (and the coupled-release call with zigpy/zha#802) belongs to a human maintainer.

Summary of what was verified (programmatically, against the ZHA library's native Inovelli classes and the coupled zigpy/zha#802): entity-set parity per model (VZM30 38/38, VZM31 40/40, VZM32 37/37, VZM35 38/38, VZM36 ep1 17/17 + ep2 15/15), metadata parity (min/max, translation keys, entity categories, enum members), unique_id preservation (64561-<attr> suffixes throughout), a byte-identical regen round-trip against zha#802's snapshots, and that the interim state (quirks side released before zha#802) is safe.

Verification details
  1. Entity-set parity per model — computed the native set as {registered 0xFC31 entity classes whose models= filter passes and whose attribute is defined on that model's quirked cluster} and matched it against the quirk declarations: VZM30 38/38, VZM31 40/40, VZM32 37/37, VZM35 38/38, VZM36 ep1 17/17 + ep2 15/15. No native entity is missing from the port, and no ported entity lacks a native counterpart. The per-model differences (no switch_type select outside VZM31, fan variants only on VZM35, no relay_click_in_on_off_mode on VZM32/VZM35, VZM36's reduced per-endpoint sets) all fall out correctly from the models= filters plus each cluster's AttributeDefs.
  2. Metadata parity — every number's min/max matches the native _attr_native_min/max_value (no native class sets step/multiplier/unit/device_class), translation keys match (including on_off_led_modeone_led_mode), entity categories match (config everywhere; both sensors diagnostic), and all seven select/sensor enums reproduce the native member names and values exactly, so HA select options and existing translations carry over unchanged.
  3. unique_id preservation — every entity carries unique_id_suffix="64561-<attr>", and the internal temperature sensor's bare "64561" is correct: the native InovelliInternalTemperature sets no _unique_id_suffix, so its unique_id was {ieee}-{ep}-64561.
  4. Regen round-trip — staged zigpy/zha#802 in a zha worktree with this PR editable-installed and regenerated all six Inovelli snapshots: byte-identical to what #802 commits.
  5. Interim state is safe — regenerating against current zha dev (native entities still present) plus this PR produces no duplicate entities and no unique_id changes: where both exist, the native entity wins the (platform, unique_id) slot and the v2 declaration fills in only the entities natives skipped. So releasing the quirks side first is fine; the only ordering constraint is that zha#802 must not ship before the zha-quirks pin includes this change.

One behavior delta worth stating in the PR body: ZHA-native config entities are skipped when the attribute has no cached value (_is_supported checks cluster.get(attr) is None), while quirks-v2 entities are created unconditionally. E.g. under zha#802 the old inovelli-vzm30-sn.json dump gains 24 config entities. This is effectively a fix — entities no longer vanish after a partial initial attribute read — and a firmware cross-check against Z2M indicates the unconditional creation is safe for exactly this entity set (no dead entities on old firmware):

Firmware-gating cross-check vs Z2M
  • Z2M added per-model firmware gating for Inovelli attributes in Koenkk/zigbee-herdsman-converters#11883 (2026-04, src/lib/inovelli.ts). The complete gate list: attribute-level gates exist only for the VZM31-SNdimmingAlgorithm (P27, fw ≥ 3.05), auxDetectionLevel (P124, fw ≥ 3.05), dumbDetectionLevel (P165, fw ≥ 3.7); older firmware returns UNSUPPORTED_ATTRIBUTE for these (Koenkk/zigbee2mqtt#31616). Enum-value gates, also VZM31-only: switch_type value 3 ("Single-Pole Full Sine Wave") was removed in fw 3.0 and fanControlMode "Toggle" was added in fw 3.0. No gates exist for any VZM30/VZM32/VZM35/VZM36 attribute.
  • This PR declares no entities for the three gated attributes — they're AttributeDefs on the cluster only, same as ZHA-native (which also created no entities for them). So every unconditionally-created entity maps to an attribute Z2M exposes ungated on all firmware versions of that model.
  • The diagnostics dumps corroborate that the sparse attribute caches are dump vintage, not firmware capability: no Inovelli dump records any unsupported_attributes on 0xFC31; the old VZM30 dump lists only 17 of the cluster's attributes (15 cached — the ZHA version at dump time only read that subset), while the newer-firmware VZM30 dump (0x01100100) is even sparser (2 cached), and the VZM31 dump (fw 0x01020212) has 37/44 cached. Z2M's VZM30 outputMode was a converter omission fixed in Jul 2025 (Koenkk/zigbee-herdsman-converters#9596), not a firmware gate.
  • Pre-existing, not a regression from this port: the VZM31 InovelliSwitchType enum keeps Single_Pole_Full_Sine = 0x03, which fw 3.0+ rejects with INVALID_VALUE per Z2M. Carried over verbatim from ZHA-native; ZHA has no firmware-conditional enum-value mechanism, so nothing actionable here.

Tests pass locally (3862 passed), pre-commit is clean on the changed files, and a Copilot (GPT-5.5) second opinion returned no findings. Open PRs #5014 and #4944 (VZM32 entity/command additions) touch the same files and will need rebasing onto the InovelliQuirkBuilder pattern once this lands.

Comment thread tests/test_inovelli.py
@TheJulianJES TheJulianJES added smash This PR is close to be merged soon priority: high This should be addressed or looked at urgently priority: medium This should be addressed or looked at soon labels Jul 23, 2026
…luster

The entity count lock and the `64561-` unique_id suffix check both pass for a
typo'd attribute name: the builder has to pass those names as string literals
(no single Inovelli cluster defines all of them, so `AttributeDefs.x.name`
references aren't practical), so a misspelled name still registers an entity
with an intact suffix and an unchanged count, and only surfaces at runtime as a
broken entity.

Assert instead that every attribute-backed Inovelli entity names an attribute
that actually exists on the cluster the quirk installs on that entity's
endpoint. The cluster is derived from the quirk's own registered zigpy
transforms (`.replaces()` per endpoint, `.replace_cluster_occurrences()` as the
device-wide fallback) rather than a hand-written model -> class table, so the
VZM36's separate light (endpoint 1) and fan (endpoint 2) clusters are each
checked against their own attributes.
@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Behavior when this PR runs without the ZHA-side removal (zigpy/zha#802): benign — no duplicate entities, no errors, and the device presents exactly as it does on current ZHA.

Both entity sources run during discovery: QuirkV2Device.discover_entities yields the ZHA-native registry-matched entities first (via super()), then this PR's v2 entities. ZHA's Device._add_pending_entities then dedupes by (platform, unique_id) with first-wins semantics (zha/zigbee/device.py, the "Ignore entities that already exist" branch) — the later entity is silently on_remove()d. Since every v2 entity here deliberately reproduces the native unique_id (64561-<attr> suffix), each one collides with its native twin and is discarded. The drop is completely silent — no warning or error, not even a DEBUG line.

Consequences worth noting:

Testing this PR as a custom quirk on stock ZHA does not exercise the v2 entities

The native classes shadow them all. The declarations only go live once zigpy/zha#802 removes the library copies.

One asymmetry: quirk entities can appear where hidden native ones didn't

Native config entities gate on _is_supported() (attribute must have a cached value), while quirk-created entities are _attr_always_supported. On a device where a native entity is currently hidden (failed initial read, firmware lacking the attribute), the v2 twin takes over the same unique_id and the entity appears where it previously didn't.

The hazard case is a unique_id mismatch, not a collision

Any v2 declaration whose suffix/endpoint doesn't exactly match its native twin would survive dedup alongside it, producing two visible entities for the same attribute. A snapshot regen against plain dev ZHA (without #802) doubles as a check — the VZM30/31/35 snapshots should come out unchanged.

Net: the coupling requirement is one-directional. Shipping the quirks release first is safe (dormant declarations); only the ZHA-side removal must not ship without the quirks release, as stated in both PR bodies.

@TheJulianJES
TheJulianJES marked this pull request as ready for review July 29, 2026 03:38
Copilot AI review requested due to automatic review settings July 29, 2026 03:38
Comment thread tests/test_inovelli.py

@TheJulianJES TheJulianJES Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll clean this up later. For now, it's good enough that we can merge the quirks and builder 😅

Comment on lines +508 to +509
"output_mode",
InovelliOutputMode,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit for future clean up: kwargs?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ports Inovelli VZM-series entities that were previously created inside the ZHA library into quirks v2, using a shared InovelliQuirkBuilder so the per-device quirk files can declaratively register the same config entities while preserving legacy unique_id behavior.

Changes:

  • Added InovelliQuirkBuilder (zhaquirks/inovelli/builder.py) with one method per Inovelli 0xFC31 config entity, including explicit unique_id_suffix rules to preserve ZHA-native unique IDs.
  • Updated VZM30/31/32/35/36 quirk files to use InovelliQuirkBuilder and declare the formerly hard-coded ZHA entities (including endpoint-specific declarations for VZM36).
  • Added tests/test_inovelli.py to validate entity counts, unique-id suffix conventions, and that declared attribute names exist on the installed Inovelli cluster per endpoint.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
zhaquirks/inovelli/VZM36.py Switches VZM36 to InovelliQuirkBuilder and declares light/fan endpoint entities explicitly.
zhaquirks/inovelli/VZM35SN.py Switches VZM35-SN to InovelliQuirkBuilder and declares the shared config entities.
zhaquirks/inovelli/VZM32SN.py Switches VZM32-SN to InovelliQuirkBuilder and declares the shared config entities.
zhaquirks/inovelli/VZM31SN.py Switches VZM31-SN to InovelliQuirkBuilder and declares the shared config entities.
zhaquirks/inovelli/VZM30SN.py Switches VZM30-SN to InovelliQuirkBuilder and declares the shared config entities.
zhaquirks/inovelli/builder.py Introduces the shared builder and associated enum types; centralizes entity metadata and unique-id preservation logic.
tests/test_inovelli.py Adds regression tests for entity counts, unique-id suffix preservation, and attribute name correctness per endpoint cluster.

Comment thread tests/test_inovelli.py
Comment on lines +22 to +24
if "inovelli" not in str(entry.source.file).lower():
continue
model = str(entry.source.file).split("/")[-1]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll clean up this entire test file next. It's good enough to release an initial version with the ported quirks for HA beta and so we can port #5014.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zigpy-review-bot Create a "Task" issue, assign + tag me there, and mention what should be improved for the Inovelli quirks/tests. Also look through (comments in) the other two recently merged Inovelli PRs. List everything, possibly use /details blocks.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #5218 (type: Task, assigned to you).

It collects the deferred items from this PR, #5014 and #4944: the tests/test_inovelli.py rewrite (with a note on which parts ZHA's snapshots now cover and which don't), the builder cleanups (the kwargs nit, missing -> Self on the VZM32-SN methods, and _inovelli_number() having no way to pass a unit — mmwave_hold_time is exposed with the raw uint32 max), the three 0xFC32 attributes that still have no entity (notably mmwave_target_info_report, without which #4944's report parsing can't be switched on from HA), the target_id int8s change that's still unverified against real hardware, and a section of known limitations for the record.

@TheJulianJES
TheJulianJES merged commit 4fd035e into dev Jul 29, 2026
10 checks passed
@TheJulianJES
TheJulianJES deleted the zigpy-bot/inovelli-v2-entities branch July 29, 2026 03:44
FerrumLogic pushed a commit to FerrumLogic/zha-device-handlers that referenced this pull request Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot: nearly ready PR is good but needs a few minor tweaks per LLM bot: needs maintainer PR does something questionable that needs a maintainer decision per LLM bot: 2.0 migration done PR is on the ZHA/quirks 2.0.0 API (migrated or written for 2.0) per LLM code quality Improvement to code quality priority: high This should be addressed or looked at urgently priority: medium This should be addressed or looked at soon smash This PR is close to be merged soon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants