-
Notifications
You must be signed in to change notification settings - Fork 796
Add Sonoff smart plug S60ZBTPF quirk #4031
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
base: dev
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #4031 +/- ##
==========================================
+ Coverage 91.19% 91.21% +0.01%
==========================================
Files 335 337 +2
Lines 10886 10950 +64
==========================================
+ Hits 9928 9988 +60
- Misses 958 962 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quirks look very similar to me. If they are indeed basically the same, we should combine them in one file, e.g. sonoff/plug.py
and add the different variants for models in one quirk using applies_to
.
async def _read_attributes( | ||
self, | ||
attribute_ids: list[t.uint16_t], | ||
*args, | ||
manufacturer: int | t.uint16_t | None = None, | ||
**kwargs, | ||
): | ||
"""Read attributes ZCL foundation command.""" | ||
return await super()._read_attributes( | ||
attribute_ids, | ||
*args, | ||
manufacturer=foundation.ZCLHeader.NO_MANUFACTURER_ID, | ||
**kwargs, | ||
) | ||
|
||
@property | ||
def _is_manuf_specific(self): | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to set manufacturer_id_override
for the cluster instead.
See as an example:
manufacturer_id_override: t.uint16_t = foundation.ZCLHeader.NO_MANUFACTURER_ID |
async def _read_attributes( | ||
self, | ||
attribute_ids: list[t.uint16_t], | ||
*args, | ||
manufacturer: int | t.uint16_t | None = None, | ||
**kwargs, | ||
): | ||
"""Read attributes ZCL foundation command.""" | ||
return await super()._read_attributes( | ||
attribute_ids, | ||
*args, | ||
manufacturer=foundation.ZCLHeader.NO_MANUFACTURER_ID, | ||
**kwargs, | ||
) | ||
|
||
@property | ||
def _is_manuf_specific(self): | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above here:
We should be able to set manufacturer_id_override
for the cluster instead. See as an example:
manufacturer_id_override: t.uint16_t = foundation.ZCLHeader.NO_MANUFACTURER_ID |
zhaquirks/sonoff/s60zbtpf.py
Outdated
# network_led = ZCLAttributeDef( | ||
# name="network_led", | ||
# id=0x0001, | ||
# type=t.Bool, | ||
# ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this attribute not work? If so, it can be removed. We shouldn't add commented code.
If the attribute does work, we can define it here (so just uncomment). We don't have to necessarily add a config entity for all attributes (although it might make sense for this attribute).
zhaquirks/sonoff/s60zbtpf.py
Outdated
# .enum( | ||
# SonoffCluster.AttributeDefs.network_led.name, | ||
# SonoffNetworkLedSetType, | ||
# 0xFC11, | ||
# translation_key = "network_led", | ||
# fallback_name = "Network led", | ||
# ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to the comment above regarding the network_led
attribute:
Does this not work? Should this be a switch
instead?
zhaquirks/sonoff/s60zbtpg.py
Outdated
.add_to_registry() | ||
) | ||
|
||
# class SonoffS60zb(CustomDevice): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this commented line.
@TheJulianJES I have already made the changes. |
Proposed change
Additional information
Checklist
pre-commit
checks pass / the code has been formatted using Black