Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compute primary entities #298

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Conversation

puddly
Copy link
Contributor

@puddly puddly commented Nov 19, 2024

Primary entities are picked with a simple weighting system that only picks a clear winner: if there are any ties, no primary entity is chosen.

"Control" entities are given a higher priority than sensors and some sensors internally are given higher priority than others:

  1. For night lights with a motion sensor, the light entity is primary.
  2. For motion sensors with occupancy, opening, and illuminance clusters, the motion entity is primary.
  3. For combined temperature, humidity, and pressure sensors, there is no primary entity.
  4. For power strips with multiple outlets, there is no primary entity.

For the most part it works for me for the vast majority of entities.

This approach does suffer from a few problems. Some devices have unnecessary entities created solely to control/be controlled by other devices.

For example, my blinds have an OnOff cluster:

image

And my water leak sensor has an Opening cluster:

image

I think we should provide an API within quirks v2 to remove entities instead of disabling clusters.

@puddly puddly requested a review from Copilot November 19, 2024 22:29

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 12 changed files in this pull request and generated no suggestions.

Files not reviewed (7)
  • zha/application/platforms/init.py: Evaluated as low risk
  • zha/application/platforms/binary_sensor/init.py: Evaluated as low risk
  • zha/application/platforms/climate/init.py: Evaluated as low risk
  • zha/application/platforms/cover/init.py: Evaluated as low risk
  • zha/application/platforms/fan/init.py: Evaluated as low risk
  • zha/application/platforms/light/init.py: Evaluated as low risk
  • zha/application/platforms/siren.py: Evaluated as low risk
Comments skipped due to low confidence (2)

zha/zigbee/device.py:1092

  • Ensure info_object exists before attempting to delete it to avoid potential AttributeError.
def _compute_primary_entity(self) -> None:

zha/application/gateway.py:357

  • Ensure that the new behavior introduced by '_compute_primary_entity' is covered by tests to verify correct computation and assignment of the primary entity.
device._compute_primary_entity()
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 98.41270% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.36%. Comparing base (82db95f) to head (b724822).

Files with missing lines Patch % Lines
zha/zigbee/device.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #298      +/-   ##
==========================================
+ Coverage   96.33%   96.36%   +0.02%     
==========================================
  Files          61       61              
  Lines        9397     9460      +63     
==========================================
+ Hits         9053     9116      +63     
  Misses        344      344              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TheJulianJES TheJulianJES added the enhancement New feature or request label Nov 24, 2024
@puddly puddly requested a review from Copilot November 25, 2024 18:18

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 12 changed files in this pull request and generated no suggestions.

Files not reviewed (7)
  • zha/application/platforms/init.py: Evaluated as low risk
  • zha/application/platforms/light/init.py: Evaluated as low risk
  • zha/application/platforms/climate/init.py: Evaluated as low risk
  • zha/application/platforms/fan/init.py: Evaluated as low risk
  • zha/application/platforms/cover/init.py: Evaluated as low risk
  • zha/application/platforms/siren.py: Evaluated as low risk
  • zha/application/platforms/lock/init.py: Evaluated as low risk
@puddly puddly marked this pull request as ready for review November 27, 2024 20:13
Copy link
Contributor

@TheJulianJES TheJulianJES left a comment

Choose a reason for hiding this comment

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

This looks like a nice approach in general

@puddly
Copy link
Contributor Author

puddly commented Nov 27, 2024

Feel free to update the weights however you see fit, I think I picked defaults that fixed most of the devices I saw in my own home and a few more from memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants