Bosch Sensortec Community

    cancel
    Showing results for 
    Search instead for 
    Did you mean: 

    bug report : BME680 I2C error retries

    bug report : BME680 I2C error retries

    rpc
    Occasional Visitor

    Hi,

    I am Using BSEC 1.4.7.4 ( I have not tried the latest one, but I checked at the code and it didn't chagned) and using the bme680 API in github repository here https://github.com/BoschSensortec/BME680_driver.

    I didn't find a way to report this on github so here I am.

    I guess I have found a bug in the i2c retries in the "read_field_data" function, there is a retry mechanism put in place ( a do while loop 10 times), however the rslt return value from the bme680_get_regs function is not checked after this function. Thus in case of a failed read ( i2c nack let's say in my case), the rslt is not equal to OK, but the retry is never triggered due to the position of the "if" coundition on rslt.

     

    Here is the function :

    /*!
     * @brief This internal API is used to calculate the field data of sensor.
     */
    static int8_t read_field_data(struct bme680_field_data *data, struct bme680_dev *dev)
    {
    	int8_t rslt;
    	uint8_t buff[BME680_FIELD_LENGTH] = { 0 };
    	uint8_t gas_range;
    	uint32_t adc_temp;
    	uint32_t adc_pres;
    	uint16_t adc_hum;
    	uint16_t adc_gas_res;
    	uint8_t tries = 10;
    
    	/* Check for null pointer in the device structure*/
    	rslt = null_ptr_check(dev);
    	do {
    		if (rslt == BME680_OK) {
    			rslt = bme680_get_regs(((uint8_t) (BME680_FIELD0_ADDR)), buff, (uint16_t) BME680_FIELD_LENGTH,
    				dev);
    
    			data->status = buff[0] & BME680_NEW_DATA_MSK;
    			data->gas_index = buff[0] & BME680_GAS_INDEX_MSK;
    			data->meas_index = buff[1];
    
    			/* read the raw data from the sensor */
    			adc_pres = (uint32_t) (((uint32_t) buff[2] * 4096) | ((uint32_t) buff[3] * 16)
    				| ((uint32_t) buff[4] / 16));
    			adc_temp = (uint32_t) (((uint32_t) buff[5] * 4096) | ((uint32_t) buff[6] * 16)
    				| ((uint32_t) buff[7] / 16));
    			adc_hum = (uint16_t) (((uint32_t) buff[8] * 256) | (uint32_t) buff[9]);
    			adc_gas_res = (uint16_t) ((uint32_t) buff[13] * 4 | (((uint32_t) buff[14]) / 64));
    			gas_range = buff[14] & BME680_GAS_RANGE_MSK;
    
    			data->status |= buff[14] & BME680_GASM_VALID_MSK;
    			data->status |= buff[14] & BME680_HEAT_STAB_MSK;
    
    			if (data->status & BME680_NEW_DATA_MSK) {
    				data->temperature = calc_temperature(adc_temp, dev);
    				data->pressure = calc_pressure(adc_pres, dev);
    				data->humidity = calc_humidity(adc_hum, dev);
    				data->gas_resistance = calc_gas_resistance(adc_gas_res, gas_range, dev);
    				break;
    			}
    			/* Delay to poll the data */
    			dev->delay_ms(BME680_POLL_PERIOD_MS);
    		}
    		tries--;
    	} while (tries);
    
    	if (!tries)
    		rslt = BME680_W_NO_NEW_DATA;
    
    	return rslt;
    }

    Here is a suggested fix that don't take into account the result of the null_ptr_check

     

    --- a/bme680.c
    +++ b/bme680.c
    @@ -1222,10 +1222,10 @@ static int8_t read_field_data(struct bme680_field_data *data, struct bme680_dev
    /* Check for null pointer in the device structure*/
    rslt = null_ptr_check(dev);
    do {
    - if (rslt == BME680_OK) {
    - rslt = bme680_get_regs(((uint8_t) (BME680_FIELD0_ADDR)), buff, (uint16_t) BME680_FIELD_LENGTH,
    - dev);

    + rslt = bme680_get_regs(((uint8_t) (BME680_FIELD0_ADDR)), buff, (uint16_t) BME680_FIELD_LENGTH,
    + dev);
    + if (rslt == BME680_OK) {
    data->status = buff[0] & BME680_NEW_DATA_MSK;
    data->gas_index = buff[0] & BME680_GAS_INDEX_MSK;
    data->meas_index = buff[1];

    1 REPLY 1

    Vincent
    Community Moderator
    Community Moderator

    Here i have different opinion: 

    we have the following code, if at your side,  your read operation is return as NACK means no data read from sensor.  then data->status bit new data is not matching.  Then it will always stay in while loop for retry.

    If we read data successfully,  so we got new data mask as desired value,  we will calculate the temperature, pressure and humidity value,  then jump out the loop. 

    For this sequence,  i didn't see same bug you reported. 

    	if (data->status & BME680_NEW_DATA_MSK) {
    				data->temperature = calc_temperature(adc_temp, dev);
    				data->pressure = calc_pressure(adc_pres, dev);
    				data->humidity = calc_humidity(adc_hum, dev);
    				data->gas_resistance = calc_gas_resistance(adc_gas_res, gas_range, dev);
    				break;
    			}

     

    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