Inovelli v2 Quirks Update

[codyhackw]

Proposed change

Updating Inovelli Quirks to the v2 formatting, as well as an update to add the leading/training edge parameter for the VZM36, but removing it from the VZM31SN per Inovelli's request (it's still configurable via button press combo at the switch, which is the recommended method).

Additional information

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

[TheJulianJES]

Oooh, awesome that you're doing this!
There are some very small changes to the quirks v2 API, example here: #3280
It'll land with the next HA beta, so should also be in the final 2024.8.0 release.

Also, there were some repo changes, so you probably need to rebase this (or merge dev into your branch).

[TheJulianJES]

This also seems to supersede this PR then:

• Convert Inovelli blue quirks to v2 quirk [currently a test - DO NOT MERGE] #3020

[codyhackw]

Oooh, awesome that you're doing this! There are some very small changes to the quirks v2 API, example here: #3280 It'll land with the next HA beta, so should also be in the final 2024.8.0 release.

Also, there were some repo changes, so you probably need to rebase this (or merge dev into your branch).

Looks like that was just the change from add_to_registry_v2 to QuirkBuilder being used and add_to_registry() called at the end now? If so, should be fixed now. I believe I chose the right option there on merging dev in, but if not, just let me know and I can get that sorted too.

[TheJulianJES]

Yep, that's fine. Pre-commit just complains about some missing docstrings now.
Tests are a bit different with quirks v2, since we no longer have specific classes for the quirks. I'll have to look at the specific test(s), but you should be able to use device = zigpy_device_from_v2_quirk("manuf", "model") to get a device with the custom clusters in a test.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.29%. Comparing base (915ff9b) to head (4b5af81).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #3232      +/-   ##
==========================================
+ Coverage   88.18%   88.29%   +0.10%     
==========================================
  Files         301      301              
  Lines        9412     9498      +86     
==========================================
+ Hits         8300     8386      +86     
  Misses       1112     1112              

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

[dmulcahey]

Just comments missing

[TheJulianJES]

And the custom cluster on endpoint 3 isn't replaced yet (when it exists). But we can't add .replaces() for that endpoint yet, since RemovesMetadata assumes the endpoint actually exists (or zigpy crashes otherwise). That seems like an issue.
I'll post more details later and/or fix the zigpy issue if it's one.

[dmulcahey]

And the custom cluster on endpoint 3 isn't replaced yet (when it exists). But we can't add .replaces() for that endpoint yet, since RemovesMetadata assumes the endpoint actually exists (or zigpy crashes otherwise). That seems like an issue. I'll post more details later and/or fix the zigpy issue if it's one.

Ooo… I wonder if we should have another implementation replace_if_present or if we should update the existing one 🤔

[TheJulianJES]

Ooo… I wonder if we should have another implementation replace_if_present or if we should update the existing one 🤔

Up until seconds ago, I thought we can just update the existing one. It adds RemovesMetadata and AddsMetadata.
If we call removes on a non-existent cluster (on a valid endpoint), nothing will happen already. We ignore that case.
But we error out if we try to remove a cluster on a non-existent endpoint.

To ignore this, we can just check if the endpoint exists:
zigpy/zigpy@dev...TheJulianJES:tjj/quirks_v2_removes_non_exist_endpoint (ignore test line change here)

But the above change is not enough. When AddsMetadata is processed, it'll also fail on the non-existent endpoint.
If we also check there that the endpoint exists, a change like this would work: zigpy/zigpy@dev...TheJulianJES:tjj/quirks_v2_removes_non_exist_endpoint_and_add

Currently thinking about if we want to error out when removes, replaces, or adds is called on a non-existent endpoint. Maybe? Like, removes already completely ignores if a cluster that is to be replaced is missing. It just errors out when the endpoint is missing too.
But with adds (also used by `replaces), I'm not sure. We may want to error out here, as the cluster will be missing if the endpoint is missing (obviously). Silently failing here might not be a good option.
But it's also unlikely to ever do this unintentionally? Like, at least when testing, you'll notice that the cluster you're trying to replace/add is missing.

Maybe we want a separate method after all? Not sure yet...

EDIT: We can use the following for this PR:

• Add new quirks v2 metadata type that allows replacing all instances of a cluster on a device zigpy#1441

[TheJulianJES]

I think we should be able to use replace_cluster_occurrences from zigpy/zigpy#1441 for the custom output clusters on all endpoints.

Haven't tested this, but this might work (if zigpy is new enough on the quirks repo, might need to be bumped?):

Suggested change

	.replaces(InovelliVZM31SNCluster, endpoint_id=2, cluster_type=ClusterType.Client)
	.replace_cluster_occurrences(InovelliVZM31SNCluster, cluster_type=ClusterType.Client)