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

fix!: Fix always use number for reportableChange #1190

Merged
merged 3 commits into from
Sep 17, 2024
Merged

Conversation

Koenkk
Copy link
Owner

@Koenkk Koenkk commented Sep 15, 2024

reportableChange can be a number or [number, number] (to support larger ints). However since int40, uint48 and uint40 easily fit in a Node Number.MAX_SAFE_INTEGER there is no need to use [number, number].

uint48 max       = 281474976710655
MAX_SAFE_INTEGER = 9007199254740991

(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:

@Koenkk Koenkk requested a review from Nerivec September 15, 2024 20:40
@Koenkk Koenkk changed the title fix: Fix int40, uint48 and uint40 parsing fix: Fix always use number for reportableChange Sep 15, 2024
@Koenkk Koenkk changed the title fix: Fix always use number for reportableChange fix!: Fix always use number for reportableChange Sep 15, 2024
Copy link
Collaborator

@Nerivec Nerivec left a 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

@Koenkk
Copy link
Owner Author

Koenkk commented Sep 16, 2024

@Nerivec Done!

@Nerivec
Copy link
Collaborator

Nerivec commented Sep 16, 2024

Made for a very nice cleanup in ZHC!

@Koenkk Koenkk merged commit e30d7b7 into master Sep 17, 2024
2 checks passed
@Koenkk Koenkk deleted the fix/remove-int40/48-sb branch September 17, 2024 20:19
Koenkk added a commit to Koenkk/zigbee-herdsman-converters that referenced this pull request Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

seMetering currentSummDelivered Reporting Configuration behaviour
2 participants