-
Notifications
You must be signed in to change notification settings - Fork 324
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
fix!: Fix always use number
for reportableChange
#1190
Conversation
int40
, uint48
and uint40
parsingnumber
for reportableChange
number
for reportableChange
number
for reportableChange
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.
Well, that fixes a leftover TODO I had marked when we moved/revamped the spec 👍
Probably should do the BigInt too (in buffalo.ts directly), if only to standardize the various parsers (currently inconsistent, as marked by the other TODO in buffaloZcl.ts). The n
-append notation (123n
) makes it easy enough to keep mostly plain numbers in the code (instead of BigInt(123)
).
Note if ever needed: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#use_within_json
@Nerivec Done! |
Made for a very nice cleanup in ZHC! |
reportableChange
can be anumber
or[number, number]
(to support larger ints). However sinceint40
,uint48
anduint40
easily fit in a NodeNumber.MAX_SAFE_INTEGER
there is no need to use[number, number]
.(u)int56 and (u)int64 could be supported through a
BigInt
, but in practice this will never be used.This fixes Koenkk/zigbee2mqtt#23711
TODO:
reportableChange
change zigbee-herdsman-converters#7996)