Add quirk for Sonoff SWV Smart Water Valve

[benbancroft]

Proposed change

Adds support for Sonoff SWV Smart Water Valve custom cluster, which exposes a new attribute called valve_status for querying if the valve is leaking or if the water supply to the valve has stopped.

Code should hopefully should be self explanatory, the only complication is the overridden _is_manuf_specific() property in SonoffFC11Cluster class. This is required, as if the device receives an attribute read or notify zigbee command which has manufacturer_specific = True set (default if cluster's ID is within manufacturer specific range which this is), it will return UNSUPPORTED_ATTRIBUTE.

This feature was requested as part of #3298

Additional information

• zha pull request for Home Assistant Sensors: Add sensors for Sonoff SWV Smart Water Valve status zha#189
• Home Assistant core pull request for language strings for sensor: Add ZHA language strings for Sonoff SWV home-assistant/core#124989

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]

Codecov Report

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

Project coverage is 88.50%. Comparing base (5c4e08d) to head (834e068).
Report is 6 commits behind head on dev.

Files with missing lines	Patch %	Lines
zhaquirks/sonoff/swv.py	95.65%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #3340      +/-   ##
==========================================
+ Coverage   88.48%   88.50%   +0.01%     
==========================================
  Files         305      306       +1     
  Lines        9623     9646      +23     
==========================================
+ Hits         8515     8537      +22     
- Misses       1108     1109       +1     

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

[fgsch]

I think you want Normal (0) and probably Leakage and Shortage (3).

[benbancroft]

So the absence of Normal (0) and Leakage and Shortage (3) was intentional - if you look at class you will see that this is a bitfield not an enum. Leakage and Shortage (3) would just be Water_Shortage (1) & Water_Leakage (2)

If you look what I've done in ZHR PR (zigpy/zha#189), I'm actually exporting two binary sensors, one for each bit. I personally think this is a lot nicer than how they did it in Zigbee2MQTT.

[fgsch]

Ah, I see, I missed the bitmap8.
Two related comments:

• The docstrings say enum :)
• While this is fine for the sensor, the other part is the response when reading the attribute directly via Manage Zigbee Device interface. This is not a big issue, but it'd be nice to have the full range of values IMO.

[fgsch]

Actually, is there any reason you cannot use an enum here and check for the specific values on the other PR?

[fgsch]

I believe attributes are deprecated. It might be better to use ZCLAttributeDef to define this.

[benbancroft]

Thanks - I based this off the other Sonoff quirk, with no prior experience writing quirks. Let me look into ZCLAttributeDef and see how easy it is to convert over.

[fgsch]

Nice! I was testing a similar change using the Quirks Builder v2 and exposing the state as a sensor.

I'll probably open a PR on Monday, as I wrote most of the code already, but I've left some comments in this PR.

[benbancroft]

Hi @fgsch

Thanks for your review comments, much appreciated! Given you've gone to the work of writing yours, I would definetly push it up. I wasn't aware of Quirks Builder v2 (with this being my first quirk), but looking at API it looks very nice and I guess it allows you to avoid a ZHA change also? What might be a pain is exporting two not one sensors like I have (which in my personal opinion makes more sense for these sensors).

[fgsch]

Hi @fgsch

Thanks for your review comments, much appreciated! Given you've gone to the work of writing yours, I would definetly push it up. I wasn't aware of Quirks Builder v2 (with this being my first quirk), but looking at API it looks very nice and I guess it allows you to avoid a ZHA change also? What might be a pain is exporting two not one sensors like I have (which in my personal opinion makes more sense for these sensors).

My original plan was to expose the valve status as a sensor, but I've been struggling to do so as I envisioned.
I don't know if it's me or if it's just not possible, but unfortunately, no other zha quirks are using this.
Might be a good time to ask around.

As for my PR, I sort of ran out of time today. I will try to do it tomorrow.

[fgsch]

And finally, here's my PR: #3346

I managed to get the sensor working as I wanted with help from some devs in discord. Unfortunately, it is not yet possible to do the same with binary sensors.