sparkfun / BMP180_Breakout

Breakout board for the Bosch BMP180 barometric pressure sensor
Other
69 stars 101 forks source link

Possible error in ReadInt #1

Closed geoffreymbrown closed 9 years ago

geoffreymbrown commented 10 years ago

The code is:

char SFE_BMP180::readInt(char address, int &value) // Read a signed integer (two bytes) from device // address: register to start reading (plus subsequent register) // value: external variable to store data (function modifies value) { unsigned char data[2];

    data[0] = address;
    if (readBytes(data,2))
    {
            value = (((int)data[0]<<8)|(int)data[1]);
            //if (*value & 0x8000) *value |= 0xFFFF0000; // sign extend if negative
            return(1);
    }
    value = 0;
    return(0);

}

I believe the line writing value should be

value = (int) ((data[0] << 8) | (data[1]));

Certainly the cast ((int) data[1]) is wrong -- if the msb of data[1] is 1, then this cast will sign extend which you don't want.

tytower commented 9 years ago

Nobody is employed to correct these I guess . Jan 2014? So is the above correct or what?

tytower commented 9 years ago

I made the change and my BMP180 being read by an 80 Mhz ESP8622 started to read properly again. It was reading 100 degrees c instead of 20 and minus thousands for pressure instead of 30 hg. It read fine on the 18Mhz ATmega328p as it was Thanks for the find!

tytower commented 9 years ago

I also see an identical line in char SFE_BMP180::readUInt(char address, uint16_t &value) Should that be changed also?

ToniCorinne commented 9 years ago

The issue has been fixed. Please keep in mind that we have a large number of repos and a small number of people looking at them at any given time. In the future, if you find a bug you'd like us to address, please consider using the pull request system used in git/GitHub. It makes us easier to debug possible problems, and allows you to get credit for the patch. It also speeds up the process of us checking suggested fixes.

tytower commented 9 years ago

So geoffreymbrown I guess this means Thank You very much but Sparkfun are not of a pursuasion or upbringing to say so directly but I do Thanks

mgrusin commented 9 years ago

Toni, did you happen to test the requested fix on a BMP180 connected to a standard Arduino? I am worried that this "fix" may have broken it.

tytower, just so you know, the existing code works(ed) correctly on the Arduino because it was coded to the datatype widths present on that platform. The reason it broke on your system is because your system uses different datatype widths. A better fix to this problem is to specify overt widths for all library variables (d_typeXX) to support other architectures, which is something that wasn't an issue when the library was written. I also believe the suggested fix is incorrect, but I'm glad it works for you. If you eventually run into weird values on your system, you'll know where they came from.

ToniCorinne commented 9 years ago

@mgrusin I have it up and running in my office if you'd like to come check it out.

mgrusin commented 9 years ago

Thanks Toni.

tytower commented 9 years ago

Appreciate the reply thanks Mike ----------------------------------------------------------------------On Tue, 02 Jun 2015 15:11:55 -0700 Mike Grusin notifications@github.com wrote:

Toni, did you happen to test the requested fix on a BMP180 connected to a standard Arduino? I am worried that this "fix" may have broken it.

tytower, just so you know, the existing code works(ed) correctly on the Arduino because it was coded to the datatype widths present on that platform. The reason it broke on your system is because your system uses different datatype widths. A better fix to this problem is to specify overt widths for all library variables (d_typeXX) to support other architectures, which is something that wasn't an issue when the library was written. I also believe the suggested fix is incorrect, but I'm glad it works for you. If you eventually run into weird values on your system, you'll know where they came from.


Reply to this email directly or view it on GitHub: https://github.com/sparkfun/BMP180_Breakout/issues/1#issuecomment-108113279

Yahoo tytower@yahoo.com

tytower commented 9 years ago

Sparkfun's librarys in the new Arduino IDE

Your Sensor library MPL3115A2 contains an invalid character in the name and this causes 15 error windows every time Arduino is started .

It has to be fixed on your end as it is coming up with that name in the boards manager and can't be changed by me

Yahoo tytower@yahoo.com

tytower commented 9 years ago

NOT HAPPY SPARKFUN!

Sparkfun's librarys in the new Arduino IDE

Your Sensor library MPL3115A2 contains an invalid character in the name and this causes 15 error windows every time Arduino is started .

It has to be fixed on your end as it is coming up with that name in the boards manager and can't be changed by me

Yahoo tytower@yahoo.com

mgrusin commented 9 years ago

Thank you for the report, we will look into it.

In the meantime, you should be able to download and install the library manually.

tytower commented 9 years ago

I have had to re download the IDE complete and start again with libraries but I have still not found how to stop these windows 16 at a time and they started after I followed your instructions for adding a zip file? See the attaches screen shot 16 times every time i start the ide and a couple each time I restart the Serial terminal --Crazy

On Thu, 04 Jun 2015 18:22:57 -0700 Mike Grusin notifications@github.com wrote:

Thank you for the report, we will look into it.

In the meantime, you should be able to download and install the library manually.


Reply to this email directly or view it on GitHub: https://github.com/sparkfun/BMP180_Breakout/issues/1#issuecomment-109114535

Yahoo tytower@yahoo.com