use power formatting factors for active power calculation #7978
Conversation
|
Hey @bluemoehre, thanks for your pull request! Tip Modified bundles can be downloaded here. DDB changesModified
ValidationTip Everything is fine ! 🕧 Updated for commit 8d423a3 |
|
Several DDFs could possibly be updated (of course this requires the related Formatting Block to report correct factors): |
|
@ebaauw :
Just for safety: Won't these files interfere? |
bluemoehre
changed the title
bluemoehre
changed the title
|
I wouldn't change the logic on the generic
Interfere with what? The methods are invoked only for non-managed devices (exposed by legacy code instead of DDF), see deconz-rest-plugin/de_web_plugin.cpp Line 1287 in ef6ca9f I think the DDF_AnnoteZclParse() is used when creating a new DDF from the GUI.
|
Most of the affected DDFs already have a custom eval prop with their own calculation. As long as their config has precedence over the generic files, we are totally fine to change that. For those which haven't I can add a custom eval prop as well, so they gonna behave the same as before.
Perfect. I was just checking if there are additional conflicts for this change. |
| @@ -15,7 +15,7 @@ | |||
| "at": "0x050b", | |||
| "cl": "0x0b04", | |||
| "ep": 0, | |||
| "eval": "if (Attr.val != -32768 && Attr.val != 32768) { Item.val = Attr.val; }" | |||
| "eval": "if (Attr.val != -32768 && Attr.val != 32768) { Item.val = Attr.val * R.item('state/power_multiplier').val / R.item('state/power_divisor').val; }" | |||
There was a problem hiding this comment.
- Might want to add
Math.round()to make sure the reported value is integer. - Signed
s16supports values from -32768 to 32767, soAttr.val != 32768is superfluous. Typically the lowest value, -32768 is used to indicate an invalid value.
Unsignedu16supports values from 0 to 65535, and uses for 65535 as invalid value.
There was a problem hiding this comment.
Signed s16 supports values from -32768 to 32767, so Attr.val != 32768 is superfluous.
Actually I didn't change that. This code is already in master. Do we need to clean it up?
There was a problem hiding this comment.
Yes, I saw that. Better cleanup while you’re touching it anyways. Sorry to put this on you.
There was a problem hiding this comment.
Might want to add Math.round() to make sure the reported value is integer.
The reported value is not required to be Integer - am I wrong? JavaScript Number(s) are always floating point. Normally it is up to the UI to take care about display and formatting.
While I was testing with the plug's, I already had problems reporting the standby demand (it was always 0W in Phoscon, while the RAW value was 4 in deCONZ). I can't tell if that's related to this specific code, but for sure if we reduce precision here, we throw away information and my end up not being able to show fractions.
EDIT: if we do not want to have float in the JSON, we need to make public alle fields of the 600 formatting block. Then the API client can decide how to deal with that information.
There was a problem hiding this comment.
Yes, I saw that. Better cleanup while you’re touching it anyways. Sorry to put this on you.
Because the issue is not only related to the files I am touching here, I would like to issue another PR for the cleanup only, to not mix up topics. (It should not matter if that one then gets merged before or after this one, since we can rebase)
There was a problem hiding this comment.
Since the cleanup was moved to a dedicated PR and doesn't block us here, is there anything which prevents us continuing? (I don't want to leave this stalled, since it already took multiple days of work)
If the calculation is fine, let's resolve this conversation.
Devices which may get affected by this change (if their formatting block contains bad values):
To prevent any possible breaking, we could add |
Since some measurement devices provide calculation factors to format measured values correctly, we need to make them available. These factors may change with each report to adapt to the current power consumption range.
(This prevents spamming the network when the "minimum change" property of the reporting binding is set to a low value.)
Based on the specifications it should be the default to use the multiplier/divisors in the active power calculation.
Devices that report wrong factors or values must workaround the misbehavior via custom DDF.
DDF configs may use these attributes to guarantee API values are always in the same unit (Watts):
Note
deCONZ defines its API units as Power in W, Consumption in wH, Current in mA
Example
Screenshot
Specifications