Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure calibration registers loaded before read #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

strongbutgood
Copy link

The calibration registers are read immediately on initialization of a BMP280 instance, however if the device itself has not completed power-on-reset these registers may not contain valid data. We should wait until the im_update bit from the status register is off before continuing (see section 4.3.3 of bst-bmp280-ds001.pdf for details).

I've tested the changes by forcing a reset (section 4.3.2) immediately before initializing BMP280 and found it takes between 1 and 3 passes through the loop before the update is complete. The following code was used in testing:

i2c = I2C(0, scl=Pin(1), sda=Pin(0))
# issue a reset directly to the bmp280 
i2c.writeto_mem(0x76, 0xE0, bytearray([0xB6]))
bmp = BMP280(i2c)

@dafvid
Copy link
Owner

dafvid commented Mar 15, 2022

The calibration data is not calculated on startup but burnt to memory at the factory. I think. So it's unique per chip and always present. It's the raw values from measuring that's sometimes missing during reading. I think compensating for sensor warm up is best handled outside the lib. It depends on your settings, specially oversampling. Also the potentially infinite loop is a bit of a high risk. Might lock the SoC totally on boot if chip is bad. So I'm a little hesitant to this change.

@strongbutgood
Copy link
Author

strongbutgood commented Mar 15, 2022

Good point re infinite loop, that could be an even more obscure rabbit hole than the issue with the calibration data itself.

The problem though is that reading the calibration data in the BMP280 initialization method, if it does so before the data is moved from non-volatile memory to the image registers will mean the instance never gets the correct values. It may be better to separate the reading of the calibration data to another method that can be called again should it fail to get the correct values during the initialization.

The datasheet specification has a max startup time of 2ms, from section 1:

Time to first communication after both VDD > 1.58V and VDDIO > 0.65V

But given my test communicates with the chip and reads is_updating as true indicates that the update of the image registers is triggered by the startup but completes at an indeterminate time after startup. From datasheet section 4.3.3:

Automatically set to ‘1’ when the NVM data are being copied to image registers and back to ‘0’ when the copying is done. The data are copied at power-on-reset and before every conversion.

An example method for reading the calibration data:

    def __init__(self, *):
        # ...setup i2c
        # init calibration data attributes
        self._T1 = self._T2 = ... = self._P9 = 0
        self.calibrate() # will refuse to update if not ready
        # ...setup other attributes

    def calibrate(self):
        if self._T1 != 0: # may need to be more thorough, but this is the first register that wouldn't have data
            return True
        if self.is_updating:
            return False
        # read calibration data
        # < little-endian
        # H unsigned short
        # h signed short
        self._T1, self._T2, self._T3, \
            self._P1, self._P2, self._P3, \
            self._P4, self._P5, self._P6, \
            self._P7, self._P8, self._P9 = unpack('<HhhHhhhhhhhh', self._read(0x88, 24))
        return True

And usage:

bmp = BMP280(I2C())
if not bmp.calibrate():
    print('Calibration data not yet available.')

Add confirmation that calibration data is read.
Refuse to read calibration data while registers are written from NVM.
Copy link
Owner

@dafvid dafvid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea to be able to "recalibrate". I was unaware of the registers sometimes having the wrong data. However I think the logic at lines 135-138 belongs outside of this function. Timings for the register data is too complicated to be asumed inside the functions. The application needs to check is_updating and the register values itself during runtime. The calibrate-function should only to just that and not handle too much logic for the user.

@strongbutgood
Copy link
Author

strongbutgood commented Mar 20, 2022

Timings for the register data is too complicated to be asumed inside the functions.

Do you mean that is_updating may change state after returning False and so the calibration read could continue successfully? Or that if the application called calibrate() at the same time as a measurement was just about to happen for example they might unnecessarily miss out on getting the values?

Really the only cases that need to be handled is on power-on-reset which occurs at power on, and immediately after receiving a reset register value (soft power on), with the latter case being initiated by the user. The chip does set the registers before a measurement so is_updating toggles at every read, but the values don't change over time so this case is not relevant. There really isn't a way to know that power on has completed definitively (as I understand the datasheet) except to put the chip in forced measurement mode, check that is_measuring is False, then check that is_updating is False. This being the case, I agree the users application needs to decide on if they check is_updating before calibrating, but then it would also be useful for the user would have to have some indication that calibration data is not valid.

@strongbutgood strongbutgood requested a review from dafvid March 20, 2022 10:48
@kr-g
Copy link

kr-g commented Jun 5, 2022

one remark.
following bosch specification (revision 1.26, section 3.10 Data register shadowing)

this should be covered in use case "normal mode" by shadowing the data registers.
so additional calibration is not required then (as far as i understood)

RFC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants