Add quirk for Sonoff ZBMINIR2 #3428
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #3428 +/- ##
=======================================
Coverage 89.44% 89.44%
=======================================
Files 311 312 +1
Lines 10031 10054 +23
=======================================
+ Hits 8972 8993 +21
- Misses 1059 1061 +2 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
Hi @TheJulianJES, I've created a PR. Is there anything I need to do before merging it? |
| 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.
Hmm, in the future, we should have a better solution for this, so we don't need to duplicate it across all quirks.
There was a problem hiding this comment.
It's possible that zigpy/zigpy#1505 can clean this up?
zhaquirks/sonoff/zbminir2.py
Outdated
| edge_trigger = 0x00 | ||
| pulse_trigger = 0x01 | ||
| normally_off_follow_trigger = 0x02 | ||
| normally_on_follow_trigger = 0x82 |
There was a problem hiding this comment.
I'll have to check which naming convention we want to use here.
Other possibilities include: EdgeTrigger, Edge_Trigger, and Edge_trigger.
TheJulianJES
added
the
needs review
|
Thank you very much for your assistance. |
|
@TheJulianJES Can this be merged? Z2M has had proper support for this device for a while, and it would be a shame to block it over variable naming. |
|
I have tested the Quirk, few insights:
|
|
I just got this device today, tried this quirk and can confirm it's still a bit weird when using detached mode like tomas said. So I'm new to these types of devices, but is it normal that when using the external trigger mode of a pulse trigger, the relay switch shows up as on for a second, then reverts to false? This seems like it would make it basically impossible to distinguish between double-tapping the switch or holding the switch for a while. Wouldn't this be what the sensor is for? |
TheJulianJES
added
the
v2 quirk
There was a problem hiding this comment.
The lines regarding the "no manufacturer id" modification are not covered.
I'm merging this to get initial support for the device, but we should try and use manufacturer_id_override = foundation.ZCLHeader.NO_MANUFACTURER_ID in the future (when zigpy/zigpy#1505 is merged) and hopefully be able to remove those uncovered lines.
Otherwise, we should add (trivial) tests for this and/or move the _read_attributes and _is_manuf_specific overrides, as they're also required for other Sonoff devices.
Regarding the device implementation of the "detached mode", that's not affected by us. Not sure we can do anything about it with a quirk.
|
I can confirm the strange behavior in detached mode. It looks like the state of the relay and also the state of the external switch are mapped to the switch entity. I tried the detached mode with the same device (firmware 0x00001002) with Z2M without any issues. The state of the relay is mapped to the switch entity and on external switch use, the HA device receives toggle actions. |
|
Since the behavior differs from Z2M, please create an issue in this repo and link it here. |
|
I've just created a new issue for this in #3579 |
Proposed change
Add quirk for Sonoff ZBMINIR2
Additional information
Checklist
pre-commitchecks pass / the code has been formatted using Black