Skip to content

Ignore runtime enabled state in primary entity election - #862

Merged
TheJulianJES merged 2 commits into
zigpy:devfrom
TheJulianJES:tjj/primary-election-ignore-enabled
Aug 25, 2026
Merged

Ignore runtime enabled state in primary entity election#862
TheJulianJES merged 2 commits into
zigpy:devfrom
TheJulianJES:tjj/primary-election-ignore-enabled

Conversation

@TheJulianJES

@TheJulianJES TheJulianJES commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #861 (see the enabled/disabled design discussion there).

The weight election previously filtered candidates on e.enabled, so an entity disabled via the entity registry in HA lost its primary spot to the runner-up on the next re-election. The primary entity describes the main feature of the device — a smart plug whose switch entity is disabled is still a smart plug — so a registry toggle should not reassign it. Since primary drives entity naming in HA (the primary entity takes the bare device name), reassigning it also caused naming churn when an entity was disabled and re-enabled.

In practice, the old behavior was timing-dependent anyway: HA Core only toggles the enabled flag on registry changes and nothing re-runs the election at that point, so a disabled winner kept primary until the next entity recomputation happened to run for an unrelated reason. HA Core also calls disable() for default-disabled entities only after the first election has already run, so the enabled filter effectively never influenced a first election either.

Changes

  • The weight election no longer filters candidates on enabled — disabled entities stay candidates and can keep (or win) the primary spot. Enabling/disabling an entity now never changes the election outcome. Together with Exclude weight 0 entities from primary entity election  #859, candidacy now simply means: non-zero primary weight and not explicitly excluded.
  • This also makes the explicit-primary check and the weight election consistent: the explicit check never looked at enabled, so an explicitly primary but disabled entity already kept its spot.

Tests

  • test_primary_entity_election_disabled_winner is renamed to test_primary_entity_election_ignores_enabled and asserts the new semantics: the disabled switch stays primary, the IAS zone runner-up does not take its spot, and re-enabling changes nothing.
  • It also pins the explicit half of the consistency claim: an explicitly primary entity keeps its spot when disabled.

Notes

  • No device snapshots change: entities are only ever disabled at runtime by HA Core, never during initial discovery, so the enabled filter never influenced a snapshot's election.
  • Default-disabled weight-0 diagnostics (LQI/RSSI) are unaffected by re-admitting disabled entities: Exclude weight 0 entities from primary entity election  #859 already excludes weight-0 entities from candidacy regardless of enabled state.
  • One deliberate behavior delta beyond the headline: disabling one of two tied equal-weight candidates (e.g. one gang of a 2-gang switch) no longer resolves the tie on a later re-election — the device keeps no primary entity. The old tie-break-by-disable was timing-dependent anyway (the first election at join always tied, since HA disables entities only afterwards).
  • Resolves the review observations from Separate computed primary entity state from _attr_primary #861: the explicit-primary path ignoring enabled is now consistent by design (documented in the code comment), and re-running the election on enable/disable is no longer needed since enable/disable is a no-op for the election.

@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.17%. Comparing base (dbdc80a) to head (b626ed4).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #862   +/-   ##
=======================================
  Coverage   97.17%   97.17%           
=======================================
  Files          57       57           
  Lines       10542    10542           
=======================================
  Hits        10244    10244           
  Misses        298      298           

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

@zigpy-review-bot zigpy-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the new commit only (40caf861) — the rest collapses once #861 lands, per the PR body. No blockers and no must-address items. Leaving this as a comment rather than an approval only because the diff will change shape on rebase; the substance looks right to me.

Verified claims

  • Zero snapshot diffs — confirmed. Ran tools/regenerate_diagnostics.py in a worktree at 40caf861; git diff --stat tests/data/devices/ is empty. That is the structurally expected result rather than a lucky one: _attr_enabled defaults to True and nothing inside zha ever sets it False, so at snapshot-generation time the removed e.enabled term was already a no-op.
  • "Nothing re-ran the election on enable/disable anyway" — confirmed on the HA side. homeassistant/components/zha/helpers.py toggles the flag in the entity-registry-updated listener (platform_entity.disable() / .enable()) with no recomputation, and the post-add pass (for entity in entities_to_add: if not entity.enabled: entity.entity_data.entity.disable()) runs after _async_add_entities, i.e. after the first election has already completed. So the old behavior was really "reassign at some arbitrary later recomputation", not "reassign on disable" — which makes this less a semantics change than a removal of nondeterminism. Worth keeping that framing in the final commit message.
  • The explicit-primary check already ignored enabled — confirmed; explicitly_primary filters on entity._attr_primary alone. I also exercised it end-to-end (set _attr_primary = True, disable(), recompute_entities()): the entity keeps the spot and the runner-up does not take it, so the consistency the new comment claims is real and not just asserted.
  • HA naming consequencehomeassistant/components/zha/entity.py sets _attr_name = None when meta.primary, so a registry-disabled primary means no entity carries the bare device name. That is the deliberate trade-off here and I think it is the right one: the alternative was handing the device name to a runner-up at an unpredictable moment.

No coverage lost in the rename

test_primary_entity_election_disabled_winner was the only test whose name pointed at #861's up-front entity.primary = False clearing loop, so repurposing it means that test no longer exercises it. I checked the loop is still covered: deleting it makes both test_primary_entity_reelection and test_primary_entity_election_explicit_primary_takes_over fail. Nothing to do — just confirming the rename is safe.

Optional

  • The description lists "the explicit-primary check never looked at enabled" as one of the two things this makes consistent, and the new code comment states it, but no test pins it (disable() appears exactly once in the suite, in the test this PR modifies). See the inline note for a few lines that would lock it in.
  • One concrete instance of the weight-0 re-admission the description flags for #859: CoordinatorDevice.discover_entities() yields only DeviceCounterSensors, which are all primary_weight = 0 and _attr_entity_registry_enabled_default = False. Once HA disables them, a coordinator re-election previously hit if not candidates: return; it now reaches the tie branch instead and logs Primary entity tie between ..., no primary entity at debug on every recomputation. The outcome is identical (no primary either way), so this is purely a new debug line on a rarely-hit path — and #859 removes it. Note, not an ask; it does support the description's "no primary for real devices" conclusion, since the one device class where every entity is weight 0 lands on the tie path.

Incidental, and correct

Dropping "non-counter entities" from the comment fixes stale wording rather than describing a behavior change. That phrase referred to the hasattr(e, "info_object") term added in #298, which #804 (ccc36515) removed while leaving the comment behind — counter entities have been candidates ever since. The rewritten comment no longer claims a filter that does not exist.

Checks

Full suite 1348 passed; ruff check and ruff format --check clean; mypy zha/ inside the worktree venv (real deps installed, not the dependency-less pre-commit env) reports no issues, so no regression against the dev baseline. A second-opinion pass scoped to this commit returned no findings.

The merge order in the description (#861#859 → this) looks right — landing #859 first is what makes the weight-0 note above moot.

Comment thread tests/test_device.py
The primary entity describes the main feature of the device, which does
not change when its entity is disabled via the entity registry in HA.
Disabled entities now stay election candidates, so disabling the
current primary entity does not hand its spot to the runner-up (and
does not cause naming churn in HA), and both the explicit-primary check
and the weight election now consistently ignore enabled state.

In practice, nothing re-ran the election on enable/disable anyway: HA
Core only toggles the flag, so a re-election happened at the earliest
on the next entity recomputation, making the previous behavior
timing-dependent.

No device snapshots change: entities are only ever disabled at runtime
by HA Core, never during initial discovery.
@TheJulianJES
TheJulianJES force-pushed the tjj/primary-election-ignore-enabled branch from 40caf86 to b626ed4 Compare August 25, 2026 21:05
@TheJulianJES
TheJulianJES requested a balanced review from Copilot August 25, 2026 21:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Keeps primary entity election stable regardless of runtime enabled state, preventing naming churn.

Changes:

  • Removes enabled-state filtering from weighted election candidates.
  • Updates tests for disabled weighted and explicit primary entities.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
zha/zigbee/device.py Preserves disabled entities as election candidates.
tests/test_device.py Verifies enabled state does not affect primary selection.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@TheJulianJES
TheJulianJES marked this pull request as ready for review August 25, 2026 21:19
@TheJulianJES
TheJulianJES merged commit 1ff5e8a into zigpy:dev Aug 25, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants