Skip to content

Conversation

@keith-packard
Copy link
Contributor

FixPoint1616_t is typedef'd to uint32_t. The result of dividing two variables of that type is also unsigned. Passing an unsigned value to abs doesn't make sense.

Cast the value to int32_t in case the computaton of diff1_mcps generates a negative result so that xTalkCorrection doesn't end up with a huge value.

@erwango
Copy link
Member

erwango commented Mar 31, 2023

I agree with the change, and I'll report it internally.
Then, we need to make sure this won't be lost if library is updated to a later version which doesn't include the fix.

Would you mind adding a note in the component README file (sensor/vl53l0x/README) indicating the change that was made and the file impacted (similarly to what is done here) ?

FixPoint1616_t is typedef'd to uint32_t. The result of dividing two
variables of that type is also unsigned. Passing an unsigned value to
abs doesn't make sense.

Cast the value to int32_t in case the computaton of diff1_mcps
generates a negative result so that xTalkCorrection doesn't end up
with a huge value.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
Copy link
Contributor Author

Would you mind adding a note in the component README file (sensor/vl53l0x/README) indicating the change that was made and the file impacted (similarly to what is done here) ?

That's a good plan. Done.

@erwango
Copy link
Member

erwango commented Apr 3, 2023

That's a good plan. Done.

Thanks. Merging

@erwango erwango merged commit 60b149c into zephyrproject-rtos:master Apr 3, 2023
@keith-packard keith-packard deleted the unsigned-abs branch April 3, 2023 16:41
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.

3 participants