-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Update frient powermeter led 2 #8044
Conversation
fe3fa6f
to
bbb37da
Compare
@Koenkk I have tested the integration locally, and it works, at least I get energy etc. from the device {
"battery": 100,
"battery_low": false,
"energy": 118.28,
"interface_mode": "electricity",
"linkquality": 112,
"power": 421,
"power_2": 357,
"pulse_configuration": 1000,
"update": {
"installed_version": 196866,
"latest_version": 196866,
"state": "idle"
},
"voltage": 3100,
"current_summation": null,
"update_available": null
} |
src/converters/fromZigbee.ts
Outdated
const multiplier = msg.endpoint.getClusterAttributeValue('seMetering', 'multiplier') as number; | ||
const divisor = msg.endpoint.getClusterAttributeValue('seMetering', 'divisor') as number; | ||
const multiplier = (msg.endpoint.getClusterAttributeValue('seMetering', 'multiplier') ?? 1) as number; | ||
const divisor = (msg.endpoint.getClusterAttributeValue('seMetering', 'divisor') ?? 1000) as number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Koenkk one thought here, could it be an idea to convert this to Number, instead of doing as number
?
like this instead
const divisor = Number(msg.endpoint.getClusterAttributeValue('seMetering', 'divisor') ?? 1000);
(and likewise for the multiplier of course)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is always a number (or undefined
) so it should not be needed. Thinking about this again, I'm not sure about these default values, why did you add them? By default it just publishes the raw value in case there is not multiplier/divisor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had problems, because they where undefined, and so factor became null. Then on line 756, it checks if factor is not null, before calculating anything. That lead up to it reporting null as energy consumption.
When I added those default values, it was reporting like it should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The divisor/multiplier can be forced in the configure. I would propose to have the same logic as instantaneousDemand
here, expose raw value when no factor is defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm.. even though that I defined the metering entity like
electricityMeter({cluster: 'metering', power: {divisor: 1000, multiplier: 1}, energy: {divisor: 1000, multiplier: 1}}),
I had trouble in the fromZigbee.metering function. This was the previous release though, so things might have changed now.
Anyways, I have changed the logic a bit, so it now returns the raw value.. I'll get around to test things in a couple of days.
Co-authored-by: Koen Kanters <koenkanters94@gmail.com>
src/lib/develco.ts
Outdated
@@ -170,4 +170,26 @@ export const develcoModernExtend = { | |||
valueIgnore: [0xffff, -0x8000], | |||
...args, | |||
}), | |||
current_summation: (args?: Partial<NumericArgs>) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current_summation: (args?: Partial<NumericArgs>) => | |
currentSummation: (args?: Partial<NumericArgs>) => |
src/lib/develco.ts
Outdated
valueMax: 268435455, | ||
...args, | ||
}), | ||
pulse_configuration: (args?: Partial<NumericArgs>) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pulse_configuration: (args?: Partial<NumericArgs>) => | |
pulseConfiguration: (args?: Partial<NumericArgs>) => |
@Koenkk I have now tested the implementation, and it works. Both changing pulse counts, and setting current summation property. |
Thanks! |
This should fix Koenkk/zigbee2mqtt#19042