Skip to content

Add entities for Inovelli VZM32-SN mmWave dimmer - #5014

Merged
TheJulianJES merged 13 commits into
zigpy:devfrom
InovelliUSA:inovelli-vzm32-w-entities
Jul 29, 2026
Merged

Add entities for Inovelli VZM32-SN mmWave dimmer#5014
TheJulianJES merged 13 commits into
zigpy:devfrom
InovelliUSA:inovelli-vzm32-w-entities

Conversation

@InovelliUSA

Copy link
Copy Markdown
Contributor

Proposed change

Adding explicit entity declarations for Inovelli mmWave dimmer. This will allow easier configuration and automation of this device.

Additional information

This PR should make this one zigpy/zha#597 obsolete so I am closing it.

Device diagnostics

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

@codecov

codecov Bot commented May 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.58%. Comparing base (4fd035e) to head (fbc9ece).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #5014      +/-   ##
==========================================
+ Coverage   92.56%   92.58%   +0.02%     
==========================================
  Files         424      424              
  Lines       14589    14642      +53     
==========================================
+ Hits        13504    13557      +53     
  Misses       1085     1085              

☔ 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 added priority: medium This should be addressed or looked at soon manufacturer This request was made by the device's manufacturer labels May 19, 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.

Thanks for the PR. After cross-referencing with the closed zha PR #597, a few things need attention before this is mergeable.

Blocker 1 — duplicate entities for existing VZM32-SN users.
ZHA core already creates ~25+ native config entities for attributes on CLUSTER_HANDLER_INOVELLI (the inovelli_vzm31sn_cluster cluster_handler shared by all Inovelli models) — InovelliButtonDelay, InovelliMinimumLoadDimmingLevel, InovelliAutoShutoffTimer, InovelliLocalDefaultLevel, the dimming-speed / ramp-rate sets, InovelliSmartBulbMode, the double-tap level/enable pair, InovelliLocalProtection/RemoteProtection, InovelliOutputMode, etc. (see zha/application/platforms/{number/__init__.py,switch.py}). ZHA discovery does not auto-suppress native entities when a v2 quirk declares one for the same cluster+attribute — only prevent_default_entity_creation() does that.

As written, every existing VZM32-SN user will get duplicate entities on upgrade: the existing native one (unique_id {ieee}-1-64561-{attr}) plus a new v2 one ({ieee}-1-{attr}). Two ways to resolve:

  • Migration approach: keep these declarations and set unique_id_suffix=f"{InovelliVZM32SNCluster.cluster_id}-{attr_name}" on each so the v2 entity adopts the existing native unique_id, plus a coordinated zha-core change adding models= filters that exclude VZM32-SN from the native classes. Existing entities continue working.
  • Scope-down approach: drop the 30+ declarations that duplicate native ZHA entities and keep this PR focused on what was new in zha #597 — the mmWave dimensional numbers, the enum selects, and switch_type. Less invasive, no migration burden.

The scope-down approach is probably the cleaner path since zha #597's design already established what's actually new beyond what ZHA core handles. Happy to discuss either.

Blocker 2 — relay_click_in_on_off_mode doesn't exist on VZM32.
The .switch("relay_click_in_on_off_mode", ...) references an attribute not defined on InovelliVZM32SNCluster (or its base InovelliCluster). The VZM30 and VZM31 clusters define it; VZM32 doesn't. Z2M's VZM32_ATTRIBUTES also omits relayClick — only VZM31_ATTRIBUTES includes it. Please remove this .switch(...).

Blocker 3 — attribute_name should reference the AttributeDef, not a string literal.
Per CLAUDE.md "Code Style", use InovelliVZM32SNCluster.AttributeDefs.foo.name rather than "foo". Recent quirks like zhaquirks/ubisys/trv_h1.py and zhaquirks/ikea/vindstyrka.py follow this. Beyond consistency, it acts as a typo guard at import time — InovelliVZM32SNCluster.AttributeDefs.relay_click_in_on_off_mode.name would have raised AttributeError and surfaced Blocker 2 immediately, instead of producing an entity that silently fails to talk to the device. Same goes for the cluster IDs — replace VZM32SN_CLUSTER_ID = 0xFC31 / MMWAVE_CLUSTER_ID = 0xFC32 with InovelliVZM32SNCluster.cluster_id / InovelliVZM32SNMMWaveCluster.cluster_id at the call sites and drop the module-level constants.

Worth discussing — number vs select for enumerated attrs.
zha #597 modeled these as select entities, exposing named labels: mmwave_detect_sensitivity (Low/Medium/High), mmwave_detect_trigger (Slow 5s/Medium 1s/Fast 0.2s), mmwave_room_size_preset (Custom/X-Small/Small/Medium/Large/X-Large), light_on_presence_behavior (7 named modes). This PR uses .number() with raw 0..N. Both are valid — .number() is simpler and works fine; .enum() (translates to a select in HA) gives users a friendlier dropdown. No strong opinion from me, but worth a deliberate choice. Also: switch_type (Single Pole / 3-Way Aux) is in zha #597 but missing here entirely — looks like an oversight.

Sentence-case fallback_names.
Per CLAUDE.md, sentence case with abbreviations preserved: mmWave height minimum (floor) (and the 5 sibling mmWave entries), LED scaling mode (not "Led").

Diagnostics — please attach a fresh dump.
There's no tests/data/devices/inovelli-vzm32-sn.json snapshot in the ZHA repo yet, so this device has zero CI coverage. Please attach a diagnostics download from a VZM32-SN with this quirk applied — ideally one taken after removing the device from ZHA and re-pairing, or hitting "Reconfigure" on the HA device page, so all attribute values are freshly read from the device (cached stale values from a long-running install muddy the picture). Drop the JSON into the PR body. That gives CI the basis to verify the entity set this PR produces.

Optional cleanup.

  • entity_type=EntityType.CONFIG is already the default for .number() and .switch().
  • off_value=0, on_value=1 are already the defaults.
  • Adding unit=... (cm for dimensional attrs, seconds for mmwave_hold_time) improves UX.

FYI — related open PR.
#4944 also touches InovelliVZM32SNMMWaveCluster (adds the mmWave area commands/reports). Worth coordinating so both can land cleanly.

Comment thread zhaquirks/inovelli/VZM32SN.py Outdated

# Cluster IDs
VZM32SN_CLUSTER_ID = 0xFC31
MMWAVE_CLUSTER_ID = 0xFC32

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.

Magic numbers — use InovelliVZM32SNCluster.cluster_id / InovelliVZM32SNMMWaveCluster.cluster_id at the call sites per CLAUDE.md and drop these constants.

Comment thread zhaquirks/inovelli/VZM32SN.py Outdated
.device_automation_triggers(INOVELLI_AUTOMATION_TRIGGERS)
# Number entities for VZM32SN cluster
.number(
"dimming_speed_up_local",

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.

Per CLAUDE.md, attribute_name should reference the AttributeDef: InovelliVZM32SNCluster.AttributeDefs.dimming_speed_up_local.name (and the same pattern throughout). Catches typos at import time — would have caught the missing relay_click_in_on_off_mode immediately.

Comment thread zhaquirks/inovelli/VZM32SN.py Outdated
fallback_name="Firmware progress LED",
)
.switch(
"relay_click_in_on_off_mode",

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.

This attribute isn't defined on InovelliVZM32SNCluster (or its base InovelliCluster). Z2M's VZM32_ATTRIBUTES also omits relayClick, so VZM32 likely doesn't expose it. Please remove this .switch(...).

Comment thread zhaquirks/inovelli/VZM32SN.py Outdated
fallback_name="Non neutral output",
)
.switch(
"led_scaling_mode",

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.

"LED scaling mode" — abbreviations stay uppercase in sentence case.

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.

This landed on the wrong line: It's about the fallback_name, which we use as the default US English translation in Home Assistant.

Comment thread zhaquirks/inovelli/VZM32SN.py Outdated
)
# MMWave cluster entities
.number(
"mmwave_height_minimum_floor",

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.

Sentence-case fallback name: mmWave height minimum (floor). Same on the five sibling mmWave dimensional entries (ceiling/left/right/near/far).

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.

Same as above: This landed on the wrong line: It's about the fallback_name, which we use as the default US English translation in Home Assistant.

@TheJulianJES TheJulianJES 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.

Thanks for the PR! Sorry that this has been dragging on for so long. The inovelli devices are in a bit off a weird spot right now with the entities still being defined in ZHA.

I should have all the devices, so I'll try to grab diagnostics dumps for each device to add it to the ZHA snapshot/diagnostics database, then migrate the entities to v2 quirks with the unique_id_suffix as mentioned above, remove them from ZHA, verify all entities are "still the same" in HA (unique ID the same), and then come back to this.

The review above is very much valid, but you can somewhat ignore it for now.

@InovelliUSA

Copy link
Copy Markdown
Contributor Author

Thanks for the PR! Sorry that this has been dragging on for so long. The inovelli devices are in a bit off a weird spot right now with the entities still being defined in ZHA.

I should have all the devices, so I'll try to grab diagnostics dumps for each device to add it to the ZHA snapshot/diagnostics database, then migrate the entities to v2 quirks with the unique_id_suffix as mentioned above, remove them from ZHA, verify all entities are "still the same" in HA (unique ID the same), and then come back to this.

The review above is very much valid, but you can somewhat ignore it for now.

Thanks @TheJulianJES, I made most of the adjustments mentioned, but would appreciate the help on preventing the duplicate entities.

zigpy-review-bot added a commit that referenced this pull request Jun 25, 2026
Build on the base Inovelli v2 entity port: add the VZM32-SN-specific entities
that the ZHA library never exposed, ported from the device-specific work in
PR #5014 but using InovelliQuirkBuilder and consistent unique_id suffixes.

On the Inovelli cluster (0xFC31): remote protection switch, VZM32 switch type,
light-on-presence behavior, and mmWave room size preset. On the mmWave cluster
(0xFC32, suffix 64562-<attribute>): detection sensitivity, target speed, hold
time, and the height/width/depth detection-zone bounds.

Unlike #5014, these reuse the base builder so the shared config entities are not
re-declared, and every entity gets a <cluster_id>-<attribute> unique_id_suffix.
@TheJulianJES

Copy link
Copy Markdown
Collaborator

Just to post an update – nothing to do on your side. With the following PR, we can simplify this and keep backwards-compability whilst porting over the existing entities to quirks v2:

When that's merged, I'll push some commits to this PR, so it uses the new system (basically the second commit from here: dev...zigpy-bot/inovelli-vzm32-entities).

I have all the devices, so I'll do a quick test then. Everything should go in for HA Core 2026.8.0.
I'm sorry for the long wait here. It was a bit awkward with the ZHA/quirks split and with a lot of big refactors happening in ZHA. But after the linked PR (and this one), we should finally be good to go and can move way quicker for new entities or devices.

@zigpy-review-bot zigpy-review-bot added bot: needs changes PR needs changes per LLM bot: 2.0 migration needed PR needs ZHA/quirks 2.0.0 migration per LLM enhancement Improve an existing quirk bot: needs maintainer PR does something questionable that needs a maintainer decision per LLM labels Jul 16, 2026
Build on the base Inovelli v2 entity port: add the VZM32-SN-specific entities
that the ZHA library never exposed, ported from the device-specific work in
PR zigpy#5014 but using InovelliQuirkBuilder and consistent unique_id suffixes.

On the Inovelli cluster (0xFC31): remote protection switch, VZM32 switch type,
light-on-presence behavior, and mmWave room size preset. On the mmWave cluster
(0xFC32, suffix 64562-<attribute>): detection sensitivity, target speed, hold
time, and the height/width/depth detection-zone bounds.

Unlike zigpy#5014, these reuse the base builder so the shared config entities are not
re-declared, and every entity gets a <cluster_id>-<attribute> unique_id_suffix.
@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Following up on the comment above#5122 is merged, so this PR has now been updated on TheJulianJES's behalf: dev was merged in and the device-specific entities were ported to the new InovelliQuirkBuilder system.

On the Inovelli cluster (0xFC31): remote protection, VZM32 switch type, light-on-presence behavior, and mmWave room size preset. On the mmWave cluster (0xFC32, 64562-<attribute> unique_id suffixes): detection sensitivity, trigger speed, hold time, and the height/width/depth detection-zone bounds.

A ZHA diagnostics regen confirms the change is purely additive — 13 new entities, no existing entities or unique_ids affected.

Note: since the quirks 2.0.0 refactor (#5113), this quirk won't work when installed as a custom quirk on HA versions before 2026.7.0.

@zigpy-review-bot zigpy-review-bot changed the title Update Inovelli VZM32-SN with Device Specific Entities Add entities for Inovelli VZM32-SN mmWave dimmer Jul 29, 2026
Comment thread tests/test_inovelli.py

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.

Similar to what I mentioned in #5122, this test will be cleaned up in a later PR. (or mostly removed since ZHA snapshot tests basically already test this?)

@TheJulianJES TheJulianJES 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.

Thanks! We should (hopefully) be able to move much faster with future Inovelli (config) entities now 😄

@TheJulianJES
TheJulianJES merged commit d657eb0 into zigpy:dev Jul 29, 2026
9 checks passed
FerrumLogic pushed a commit to FerrumLogic/zha-device-handlers that referenced this pull request Jul 29, 2026
Co-authored-by: William Devereux <william.devereux@outlook.com>
Co-authored-by: TheJulianJES <TheJulianJES@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot: needs changes PR needs changes per LLM bot: needs maintainer PR does something questionable that needs a maintainer decision per LLM bot: 2.0 migration needed PR needs ZHA/quirks 2.0.0 migration per LLM enhancement Improve an existing quirk manufacturer This request was made by the device's manufacturer priority: medium This should be addressed or looked at soon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants