Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use power formatting factors for active power calculation #7978

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions devices/generic/items/state_power_divisor_item.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"schema": "resourceitem1.schema.json",
"id": "state/power_divisor",
"datatype": "UInt16",
"access": "R",
"public": false,
"description": "AC Power Divisor to be divided against the Instantaneous Power and Active Power attributes.",
"read": {
"at": "0x0605",
"cl": "0x0b04",
"ep": 0,
"fn": "zcl:attr"
},
"parse": {
"at": "0x0605",
"cl": "0x0b04",
"ep": 0,
"eval": "if (Attr.val > 0) { Item.val = Attr.val; }"
},
"refresh.interval": 300,
"default": 1
}
6 changes: 3 additions & 3 deletions devices/generic/items/state_power_item.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
"schema": "resourceitem1.schema.json",
"id": "state/power",
"datatype": "Int16",
"access": "RW",
"access": "R",
"public": true,
"description": "The measured power (in W).",
"description": "Active Power (in Watts). Positive values indicate power delivered, negative values indicate power received.",
"read": {
"at": "0x050b",
"cl": "0x0b04",
Expand All @@ -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; }"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Might want to add Math.round() to make sure the reported value is integer.
  • Signed s16 supports values from -32768 to 32767, so Attr.val != 32768 is superfluous. Typically the lowest value, -32768 is used to indicate an invalid value.
    Unsigned u16 supports values from 0 to 65535, and uses for 65535 as invalid value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I saw that. Better cleanup while you’re touching it anyways. Sorry to put this on you.

Copy link
Contributor Author

@bluemoehre bluemoehre Oct 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@bluemoehre bluemoehre Oct 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

#7984

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ebaauw bump

},
"refresh.interval": 300,
"default": 0
Expand Down
22 changes: 22 additions & 0 deletions devices/generic/items/state_power_multiplier_item.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"schema": "resourceitem1.schema.json",
"id": "state/power_multiplier",
"datatype": "UInt16",
"access": "R",
"public": false,
"description": "AC Power Divisor to be divided against the Instantaneous Power and Active Power attributes.",
"read": {
"at": "0x0604",
"cl": "0x0b04",
"ep": 0,
"fn": "zcl:attr"
},
"parse": {
"at": "0x0604",
"cl": "0x0b04",
"ep": 0,
"eval": "if (Attr.val > 0) { Item.val = Attr.val; }"
},
"refresh.interval": 300,
"default": 1
}