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

Incorrect bme680 calibration register address #93

Open
Tom-Newton opened this issue Dec 6, 2020 · 3 comments
Open

Incorrect bme680 calibration register address #93

Tom-Newton opened this issue Dec 6, 2020 · 3 comments

Comments

@Tom-Newton
Copy link

This doesn't actually seem to cause any problems though I suspect if compiled in debug mode this would cause crashes due to trying to write to unallocated memory. I just spotted this when looking at the code.

The number of bme680 calibration constants:

kWarpSizesBME680CalibrationValuesCount = 41,
is not consistent with the range of addresses it attempts to read from:
kWarpSensorConfigurationRegisterBME680CalibrationRegion1Start = 0x89,
kWarpSensorConfigurationRegisterBME680CalibrationRegion1End = 0xA2,
kWarpSensorConfigurationRegisterBME680CalibrationRegion2Start = 0xE1,
kWarpSensorConfigurationRegisterBME680CalibrationRegion2End = 0xF2,

These constants are used here:

for ( reg = kWarpSensorConfigurationRegisterBME680CalibrationRegion1Start;
reg < kWarpSensorConfigurationRegisterBME680CalibrationRegion1End;
reg++)
{
status4 |= readSensorRegisterBME680(reg, 1 /* numberOfBytes */);
deviceBME680CalibrationValues[index++] = deviceBME680State.i2cBuffer[0];
}
for ( reg = kWarpSensorConfigurationRegisterBME680CalibrationRegion2Start;
reg < kWarpSensorConfigurationRegisterBME680CalibrationRegion2End;
reg++)
{
status4 |= readSensorRegisterBME680(reg, 1 /* numberOfBytes */);
deviceBME680CalibrationValues[index++] = deviceBME680State.i2cBuffer[0];
}
and this declaration is relavent
volatile uint8_t deviceBME680CalibrationValues[kWarpSizesBME680CalibrationValuesCount];

These for loops read from 42 different memory locations and write the contents to an array of length 41.

I believe the fix is to set this:

kWarpSensorConfigurationRegisterBME680CalibrationRegion2End = 0xF2,
to 0xf1.

Hopefully I haven't just completely misunderstood something here.

@JamesTimothyMeech
Copy link
Contributor

JamesTimothyMeech commented Jan 9, 2021

Hello Tom, sorry about the delay in getting back to you on this.

Looks like you're correct and there is a mistake here

Would you like to go ahead and make a pull request for this so you get credit for the fix?

Or would you prefer that I go ahead and do it?

@Tom-Newton
Copy link
Author

I'm happy to make a PR but I don't know any way that I could test it.

@JamesTimothyMeech
Copy link
Contributor

Ah I see what you mean. I'll make the change and test it sometime soon. Providing it doesn't break anything we can then make the PR and merge it in.

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

No branches or pull requests

2 participants