Add flow volume sensor entity for the FlowMeasurement cluster

[benbancroft]

Proposed change

Adds a new "volume flow rate" sensor for discovered FlowMeasurement clusters. This is required for the Sonoff SWV-BSP Smart Water Valve, but likely other devices with FlowMeasurement being a standardised Zigbee cluster.

Raw value of "measured_value" is 10 * flow_rate, as per Zigbee cluster specification for FlowMeasurement.

This feature was requested as part of zigpy/zha-device-handlers#3298

Testing

I have tested this with my Sonoff SWV-BSP Smart Water Valve and the sensor is appearing and being exported to Home Assistant no issues with a sane value that matches the flow of water. I have not tested this with any other device that implements FlowMeasurement cluster, but I assume it should work fine.

[tofurky]

these appear to be otherwise alphabetized

[benbancroft]

Thanks. Hopefully resolved in latest commit (plus other places I'd not stuck to alphabetical order). I also dropped _MEASUREMENT for consistency with other clusters/sensors (e.g. TemperatureMeasurement is called CLUSTER_HANDLER_TEMPERATURE).

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 96.39%. Comparing base (57cc44b) to head (ec3d7bc).
Report is 20 commits behind head on dev.

Files with missing lines	Patch %	Lines
zha/application/platforms/sensor/__init__.py	72.72%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #187      +/-   ##
==========================================
+ Coverage   95.89%   96.39%   +0.49%     
==========================================
  Files          61       61              
  Lines        9404     9402       -2     
==========================================
+ Hits         9018     9063      +45     
+ Misses        386      339      -47     

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

[fgsch]

This is great!

Is there anything we can do to get this reviewed and merged?

[dmulcahey]

If we are going to implement this we should add the other 3 attributes to the ZCL init attrs on the cluster handler. Also, take note that a measured value of 0xffff means the measurement is unknown and the note from 4.1.3 about how the min and max affect the readings of measured value.

[dmulcahey]

@benbancroft bumping this... can you make the small tweaks from my comment above?

[benbancroft]

Hi @dmulcahey

I've not had the time to address this yet unfortunately - had family over this weekend.
Agree with your comment regarding implementation of cluster constraints. I plan to have a go at addressing this soon (hopefully later this week).

Question: where would you recommend implementing these checks? Are there other examples of where other clusters do similar I can reference?

[dmulcahey]

Hi @dmulcahey

I've not had the time to address this yet unfortunately - had family over this weekend. Agree with your comment regarding implementation of cluster constraints. I plan to have a go at addressing this soon (hopefully later this week).

Question: where would you recommend implementing these checks? Are there other examples of where other clusters do similar I can reference?

I know illuminance does something like this in the formatter:

zha/zha/application/platforms/sensor/__init__.py

Lines 760 to 766 in c99237f

	def formatter(self, value: int) -> int | None:
	"""Convert illumination data."""
	if value == 0:
	return 0
	if value == 0xFFFF:
	return None
	return round(pow(10, ((value - 1) / 10000)))

[benbancroft]

Hi @dmulcahey

Thanks! I've gone ahead and implemented the 0xffff unknown state in the same way via a formatter.

Regarding MinMeasuredValue and MaxMeasuredValue - I've left this as is for now as this looks like a wider issue throughout most other sensors within ZHA (e.g. Temperature Measurement cluster).

I guess strictly speaking, we can largely place trust in the server that it is implementing these constraints for the measured values it reports. The only real use on the client side is enforcing this relationship, or utilising min/max for display of data (e.g. rendering a sliding display). For the latter, as this sensor is exported as a numerical to Home Assistant, it doesn't apply here.

Please let me know your thoughts/suggestions here.

[fgsch]

Can someone review and merge this if it looks OK? 🙏