Bosch Sensortec Community

    cancel
    Showing results for 
    Search instead for 
    Did you mean: 
    SOLVED

    [BME280] Null Pointer Check prevents clean encapsulation

    [BME280] Null Pointer Check prevents clean encapsulation

    maikwoehl
    New Poster

    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

    2 REPLIES 2

    Minhwan
    Community Moderator
    Community Moderator

    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. 

     

     

    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

    Icon--AD-black-48x48Icon--address-consumer-data-black-48x48Icon--appointment-black-48x48Icon--back-left-black-48x48Icon--calendar-black-48x48Icon--center-alignedIcon--Checkbox-checkIcon--clock-black-48x48Icon--close-black-48x48Icon--compare-black-48x48Icon--confirmation-black-48x48Icon--dealer-details-black-48x48Icon--delete-black-48x48Icon--delivery-black-48x48Icon--down-black-48x48Icon--download-black-48x48Ic-OverlayAlertIcon--externallink-black-48x48Icon-Filledforward-right_adjustedIcon--grid-view-black-48x48IC_gd_Check-Circle170821_Icons_Community170823_Bosch_Icons170823_Bosch_Icons170821_Icons_CommunityIC-logout170821_Icons_Community170825_Bosch_Icons170821_Icons_CommunityIC-shopping-cart2170821_Icons_CommunityIC-upIC_UserIcon--imageIcon--info-i-black-48x48Icon--left-alignedIcon--Less-minimize-black-48x48Icon-FilledIcon--List-Check-grennIcon--List-Check-blackIcon--List-Cross-blackIcon--list-view-mobile-black-48x48Icon--list-view-black-48x48Icon--More-Maximize-black-48x48Icon--my-product-black-48x48Icon--newsletter-black-48x48Icon--payment-black-48x48Icon--print-black-48x48Icon--promotion-black-48x48Icon--registration-black-48x48Icon--Reset-black-48x48Icon--right-alignedshare-circle1Icon--share-black-48x48Icon--shopping-bag-black-48x48Icon-shopping-cartIcon--start-play-black-48x48Icon--store-locator-black-48x48Ic-OverlayAlertIcon--summary-black-48x48tumblrIcon-FilledvineIc-OverlayAlertwhishlist