bme280 maybe found some error

[quinte17]

hi. i myself wrote a bme driver (https://github.com/quinte17/bme280) too but with other interfaces. but thats not to be discussed here ;) the point is that your calculation of the calibrated data import maybe isnt that correct. i have:

    // H4 and H5 are a little bit tricky alligned.
    bme.calib.hum.H4 = int16(calib2[3])<<4 | int16(calib2[4]&0x0F)
    bme.calib.hum.H5 = int16(calib2[5])<<4 | int16(calib2[4]&0xF0)>>4

your version:

    d.c.h4 = int16(h[3])<<4 | int16(h[4])&0xF
    d.c.h5 = int16(h[4])&0xF0 | int16(h[5])<<4

also my functions do use float calculations. i dont know if they are 100% correct but at least i get reasonable values when measuring.

[maruel]

Good observation. I've reread page 23 and the relevant line is:

0xE5[7:4] / 0xE6   |   dig_H5 [3:0] / [11:4]   |   signed short

so you are right, h[4] needs to be shifted right when assigning to h5. I've fixed this in a commit I'll push soon, I'm taking the occasion to fix other issues in this module. Interestingly it didn't change the output in my test code at all, likely because these 4 bits were 0 on my test device. I haven't verified but will do later today.

[quinte17]

also here:

func (c *calibration) compensatePressureInt64(raw, tFine int32) uint32 {
    x := int64(tFine) - 128000
    y := x * x * int64(c.p6)
    y += (x * int64(c.p5)) << 17
    y += int64(c.p4) << 35
    x = (x*x*int64(c.p3))>>8 + ((x * int64(c.p2)) << 12)
x = ((int64(1)<<47 + x) * int64(c.p1)) >> 33 // <- HERE
    if x == 0 {
        return 0
    }
    p := ((((1048576 - int64(raw)) << 31) - y) * 3125) / x
    x = (int64(c.p9) * (p >> 13) * (p >> 13)) >> 25
    y = (int64(c.p8) * p) >> 19
    return uint32(((p + x + y) >> 8) + (int64(c.p7) << 4))
}

i think there is a brace error. this should be:

x = (int64(1)<<47 + x) * (int64(c.p1) >> 33)

[maruel]

Oops I forgot about the second issue, reopened and will investigate.

[maruel]

The original code is:

var1 = (((((BME280_S64_t)1)<<47)+var1))*((BME280_S64_t)dig_P1)>>33;

It is missing parenthesis for readability (but it's not like the author cared about readability at all) and is based on assumption of C's priority of operations. FWIU as I forgot, * has an higher priority than >> so my code is right. If I replace it with the line you propose, the function always return 0: c.p1>>33 is guaranteed to be zero since p1 is only 16 bits.

[quinte17]

thank you for clarifying.

[maruel]

Thank you for spotting the calibration bug in the first place!