[BME280] Null Pointer Check prevents clean encapsulation

[maikwoehl]

Dear developers,

today I have tried to get the BME280 sensor in a AZDelivery package running. I have used the Espressif ESP-IDF platform to develop my code. I have noticed that the BME280 driver makes use of an intf_ptr for passing data into the read and write user functions. Since I use the configuration framework of the ESP-IDF platform I do not use this field and it will produce a null pointer exception, if you use that inside an abstraction layer. Otherwise you get a dangling pointer, if you do not define it globally.

I will show you an example. First of all I have defined a wrapper function for the initialization:

esp_err_t app_bme280_init(struct bme280_dev *dev) {
    /* initialize i2c controller */
    int8_t rslt = BME280_OK;

    uint8_t dev_addr = CONFIG_BME280_I2C_ADDRESS;
    dev->intf_ptr = &dev_addr;
    dev->intf = BME280_I2C_INTF;
    dev->read = user_i2c_read;
    dev->write = user_i2c_write;
    dev->delay_us = user_delay_us;

    rslt = bme280_init(dev);
    /* error handling and bme280 configuration */
}

 You can see that I have assigned a pointer to dev_addr to intf_ptr. That is unfortunately a wrong way, because dev_addr gets freed after function exit. I'm not so familiar with C programming, because I'm used to do StructuredText PLC programming, but with an oscilloscope and some time I have figured it out. That was a little bit embarrassing for me 😅.

It will led to a wrong address selection, when I use the intf_ptr to address the sensor over I2C:

int8_t user_i2c_read(uint8_t reg_addr, uint8_t *reg_data, uint32_t len, void *intf_ptr) {
    int8_t rslt = 0; /* Return 0 for Success, non-zero for failure */

    int i2c_port = I2C_PORT;
    uint8_t i2c_addr = *( (uint32_t*)intf_ptr );

    i2c_cmd_handle_t cmd = i2c_cmd_link_create();
    i2c_master_start(cmd);
    /* register selection */
    i2c_master_write_byte(cmd, i2c_addr << 1 | I2C_MASTER_WRITE, ACK_CHECK_EN);
    i2c_master_write_byte(cmd, reg_addr, ACK_CHECK_EN);
    i2c_master_start(cmd);
    /* read start command */
    i2c_master_write_byte(cmd, i2c_addr << 1 | I2C_MASTER_READ, ACK_CHECK_EN);
    i2c_master_read(cmd, reg_data, len, I2C_MASTER_LAST_NACK);
    i2c_master_stop(cmd);
    rslt = i2c_master_cmd_begin(
        i2c_port,
        cmd,
        1000 / portTICK_RATE_MS
    );
    i2c_cmd_link_delete(cmd);

    if (rslt != ESP_OK) {
        return rslt;
    }

    return rslt;
}

Of course, the intf_ptr points to a previously freed memory location.

I could map the intf_ptr in the calling scope to a valid memory location, that is the root scope indeed, but I want to prevent that. So I have to use a global variable in my setup to be able to have always a valid memory location for that pointer. This is just a prototyping project, but as it will get used in a bigger data acquisition experiment I want to hide the driver structs completely from the using developer in the root scope and just have a big application struct with some function pointers to trigger a temperature read with a high level function. For example that would be something nice:

int main(void) {
    acs_dev* acs = acs_init(ACS_THP_SENS | ACS_DIST_SENS);
    acs_app* app;
    app->init(acs);
    
    while (1) {
        printf("Temperature: %0.2f dC\n Humidity: %0.2f %%\n Pressure: %0.2f hPa | %0.2f bar",
               app->get_temperature(acs),
               app->get_humditiy(acs),
               app->get_pressure(acs) / 100,
               app->get_pressure(acs) / 100000,
        );
        
        vTaskDelay(10000 / portTICK_PERIOD_MS);
    }
}

It is all just personal stuff and not really relevant, but maybe someone will encounter the same issue while developing with the driver. Because I have tried to set that to NULL, because I don't need that. And then it has totally panicked in front of my eyes.

Maybe it is possible to exclude this pointer from a panicking null pointer check? It is not necessary, but would be nice as it does not need to be set to work properly, I think.

Best regards,

Maik Wöhl

[Minhwan · Accepted solution]

Hello Maik, 

Hope you to download COINES. 

https://www.bosch-sensortec.com/software-tools/tools/coines/

You can use intf_ptr using static varialbe as attached file. 

And also read and write functions you can use it as below. 

/*! This API is used to perform I2C read operation with sensor */
int8_t bmi2xy_hal_i2c_bus_read(uint8_t reg_addr, uint8_t *reg_data, uint32_t length, void *intf_ptr)
{
int8_t rslt = 0;
//uint8_t dev_id = 0x68;
uint8_t* dev_id = (uint8_t *)intf_ptr;

rslt = coines_read_i2c(*dev_id, reg_addr, reg_data, length);

return rslt;
}

Thank you. 

[maikwoehl]

Hello Minhwan,

thank you for your fast reply and the hint to the static variable.

I will look into the COINES library.

Regards,

Maik Wöhl