adafruit / Adafruit-BMP085-Library

A powerful but easy to use BMP085/BMP180 Arduino library
http://www.adafruit.com/products/1603
233 stars 194 forks source link

Wrong calculations on readings. #5

Closed sanelss closed 11 years ago

sanelss commented 12 years ago

Going through the datasheet and writing my own library for the chip and using this as a cross reference I have noticed that the temperature reading is incorrect. I would imagine this would make the others incorrect as well. In my specific case it's telling me the temperature is 15_c with this library and my version tells me it's 25.9_c. The calculation I did was pulled directly from the datasheet whereas in this library it's a different calculation.

From the library:

in one location its: // do temperature calculations X1 = ((UT - (int32_t)ac6) * (int32_t)ac5) >> 15; X2 = ((int32_t)mc << 11) - (X1 + md)/2; // round up X2 /= (X1 + md); B5 = X1 + X2; and in another location its: // step 1 X1 = ((UT - (int32_t)ac6) * (int32_t)ac5) >> 15; X2 = ((int32_t)mc << 11) / (X1 + (int32_t)md); B5 = X1 + X2; temp = (B5 + 8) >> 4; temp /= 10;

From my code:

x1=(ut-(int32_t)((uint16_t)barcalib[ac6]))((int32_t)((uint16_t)bar_calib[ac5]))/pow(2,15); x2=((int32_t)bar_calib[mc]_pow(2,11))/(x1+(int32_t)bar_calib[md]); b5=x1+x2; bar_data[temp]=(b5+8)/pow(2,4);

In my code i cast ac6 and ac5 into uint16_6 first since it's stored in an int16_t array then cast it to length. The way I calculated it is directly as stated in the datasheet unlike this library's code. If the temperature reading is wrong I would also imagine that pressure and altitude calculations would also be wrong.

Also ran sparkfun's listed library and it give's me the same reading as my code and they have:

x1 = (((long)ut - (long)ac6)*(long)ac5) >> 15; x2 = ((long)mc << 11)/(x1 + md); b5 = x1 + x2;

return ((b5 + 8)>>4);

microbuilder commented 11 years ago

Fixed ... you're right that these were off. Thanks for brining it up.

keestux commented 10 years ago

Even though the issue is closed I want to bring it to your attention that the fix is very much debatable. Using pow() is a bad choice since the operation can be done with shifting. Notice that pow() is a floating point function.

And, I can't see what is wrong with the original code. The Sparkfun code is very much the same and yet it is said that that code works as expected.

To conclude, the patch was not needed and only brings less efficient code.

tdicola commented 10 years ago

Thanks for the suggestion--I just committed https://github.com/adafruit/Adafruit-BMP085-Library/commit/4466c2651ce35474b63f2f7e8ea953fac638c426 which switches to bit shifts instead of pow. Tested on an Arduino and the results were consistent with the previous pow-based code.