-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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 64-bit modbus sensor register reads #25672
Fix 64-bit modbus sensor register reads #25672
Conversation
4b5d90d
to
48a71a4
Compare
48a71a4
to
7c44081
Compare
All review comments addressed and checks pass. |
When reading four 16-bit modbus registers as a sensor value, slave output is stored first as 64-bit integer, but before returning that value is converted to double precision floating point. This causes rounding errors for integer values bigger than 2^53. After this change floating point conversion is done only if user has configured scaling or offset using floating points.
7c44081
to
88dc1b7
Compare
Please don't squash commits after review has started to make it easier for readers to track changes. |
Ok. I guess I am too used to using gerrit. |
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.
Looks good!
Can be merged when build passes. |
Description:
Previously when reading four 16-bit modbus registers as a sensor
value, slave output was stored first as 64-bit integer, but before returning
that value was converted to double precision floating point. This
caused rounding errors for integer values bigger than 2^53.
After this change floating point conversion is done only if user
has configured scaling or offset to use floating points. In the case
where scale and offset are integers (also the default) only integer
arithmetic is used and 64-bit register values will be read correctly.
This PR adds also unit tests for modbus sensor value conversions
that were missing completely.
Related issue (if applicable): fixes #25671
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#10043
Example entry for
configuration.yaml
(if applicable):Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest
.requirements_all.txt
by runningpython3 -m script.gen_requirements_all
..coveragerc
.If the code does not interact with devices: