quirk v2 entities are force-creates even if they are not supported

[ikruglov]

Hey,

While working on zigpy/zha-device-handlers#3568, I've noticed that HA/ZHA creates select entities even if a device doesn't support them due to differences in firmware, for instance.

After some debugging, the problem is in this if condition:

zha/zha/application/platforms/select.py

Line 190 in 6737b36

cls._attribute_name in cluster_handler.cluster.unsupported_attributes

The ENTITY_METADATA not in kwargs part is not true for entities from quirks (v2, at least).

For more details, please see the discussion in zigpy/zha-device-handlers#3568

[dmulcahey]

Why not have separate quirks for each FW version and use filters so they only apply to the correct firmware? I think we should get away from a quirk specifying functionality only to have it vanish because of an unsupported attribute and we also shouldn't forcibly expose entities that aren't actually supported IMO. @TheJulianJES @puddly thoughts?

[TheJulianJES]

Hmm, I'll take another look again. I don't think we can use filter if the signature (clusters + endpoints) is the same and only the sw_build_id differs.

[dmulcahey]

Any arbitrary callable can be used iirc… don’t even need to look at the signature in V2

[TheJulianJES]

Yeah, but we can't wait to read attributes at that stage, right?

[dmulcahey]

Yeah, but we can't wait to read attributes at that stage, right?

There is no reason we can’t… the ergonomics of the API should be enhanced to support it though… rn filters are sync. We already read model and manufacturer https://github.com/zigpy/zigpy/blob/95baa8b07547d34c733936b946891990e5695145/zigpy/endpoint.py#L195 before we apply quirks… maybe we should also read sw_build_id from basic at the same time?

Either way… we can read this early or make the filter spec async… thoughts?

[ikruglov]

Is there a consensus on what the right approach is?

[ikruglov]

@dmulcahey @TheJulianJES can we please conclude here?

Should I write a filter() in my quirk that reads sw_build_id similarly to how get_model_info() does it?

[TheJulianJES]

After #365 is merged, we may want to revisit this and consider if we want to include an option in v2 quirks that only enables entity creation if the attribute is (1) readable/cached and/or (2) not marked as unsupported.