Add ELIT Scandinavia IHC Link quirk #3961
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #3961 +/- ##
=======================================
Coverage 91.00% 91.00%
=======================================
Files 328 328
Lines 10656 10656
=======================================
Hits 9698 9698
Misses 958 958 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
TheJulianJES
added
the
needs review
TheJulianJES
added
the
needs reviewer answer
TheJulianJES
changed the title
| async def apply_custom_configuration(self, *args, **kwargs): | ||
| """Apply custom configuration to device. Bind multistate input cluster and configure reporting.""" | ||
| for endpoint in self.endpoints.values(): | ||
| if isinstance(endpoint, ZDO): | ||
| continue | ||
| cluster = endpoint.in_clusters.get(MultistateInput.cluster_id) | ||
| if cluster: | ||
| await cluster.bind() | ||
| await cluster.configure_reporting( | ||
| MultistateInput.AttributeDefs.present_value.id, 0, 0, 0 | ||
| ) | ||
|
|
||
| cluster = endpoint.in_clusters.get(OnOff.cluster_id) | ||
| if cluster: | ||
| await cluster.bind() |
There was a problem hiding this comment.
Is binding and manually configuring reporting really required here?
We already bind the MultistateInput cluster by default and set up reporting for the present_value attribute in ZHA: https://github.com/zigpy/zha/blob/695475a927451524475bdcef90a25177727e2345/zha/zigbee/cluster_handlers/general.py#L383-L392
TheJulianJES
added
waiting for reply
| ( | ||
| QuirkBuilder(ELIT, "IHC Link") | ||
| .device_class(EHCLinkDevice) | ||
| .skip_configuration() |
There was a problem hiding this comment.
Ah, I see you're skipping the ZHA configuration here. I don't think we should do that.
Can you provide the full device signature to see if there are any clusters that perhaps shouldn't be bound (which I'm guessing this is why you've added this)?
There was a problem hiding this comment.
Yes, there are really many endpoints (34 endpoints in worst case) and therefore we have to skip the automatic bindings, otherwise the device will work very slow and in some cases the binding table runs full. For the endpoints with MultistateInput, only this attribute needs binding.
This device enables/disables endpoints when it powered up the first time (is static after joining), depending on what is connected to it wired databusses. So it can have either 32 input endpoints, or 16 inputs + 8 outputs, or 16 outputs.
TheJulianJES
removed
the
waiting for reply
Proposed change
Additional information
Checklist
pre-commitchecks pass / the code has been formatted using Black