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

Set ep_attribute of Xiaomi class to "opple_cluster" #2667

Merged

Conversation

TheJulianJES
Copy link
Collaborator

@TheJulianJES TheJulianJES commented Oct 21, 2023

Proposed change

This sets ep_attribute = "opple_cluster" in the XiaomiAqaraE1Cluster class and removes the assignment from all classes that inherit from this class.

Two quirks also used "aqara_cluster" as the ep_attribute instead. All quirks seem to have "standardized" upon using "opple_cluster" as the ep_attribute. So this PR also changes those two quirks.

From what I can see, the ep_attribute of those two quirks was never used anywhere, so we are free to change it without causing any issues. This addresses part of the late review in https://github.com/zigpy/zha-device-handlers/pull/2408/files#r1236909366

Lastly, it also removes inheriting ManufacturerSpecificCluster in XiaomiAqaraE1Cluster.
ManufacturerSpecificCluster marks name and ep_attribute as Final, but we're re-assigning them.
There's no reason to inherit from ManufacturerSpecificCluster in this case. It's mostly used as an internal class for zigpy for non-quirked devices.

Additional information

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works

@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cfd4be0) 86.81% compared to head (586992c) 86.79%.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2667      +/-   ##
==========================================
- Coverage   86.81%   86.79%   -0.02%     
==========================================
  Files         280      280              
  Lines        8575     8564      -11     
==========================================
- Hits         7444     7433      -11     
  Misses       1131     1131              
Files Coverage Δ
zhaquirks/xiaomi/__init__.py 92.69% <100.00%> (ø)
zhaquirks/xiaomi/aqara/feeder_acn001.py 96.49% <ø> (-0.04%) ⬇️
zhaquirks/xiaomi/aqara/illumination.py 100.00% <ø> (ø)
zhaquirks/xiaomi/aqara/motion_ac01.py 86.36% <ø> (-0.31%) ⬇️
zhaquirks/xiaomi/aqara/motion_ac02.py 88.46% <ø> (-0.22%) ⬇️
zhaquirks/xiaomi/aqara/motion_agl04.py 66.66% <ø> (-1.08%) ⬇️
zhaquirks/xiaomi/aqara/plug_eu.py 100.00% <ø> (ø)
zhaquirks/xiaomi/aqara/remote_h1.py 100.00% <ø> (ø)
zhaquirks/xiaomi/aqara/smoke.py 100.00% <ø> (ø)
zhaquirks/xiaomi/aqara/switch_t1.py 100.00% <ø> (ø)
... and 2 more

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

`ManufacturerSpecificCluster` marks `name` and `ep_attribute` as `Final`, but we're re-assigning them.
There's no reason to inherit from `ManufacturerSpecificCluster` in this case. It's mostly used as an internal class for zigpy (for non-quirked devices) IIRC.
…iAqaraE1Cluster`

`XiaomiAqaraE1Cluster` now sets the `ep_attribute` to `"opple_cluster"` already.
@TheJulianJES TheJulianJES force-pushed the tjj/xiaomi_aqara_opple_ep_attribute branch from 59066f4 to 586992c Compare October 21, 2023 01:33
@TheJulianJES TheJulianJES merged commit 52667ef into zigpy:dev Oct 23, 2023
14 checks passed
elupus pushed a commit to elupus/zha-device-handlers that referenced this pull request Jan 17, 2024
* Add `opple_cluster` ep_attribute for `XiaomiAqaraE1Cluster`

* Don't inherit `ManufacturerSpecificCluster` to `XiaomiAqaraE1Cluster`

`ManufacturerSpecificCluster` marks `name` and `ep_attribute` as `Final`, but we're re-assigning them.
There's no reason to inherit from `ManufacturerSpecificCluster` in this case. It's mostly used as an internal class for zigpy (for non-quirked devices) IIRC.

* Remove `ep_attribute = "opple_cluster"` for classes inheriting `XiaomiAqaraE1Cluster`

`XiaomiAqaraE1Cluster` now sets the `ep_attribute` to `"opple_cluster"` already.

* Use `opple_cluster` `ep_attribute` in Xiaomi tests (instead of cluster ID)

* Remove `ep_attribute = "aqara_cluster"` for two quirks
lgraf pushed a commit to lgraf/zha-device-handlers that referenced this pull request May 6, 2024
* Add `opple_cluster` ep_attribute for `XiaomiAqaraE1Cluster`

* Don't inherit `ManufacturerSpecificCluster` to `XiaomiAqaraE1Cluster`

`ManufacturerSpecificCluster` marks `name` and `ep_attribute` as `Final`, but we're re-assigning them.
There's no reason to inherit from `ManufacturerSpecificCluster` in this case. It's mostly used as an internal class for zigpy (for non-quirked devices) IIRC.

* Remove `ep_attribute = "opple_cluster"` for classes inheriting `XiaomiAqaraE1Cluster`

`XiaomiAqaraE1Cluster` now sets the `ep_attribute` to `"opple_cluster"` already.

* Use `opple_cluster` `ep_attribute` in Xiaomi tests (instead of cluster ID)

* Remove `ep_attribute = "aqara_cluster"` for two quirks
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.

1 participant