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.

Sign up for GitHub

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

Jump to bottom

drivers: lps25hb driver returning wrong fractional part of a pressure #46615

Closed
karold2412 opened this issue Jun 16, 2022 · 1 comment · Fixed by #72681
Closed

drivers: lps25hb driver returning wrong fractional part of a pressure #46615

karold2412 opened this issue Jun 16, 2022 · 1 comment · Fixed by #72681
Assignees
@avisconti
Labels
area: Sensors Sensors Enhancement Changes/Updates/Additions to existing features

Comments

@karold2412
Copy link

During my work with lps25hb pressure sensor I have faced problem that fractional part of the measurement is completely wrong. During debug I figured out that we are probably overflowing int32_t range during computations in this function:
lps25hb_problem
Since this sensor is returning 24 bit wide result - when we multiply it million times - it will clearly overflow.

So my local solution for now is:
suggested_change
To give compiler the idea that we need 64 bit value in order to proceed with calculations. As far as I have tested it - it works like a charm.

What do you guys think? Is it a good solution? Should I create a pull request for that?
@karold2412 karold2412 added the Enhancement Changes/Updates/Additions to existing features label Jun 16, 2022
@avisconti
Copy link
Collaborator

@karold2412 Sorry, I'm looking now at very old issues. If you still work on zephyr you may create and submit a PR. Otherwise I'll do it. Thanks

avisconti added a commit to avisconti/zephyr that referenced this issue May 13, 2024
@avisconti
drivers/sensor: lps25hb: Fix int32 overflow in the val2 part
77dda0d 
The val2 calculation was done using (1000000 / 40960) as
multiplying factor, which was sometimes leading to a
int32 overflow. So, let's use the equivalent (but smaller)
(3125 / 128).

Fix zephyrproject-rtos#46615

Signed-off-by: Armando Visconti <armando.visconti@st.com>
@avisconti avisconti mentioned this issue May 13, 2024
Merged
aescolar pushed a commit that referenced this issue May 15, 2024
@avisconti @aescolar
drivers/sensor: lps25hb: Fix int32 overflow in the val2 part
8e13268 
The val2 calculation was done using (1000000 / 40960) as
multiplying factor, which was sometimes leading to a
int32 overflow. So, let's use the equivalent (but smaller)
(3125 / 128).

Fix #46615

Signed-off-by: Armando Visconti <armando.visconti@st.com>
ycsin pushed a commit to ycsin/zephyr that referenced this issue May 17, 2024
@avisconti @ycsin
drivers/sensor: lps25hb: Fix int32 overflow in the val2 part
497a77f 
The val2 calculation was done using (1000000 / 40960) as
multiplying factor, which was sometimes leading to a
int32 overflow. So, let's use the equivalent (but smaller)
(3125 / 128).

Fix zephyrproject-rtos#46615

Signed-off-by: Armando Visconti <armando.visconti@st.com>
mariopaja pushed a commit to mariopaja/zephyr that referenced this issue May 26, 2024
@avisconti
drivers/sensor: lps25hb: Fix int32 overflow in the val2 part
f9beb8e 
The val2 calculation was done using (1000000 / 40960) as
multiplying factor, which was sometimes leading to a
int32 overflow. So, let's use the equivalent (but smaller)
(3125 / 128).

Fix zephyrproject-rtos#46615

Signed-off-by: Armando Visconti <armando.visconti@st.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@avisconti avisconti

Labels
area: Sensors Sensors Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

drivers/sensor: lps25hb: Fix int32 overflow in the val2 part avisconti/zephyr
3 participants
@avisconti @henrikbrixandersen @karold2412