Use 'current_summ_delivered' attrib for ZHA Smart energy channel #50471
Conversation
Use the 'current_summ_delivered' attrib (id: 0x0000) on the 'smartenergy_metering' cluster (id: 0x0702); don't use the 'instantaneous_demand' attr (id: 0x0400).
|
Hi @stattin42, It seems you haven't yet signed a CLA. Please do so here. Once you do that we will be able to review and accept this pull request. Thanks! |
probot-home-assistant
bot
added
breaking-change
integration: zha
small-pr
|
Hey there @dmulcahey, @Adminiuga, mind taking a look at this pull request as its been labeled with an integration ( |
|
We shouldn’t just change this like this. What we need to do is enable support for multiple sensors per zigbee cluster. |
|
+1. As. Middle ground, short term solution should we just have a sensor class for specific vendors to use the summation attr? |
|
Or, we can fire events for attr changes… and let folks add template entities until we’re ready to support them correctly 🤷🏻♂️ |
|
That's not a bad idea. I'm on the fence if we should do it for all attributes or only for those specifically not handled by the entities? Probably the later... |
|
Hi, and thanks for looking at this. I don't have a strong view on particular solution, but the attribute currently used for the Smart Energy Channel represents a load measurement rather than an energy measurement so the current state of things is unintuitive at the very least and there currently seems to be no means to get the actual energy attribute. |
Are you using Tuya TS0121 or rebranded devices of it as reference hardware to test this? See -> zigpy/zha-device-handlers#605 Users there report that they retrieve the total usage from the device in the Metering cluster (0x0702) but there is no sensor for it. Users tested/confirmed issue on BlitzWolf BW-SHP13, Innr SP 120 / SP120, Schwaiger ZHS15, Lonsonho 16A Energy Monitoring Plug |
I read from developers answer that problem is "only one attribute per cluster"
We need from Metering cluster 0x0702 attribute 0x0000 "current_summ_delivered" I understand that bigger change is needed to get all possible attributes, but maybe simple temporary change, to get kWh, helps many users to wait perfect solution more happily. |
|
The problem is, that your device support sum delivered, when this was added, most devices supported instantaneous demand. So this is a breaking change, without much gain for all users. |
|
Update channel to support both attributes, add a sensor entity scoped to your specific vendor. |
Yes, it can be considered a breaking change. But there is something in your logic w r t the gain which does not seem entirely right to me. To my understanding instantaneous demand is not reliable for energy metering as, according to previous description of yours, it would rely on being retrieved/read very regularly. Any missed or lost reports would introduce error in metering. Thus, to me the change would seem to provide gain for all users and huge gain for (increasingly) many users. I don't know the detailed rationale for using instantaneous demand in the first place, but considering that it may not be reliable for metering, you seem to imply that early devices did not support the mandatory current summation delivered attribute. If that is the case, your view seems to favour support of non-compliant devices over support of compliant devices. As I don't believe it is your intention to be unreasonable, we are probably missing something. But, at least to me, it is not clear what. Why do you believe instantaneous demand is the correct attribute for energy metering and why should HA (unconditionally) assume or require that devices support an optional attribute when there is a mandatory attribute which should be more reliable for energy metering and which should presumably be supported by all devices? |
I have Shwaiger, BlitzWolf, Lonshonto, Innr SP120 plugs, all acting similarly. Other users report more models with my problem. |
I think supporting both attributes would be good, but isn't it backwards to condition a sensor for current summation delivered on specific vendors? It is current summation delivered which is mandatory and instantaneous demand which is optional and only supported by certain devices, so wouldnt it make more sense to make the sensor for instantaneous demand conditional on vendor or device? ... or at least not create the sensor for instantaneous demand if the device does not support it? |
Use attr current_summ_delivered for smart energy metering sensor
|
running this branch results in a populated smartenergy_metering field, but the value is not in kWh, as it pretends to be. |
You can do that right in the integration: https://github.com/flabbamann/core/blob/zha-current-summ-delivered/homeassistant/components/zha/core/channels/smartenergy.py#L151 I'm now running zha as a custom component, but I'm not really happy with that 😕. I see that this change may break other peoples setups. Unfortunately I have too little knowledge about zigbee to fix this properly... |
|
I believe that the integration platform can provide the Riemann sum of the values provided by a source sensor? |
How? The cluster Metering (Id: 0x0702), Attribute unit_of_measure (id: 0x0300) returns "enum8.undefined_0x00", do I have to overwrite it? Btw, if anyone wonders why device triggers don't work anymore: When running ZHA as a custom component, the devices get loaded after the automations and produce an error. Reloading the automations after restarting fixes them. |
|
Since it was already decided that this PR can't be applied, can we close this? The focus should be on providing an alternative (either by refactoring ZHA to support multiple sensors per cluster, or by having events for this). Discussions on how to use this as a custom_component, or how to replace this with the Riemann integration should happen on the forums or discord, not on PRs. |
|
A least for me it remains unclear, why developers stick to keep solution what works only for few plug types and not working for majority other types (replays for this thread show over 10 types of plugs waiting this change) |
Do you have any hard data that it "works only for a few plug types"? Changing this will be a breaking change. Without have a clear picture of the actual number of users, it does not make sense to proceed and break. A new PR implementing proper multi-entity support for clusters would be nice, though :) |
|
You are correct, I haven't correct data how many plugs use this non
standard cluster. I asked about this in here and got reply only for one
plug type.
Just idea, maybe new analytics function could gather data about used plug
types?
…On Wed, Sep 15, 2021, 20:20 Abílio Costa ***@***.***> wrote:
A least for me it remains unclear, why developers stick to keep solution
what works only for few plug types and not working for majority other types
(replays for this thread show over 10 types of plugs waiting this change)
Sure, it is breaking change, but only for maybe few users. 10 time more
users would be happy when their plugs start to show energy.
Just can't understand this decision.
Do you have any hard data that it "works only for a few plug types"?
The reason why you see "over 10 types of plugs" with this issue, and not
much of the other type, is the fact that the people who have the ones that
are currently working, are not complaining :) So your sample size here is
completely bias towards the failing use case.
Changing this will be a breaking change. Without have a clear picture of
the actual number of users, it does not make sense to proceed and break.
A new PR implementing proper multi-entity support for clusters would be
nice, though :)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#50471 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBTCAR45VX4CFW3CFLR7HTUCDIXBANCNFSM44WKOCDA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
As current official solution reads non standard cluster (instantaneous_demand) instead of current_summ_delivered, it seems logical to me that there are more plugs that support standard and only few will suffer breaking change |
|
The breaking change would be permanent for some. If they have any automations triggered off the instantaneous power (eg to detect washing machine start/finish), if I understand correctly they would have to abandon them. I feel the same disappointment due to the missing new feature but I suggest effort is better spent on supporting both sensors. |
@juhanjuku instantaneous_demand is not a cluster, it is an attribute of the Smart Energy cluster. It is part of the standard, AFAIK. Why do you say it is "non standard". |
@abmantis |
|
The ... but, then again, it would be a breaking change, no argument about that. Question is which is the lesser evil until there is multi-entity support ... For me, unfortunately, it is clearly to keep patching every release. I would love to contribute to a more universal/acceptable solution, but I am afraid I simply cannot find the time now to try to look into it all on my own. |
|
Moreover instantaneous_demand attribute is not suitable to match the state class: total_increasing in order to coop with the energy dashboard. Could be of interest to observe how many negative feedback could bouncing back after a breaking change, a revert back action is still possible then.... |
|
+1 for relacing and using This is a confirmed issue on Tuya TS012, BlitzWolf BW-SHP13, Aqara LUMI Plug, Innr SP 120, SmartThings Smart Plug (UK), Schwaiger ZHS15, Lonsonho 16A Energy Monitoring Plug, Rehentele 16A Energy Monitoring Plug, Immax Plug-230V-ZB3.0 Smart Plug, Develco Frient SPLZB-131 Smart Plug Mini, Develco Prosumer Meter, and Develco Products A/S EMIZB-132 Norwegian HAN adapter. |
|
... as well as on TuYa TS0121, BlitzWolf BW-SHP15 ... |
|
As Adminiuga already mentioned, you can have that attribute for specific brands/manufacturers. No need to break everything IMO. |
Quote from stattin42 > 'The instantaneous_demand attribute of the Smart Energy cluster seems to essentially provide the same information as the active_power attribute of the Electrical Measurement cluster.'. It seems' that breaking change could be easily fixed by replacing instantaneous_demand with active power in automation. This is my last attempt to change developers decision. |
|
Is there a tracking issue/PR we can follow for supporting multiple entities per cluster? |
|
In ZigBee Cluster Library Specification documentation we can call it "The Zigbee Bible", as stattin42 underlines, the Metering cluster (0x0702) as subset of the Smart energy cluster library indicate the CurrentSummationDelivered attribute 0x0000 as Mandatory. |
|
You've convinced me. But I think the release notes(/breaking change) needs a very clear explanation of what would happen from a user's point of view, and what they should do about it. e.g. "any existing power monitoring sensors (e.g. W or kW) for each ZHA device will stop working, and if they are still needed they will need to be re-added manually by doing xyz. Any automations that used the old sensors will need to be adjusted manually." Please bear in mind that to normal users, the terms 'instantaneous_demand', 'current_summ_delivered' don't mean much. |
|
@davet2001 you are completely right, what I would like to remark is that I do not want to act as the "nasty elephant", my approval activity goes beyond my intention as I'm still an explorer in GitHub, it would be of fundamental importance to drag, in first, the assignees approval. |
|
We are currently determining what needs to be done to support multiple sensors per cluster. Once we work that out this change won’t be necessary as there will be sensors for both. I do not have an ETA. |
|
@dmulcahey, it sound good! Is in my experience (by temporary use of sql sensors) that different devices make use of different conversion factors in current_summation_delivered register, e.g. Agara Lumi smart plug (lumi.plug.maeu01) in order to obtain kWh need 0.001, on the other hand Tuya smart plug (TS0121) need 0.01, the adoption of specific zhaquirks. will make justice of that? |
@Stefano0042 Ideally devices would provide the corresponding scaling factor in the associated divisor or multiplier attribute (depending on cluster and attribute in question), but (sadly) not all devices do. |
|
closing in favor of #56666 |
Use the 'current_summ_delivered' attrib (id: 0x0000) on the 'smartenergy_metering' cluster (id: 0x0702); don't use the 'instantaneous_demand' attr (id: 0x0400).
Breaking change
ZHA Smart energy channel no longer reports attribute 'instantaneous_demand', but
attribute 'current_summ_delivered'.
Proposed change
As reported in issue #44539 "ZHA plugs - smartenergy_metering > wrong cluster"
many (most?) energy metering capable devices support reporting energy
consumption with attribute 'current_summ_delivered'; not with attribute
'instantaneous_demand'.
Furthermore, Page 215/16 of the zigbee smart energy profile says that cluster
0x702 attribute 0x0 is "CurrentSummationDelivered represents the most recent
summed value of Energy, Gas, or Water delivered and consumed in the premises.
CurrentSummationDelivered is mandatory and must be provided as part of the
minimum data set to be provided by the metering device"
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: