Skip to content

Corrected data type on S variable to avoid NaN and devices crashing above 200C #10

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dzalf
Copy link

@dzalf dzalf commented Jun 3, 2024

This PR addresses an issue when measuring temperatures above 200 C. Here, the getObjectTemp() method causes the sensor to report NaN values.

Even worse, when connecting multiple FIR devices (with different addresses) to the same I2C bus ALL of them would report NaN even if only one of them measured above 200C. Weird behaviour.

Our solution is on line 283 (278 in the original code):

// float S = (float)(lowerRAM + upperRAM) / 2.0;    // before
 double S = ((double)(lowerRAM * 1.0) + (double)(upperRAM * 1.0)) / 2.0;    // explicit conversion and casting to double

since the lowerRAM and upperRAM variables are declared as int16_t the sum seems to overflow. The overflow created a divide-by-zero scenario on the following operations. The solution that we found was by casting and forcing them as doubles.

We have successfully tested our solution without further crashing, however, the sensors report a maximum value of 245C, which is well beyond their rated range of 200C (and perhaps in the non-linear range or in the region where the polynomial function is not valid, however, we effectively eliminate the NaN on the current and other devices).

We believe that the rest of the devices connected to the same bus crashed too since a "Global Reset" communication is triggered (address 0x00 during the overflow, as specified by the datasheet on page 24:

image

Please try to carry out further tests and merge this PR if you don't find any conflicts.

Thanks, guys

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.

1 participant