Fix IasZone binary sensor not implementing recompute_capabilities

[TheJulianJES]

Proposed change

This PR fixes an issue where the IasZone binary sensor did not get its device class and translation key assigned.

This is done by correctly implementing recompute_capabilities for the IASZone class. Before, it tried to directly access attribute cache during initialization, at which point it would not have been read for real devices.

Additional information

• Related PR (which "caused" the issue): [RFC] Explicit entity object discovery, lifetimes, and registration #365
• Fixes [ZHA] IasZone binary sensor created before attributes read backlog#50
• Related code (showing order of init + read + recompute): zha/zigbee/device.py#L956-L975

TODO:

• see if we can modify our diagnostics tests for this OR add a specific unit test for just this case?
• check if other entities were also overlooked
• fix quirks prevent_default_entity_creation for these devices, relying on directly accessing entity.translation_key:
  • zhaquirks/develco/smart_siren.py#L20
  • zhaquirks/develco/intelligent_keypad.py#L15

-> see comment for TODOs: #545 (comment)

[TheJulianJES]

The test failure is "expected" for now, but reveals a (bigger) underlying issue:

The linked Frient quirks prevent entity creation by directly accessing translation_key in _discover_new_entities, which isn't optimal in the first place. However, this will not have been set for IasZone entities, as this is only done in recompute_capabilities, after the attributes have been read.

For real world devices, this likely didn't cause any issues, as no known zone_type is set, meaning we get the default "ias_zone" translation key after a HA restart. Before a restart, so during pairing, we also get that translation key set to "ias_zone", but only because the attribute would not have been read at that point, yet.

Now, when _is_entity_removed_by_quirk is called in _discover_new_entities, translation_key is not set at all yet.
Also, it looks like other platforms call recompute_capabilities after initialization. This is probably something we should also do (is there a reason this isn't done for every platform by default?), but it would hide the underlying issue with quirks again.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.94%. Comparing base (b172549) to head (52544c6).
⚠️ Report is 7 commits behind head on dev.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev     #545   +/-   ##
=======================================
  Coverage   96.94%   96.94%           
=======================================
  Files          63       63           
  Lines       10518    10518           
=======================================
  Hits        10197    10197           
  Misses        321      321           

☔ View full report in Codecov by Sentry.
📢 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.

[puddly]

This method call is a bit weird in ZHA in general. self.recompute_capabilities() should be called as part of creating the instance but we don't have a way to do that in the base class.

[TheJulianJES]

Yeah, I noticed 😄

[TheJulianJES]

I think we can merge it as is for now to already get the actual bugfix in the beta. For the TODOs:

1. see if we can modify our diagnostics tests for this OR add a specific unit test for just this case?
2. check if other entities were also overlooked
3. fix quirks prevent_default_entity_creation for these devices, relying on directly accessing entity.translation_key:

1. I've (briefly) looked at this last week or so, and modifying diagnostics to only populate attributes after they were read once is something that should work, but my initial implementation broke a lot of other devices. I think this is something we should investigate at a later day though.
   For just adding a unit test for this, it's something I can do, but I'm not sure I really see the point in just checking that for IAS zone alone, since it'd basically only check that the stuff that's in recompute_capabilities stays in recompute_capabilities. I don't think we have explicit tests for the other entities that do this. We should instead try to ensure this works for all entities (e.g. via the diagnostics stuff I mentioned above).
2. I think no others really were, but should also be double checked (later).
3. This is something that will still work, but mostly by luck. See the comment above for details: Fix IasZone binary sensor not implementing recompute_capabilities #545 (comment). We need a better solution for hiding the specified entities that doesn't rely on the (dynamically computed) translation_key, which is always "ias_zone" when first pairing the device. By "luck", it's also ias_zone after a HA restart for these entities, as the zone type isn't something we know.