Codex updated: Preserve discovery progress and group cleanup on membership changes#835
Codex updated: Preserve discovery progress and group cleanup on membership changes#835dmulcahey wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #835 +/- ##
=======================================
Coverage 97.27% 97.27%
=======================================
Files 55 55
Lines 10930 10954 +24
=======================================
+ Hits 10632 10656 +24
Misses 298 298 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3680327 to
abed086
Compare
|
This is an interesting edge case... not sure how many folks would hit this in reality but after poking through it I figured it would be worth at least sharing it. Codex Suggested Follow-up plan: semantic group-control eligibility and retained-group reconciliationPR #835 is now intentionally limited to its original scope: endpoint-discovery error containment plus safe group-entity lifecycle cleanup when total or platform membership falls below quorum. The later experiment around group-control eligibility, per-endpoint counting, aggregate filtering, and retained-entity reconciliation has been removed from this PR and should be handled in a dedicated follow-up. This comment records the evidence and implementation plan so that work is not lost. Problem to solveZHA currently treats the existence of a light, fan, or switch platform entity on a member endpoint as a proxy for whether that endpoint should:
Those concepts are not equivalent. One endpoint may expose multiple same-platform entities, and a user-facing The removed experiment improved two things by counting one platform vote per endpoint and excluding configuration/diagnostic entities. However, Zigbee invariantsThe design should preserve these protocol facts:
Therefore, quorum should count each unique member endpoint at most once per platform. Tests should not model duplicate insertion of the same endpoint as two memberships. Captured-device evidenceThe complete current fixture corpus was scanned:
The 11 Groups-without-OnOff fixtures are:
Two distinct real captures, Three additional STANDARD Recommended eligibility designIntroduce one explicit semantic capability on platform entities. Naming can follow maintainer preference; the important behavior is: class PlatformEntity:
_attr_supports_group_control = True
@property
def supports_group_control(self) -> bool:
return self.entity_category is None and self._attr_supports_group_control
class ConfigurableAttributeSwitch(PlatformEntity):
_attr_supports_group_control = FalseThen:
This is preferable to importing and type-checking Why simpler heuristics are insufficient
The explicit capability makes the semantic contract reviewable instead of inferring it from discovery metadata. Retained group reconciliationAfter eligibility is defined correctly, direct member-add/remove handling should:
This is where The light minimum-transition reset belongs in the same follow-up. Required tests
Separate physical-groupcast questionThe corpus also contains eight The minimal follow-up should not silently reject those devices based only on JSON evidence. Original-signature enforcement should be an explicit design decision, preferably supported by hardware testing and an override mechanism for known non-conformant devices. Acceptance criteria and sequencing
Suggested implementation sequence:
|
This pull request was created with Codex and is an updated version of the pull request it supersedes.
Original pull request: #691
This pull request introduces robust error handling and lifecycle management improvements for device and group entity discovery in the ZHA integration. The changes ensure that exceptions during endpoint discovery do not halt discovery for other endpoints, and that group entity cleanup properly invokes lifecycle methods, preventing resource leaks and stale entities.
Improved error handling for entity discovery
discover_device_entitiesto catch exceptions during endpoint discovery, log the error, and continue processing remaining endpoints, preventing partial entity loss.test_discover_device_entities_continues_after_endpoint_exception) to verify that discovery continues after an endpoint exception.Group entity lifecycle and cleanup enhancements
discover_group_entitiesto schedule lifecycle cleanup (on_remove) for group entities when group membership drops below quorum, instead of directly clearing entities, ensuring proper cleanup and avoiding leaks.test_discover_group_entities_member_drop_runs_group_entity_on_remove) and stale platform entity pruning (test_discover_group_entities_platform_quorum_drop_prunes_stale_platform_entity).Test infrastructure updates
tests/test_discover.pyto support the new tests and discovery logic.## SummaryMakes discovery resilient to per-endpoint exceptions and ensures stale group entities are removed via lifecycle cleanup when membership/platform quorum changes.
Failing tests addressed
tests/test_discover.py::test_discover_device_entities_continues_after_endpoint_exceptiontests/test_discover.py::test_discover_group_entities_member_drop_runs_group_entity_on_removetests/test_discover.py::test_discover_group_entities_platform_quorum_drop_prunes_stale_platform_entityVerification
pytest(branch-local targeted run): all listed tests pass.Scoped replacement validation
The original summary above is preserved verbatim. This replacement is intentionally limited to endpoint-discovery resilience and safe group-entity lifecycle cleanup. Semantic group-control eligibility and retained aggregate/capability reconciliation are documented separately as follow-up work.
1341 passeddevatda781c789433e3d8a8f8277d47bbdeb4426f9cf2