pasko-zh / brzo_i2c

Brzo I2C is a fast I2C Implementation written in Assembly for the esp8266
GNU General Public License v3.0
244 stars 47 forks source link

Migrate from Wire library #18

Closed jlohmoeller closed 7 years ago

jlohmoeller commented 7 years ago

I am failing to get this library working with a BME280 Environment Sensor. I moved from the Arduino Wire library to brzo_i2c since the AMS iAQ-Core air quality sensor did not work correctly with the builtin Wire lib. I took the code from Adafruit and changed the parts handling I2C communication.

Original Code:

void Adafruit_BME280::write8(byte reg, byte value) {
    if (_cs == -1) {
        Wire.beginTransmission((uint8_t)_i2caddr);
        Wire.write((uint8_t)reg);
        Wire.write((uint8_t)value);
        Wire.endTransmission();
    } else {
        if (_sck == -1)
            SPI.beginTransaction(SPISettings(500000, MSBFIRST, SPI_MODE0));
        digitalWrite(_cs, LOW);
        spixfer(reg & ~0x80); // write, bit 7 low
        spixfer(value);
        digitalWrite(_cs, HIGH);
    if (_sck == -1)
        SPI.endTransaction(); // release the SPI bus
    }
}

uint8_t Adafruit_BME280::read8(byte reg) {
    uint8_t value;

    if (_cs == -1) {
        Wire.beginTransmission((uint8_t)_i2caddr);
        Wire.write((uint8_t)reg);
        Wire.endTransmission();
        Wire.requestFrom((uint8_t)_i2caddr, (byte)1);
        value = Wire.read();
    } else {
        if (_sck == -1)
            SPI.beginTransaction(SPISettings(500000, MSBFIRST, SPI_MODE0));
        digitalWrite(_cs, LOW);
        spixfer(reg | 0x80); // read, bit 7 high
        value = spixfer(0);
        digitalWrite(_cs, HIGH);
        if (_sck == -1)
            SPI.endTransaction(); // release the SPI bus
    }
    return value;
}

uint16_t Adafruit_BME280::read16(byte reg)
{
    uint16_t value;

    if (_cs == -1) {
        Wire.beginTransmission((uint8_t)_i2caddr);
        Wire.write((uint8_t)reg);
        Wire.endTransmission();
        Wire.requestFrom((uint8_t)_i2caddr, (byte)2);
        value = (Wire.read() << 8) | Wire.read();
    } else {
        if (_sck == -1)
            SPI.beginTransaction(SPISettings(500000, MSBFIRST, SPI_MODE0));
        digitalWrite(_cs, LOW);
        spixfer(reg | 0x80); // read, bit 7 high
        value = (spixfer(0) << 8) | spixfer(0);
        digitalWrite(_cs, HIGH);
        if (_sck == -1)
            SPI.endTransaction(); // release the SPI bus
    }

    return value;
}

uint32_t Adafruit_BME280::read24(byte reg)
{
    uint32_t value;

    if (_cs == -1) {
        Wire.beginTransmission((uint8_t)_i2caddr);
        Wire.write((uint8_t)reg);
        Wire.endTransmission();
        Wire.requestFrom((uint8_t)_i2caddr, (byte)3);

        value = Wire.read();
        value <<= 8;
        value |= Wire.read();
        value <<= 8;
        value |= Wire.read();
    } else {
        if (_sck == -1)
            SPI.beginTransaction(SPISettings(500000, MSBFIRST, SPI_MODE0));
        digitalWrite(_cs, LOW);
        spixfer(reg | 0x80); // read, bit 7 high

        value = spixfer(0);
        value <<= 8;
        value |= spixfer(0);
        value <<= 8;
        value |= spixfer(0);

        digitalWrite(_cs, HIGH);
        if (_sck == -1)
            SPI.endTransaction(); // release the SPI bus
    }

    return value;
}

My adapted version: (except leaving out the SPI stuff and Wire.begin() the rest of the code has not changed)

#define I2C_FREQ 100
brzo_i2c_setup(4,5,10000);
i2c_buffer = (uint8_t*)os_malloc(10);
//...
void BME280::write8(byte reg, byte value) {
    i2c_buffer[0] = reg;
    i2c_buffer[1] = value;
    brzo_i2c_start_transaction((uint8_t)_i2caddr, I2C_FREQ);
    brzo_i2c_write(i2c_buffer, 1, false);
    brzo_i2c_write(&i2c_buffer[1], 1, false);
    uint8_t error = brzo_i2c_end_transaction();
    Serial.print("write8  W: ");Serial.print(reg, HEX);Serial.print(", R: ");Serial.print(value, HEX);
    Serial.print(" (");Serial.print(value, BIN);Serial.print("), i2c: "); Serial.println(error);
}

uint8_t BME280::read8(byte reg) {
    i2c_buffer[0] = reg;
    brzo_i2c_start_transaction((uint8_t)_i2caddr, I2C_FREQ);
    brzo_i2c_write(i2c_buffer, 1, false);
    brzo_i2c_read(&i2c_buffer[1], 1, false);
    uint8_t error = brzo_i2c_end_transaction();
    Serial.print("read8  W: ");Serial.print(reg, HEX);Serial.print(", R: ");Serial.print(i2c_buffer[1], HEX);
    Serial.print(" (");Serial.print(i2c_buffer[1], BIN);Serial.print("), i2c: "); Serial.println(error);
    return i2c_buffer[1];
}

uint16_t BME280::read16(byte reg)
{
    uint16_t value;
    i2c_buffer[0] = reg;
    brzo_i2c_start_transaction((uint8_t)_i2caddr, I2C_FREQ);
    brzo_i2c_write(i2c_buffer, 1, false);
    brzo_i2c_read(&i2c_buffer[1], 2, false);
    uint8_t error = brzo_i2c_end_transaction();
    value = i2c_buffer[1]<<8 | i2c_buffer[2];
    Serial.print("read16 W: ");Serial.print(reg, HEX);Serial.print(", R: ");Serial.print(value, HEX);Serial.print(", i2c: "); Serial.println(error);
    return value;
}

uint32_t BME280::read24(byte reg)
{
    uint32_t value = 0;
    i2c_buffer[0]=reg;
    brzo_i2c_start_transaction((uint8_t)_i2caddr, I2C_FREQ);
    brzo_i2c_write(i2c_buffer, 1, false);
    brzo_i2c_read(&i2c_buffer[1], 3, false);
    uint8_t error = brzo_i2c_end_transaction();
    value = ((uint32_t)i2c_buffer[1])<<16 | ((uint16_t)i2c_buffer[2])<<8 | i2c_buffer[3];
    Serial.print("read24 W: ");Serial.print(reg, HEX);Serial.print(", R: ");Serial.print(value, HEX);Serial.print(", i2c: "); Serial.println(error);
    return value;
}

I get the following output when requesting the registers holding measured data:

write8  W: F4, R: 51 (1010001), i2c: 0   //(1) bme280.takeForcedMeasurement();
read8  W: F3, R: 0 (0), i2c: 0
read24 W: FA, R: 800000, i2c: 0          //bme280.readTemperature();
read24 W: FA, R: 800000, i2c: 0          //bme280.readPressure();
read24 W: F7, R: 800000, i2c: 0          
read24 W: FA, R: 800000, i2c: 0          //bme280.readHumidity();
read16 W: FD, R: 8000, i2c: 0
Temp: nan *C    Pressure: nan hPa   Humidity: nan % // Calculated

For me it seems like reading works, but the BME280 does not measure and hence does not fill its registers with correct values. According to the datasheet 800000 is the default value. Writing (1) should force measurement and after this the registers should be filled with data. I guess there is something wrong in the write8(...) method.

If I change back to wire (by copy-pasting the methods shown above), the BME280 works fine (so there is no hardware fault)...

Maybe an example in the Wiki comparing Arduinos Wire-lib and brzo_i2c would be helpful :)

pasko-zh commented 7 years ago

Well, we have three different levels: The Adafruit Library and the underlaying wire Library and last but not least the i2c communication of the sensor. So, debugging means understanding all three of those levels. I had very quick look at the Adafruit Library and the spec of the bosch sensor and then your modified parts. It's hard to tell what is going wrong, especially since I don't have the sensor, so I cannot check i2c timings with the scope. I will have a further look at it, probably next week. ps: When "migrating" from wire Library to brzo_i2c, I would start with having a look at the i2c communication of the sensor and not the actual calls to the wire Library. So, some of the sensors need clock stretching, burst read or .... So, it is this combination of i2c read/writes and the timing which is the crucial thing.

pasko-zh commented 7 years ago

Hmmm, without the i2c device it is hard to tell where the problem is...

The only thing I could think of: According to the datasheet, section 6.2.1 the i2c master should send: START, Slave address, register address, register data, STOP. Now, in your code you are writing register and register data seperately, i.e. with two brzo_i2c_write calls. The first call brzo_i2c_write(i2c_buffer, 1, false); will send a STOP at the end. So, the i2c device "sees" this as the end of a write transaction, cf. figure 9 in the datasheet. So, I would combine the writes and thus avoiding the STOP between register address and register data.

I would do brzo_i2c_write(i2c_buffer, 2, false)

jlohmoeller commented 7 years ago

Thank you for your answer, brzo_i2c_write(i2c_buffer, 2, false) did the trick. I assumed the STOP is sent when transmission is closed :(

pasko-zh commented 7 years ago

@jlohmoeller : I've added a wiki section on "How to migrate from Wire Library", see here