bme688 driver and example - #870
Conversation
|
Hello @hoanga thank for the addition. Can you please add a smoketest to https://github.com/tinygo-org/drivers/blob/dev/smoketest.sh |
|
Also please remove We only care about the human who submitted the PR, not the tools used. |
|
Hello, I have made updates per suggestions. Hopefully, I understood the request properly. I left the note in an (over)attempt to keep attributions however from the explanation regarding tooling, makes sense. If the PRs end up conflicting because of the smoketest add, I will can fix the aftermath depending on which PR goes in first. |
deadprogram
left a comment
deadprogram
left a comment
There was a problem hiding this comment.
Thanks for this driver. The structure follows the repo conventions well, and it keeps the same API shape as bme280. The calibration parse, calcResHeat and calcGasWait agree with the Bosch BME68x API.
I put the details in inline comments. Two of them explain why the gas sensor does not work: the driver asks the sensor for a high-range gas measurement, but it reads and calculates the low-range result. The other comments are smaller.
| adcG := uint16(f[13])<<2 | uint16(f[14]>>6) | ||
| gasRange := f[14] & GAS_RANGE_MSK |
There was a problem hiding this comment.
The gas result is always read from the low-range bytes. This is one of the two reasons the gas sensor does not work on a BME688.
Configure sets RUN_GAS_688 (bit 5), so the sensor makes a high-range measurement. But for variant 0x01 the Bosch API reads the gas ADC from field bytes 15 and 16, gas_range from byte 16, and the GASM_VALID and HEAT_STAB bits also from byte 16. Bytes 13 and 14 hold the low-range result only.
See read_field_data() in the Bosch BME68x API. The parse must select the byte pair from d.variantID, and lines 353 to 354 must use the same byte.
|
|
||
| // calcGasResistance returns gas resistance in Ohms. | ||
| // Formula from Bosch BME68x API using integer lookup tables. | ||
| func (d *Device) calcGasResistance(adcG uint16, gasRange byte) uint32 { |
There was a problem hiding this comment.
This is the low-range formula (calc_gas_resistance_low), but the BME688 needs the high-range one. This is the second reason the gas sensor does not work.
The Bosch API uses calc_gas_resistance_high when variant_id == 0x01:
var1 := uint32(262144) >> gasRange
var2 := (int32(adcG)-512)*3 + 4096
return uint32(1000000 * int32(var1) / var2)Please keep the low-range version for the BME680 and select on d.variantID. Then gasLookupTable1 and gasLookupTable2 stay necessary.
| // Gas calibration extras (coeff3 block, from 0x00) | ||
| d.calib.resHeatVal = int8(c3[0]) | ||
| d.calib.resHeatRange = (c3[2] & 0x30) >> 4 | ||
| d.calib.rangeSWErr = int8((c3[4] & 0xF0) >> 4) |
There was a problem hiding this comment.
range_sw_err is signed, but this shift is unsigned, so the top nibble does not sign-extend. The Bosch API does a signed shift.
d.calib.rangeSWErr = int8(c3[4]) >> 4This changes the low-range gas result only, but it is a transcription error.
| for i := 0; i < 10; i++ { | ||
| legacy.ReadRegister(d.bus, uint8(d.Address), REG_FIELD_0, field[:]) | ||
| if field[0]&NEW_DATA_MSK != 0 { | ||
| break | ||
| } | ||
| time.Sleep(5 * time.Millisecond) | ||
| } | ||
|
|
||
| return d.parseField(field), nil |
There was a problem hiding this comment.
If the new-data bit never becomes set, the loop ends and parseField runs on the still-zero field array. The caller then gets a nonsense temperature with a nil error.
Please return an error after the last try. At the moment Read never returns a non-nil error, so the error branch in the example is dead code.
| // Zero-value fields revert to the defaults described in Configure. | ||
| func (d *Device) ConfigureWithSettings(config Config) { | ||
| d.Config = config | ||
| if d.Config == (Config{}) { |
There was a problem hiding this comment.
The comment and the code do not agree. The test is for the complete struct, so the defaults apply only when every field is zero.
If a user sets Temperature: Sampling16X and nothing else, pressure and humidity oversampling become off and the gas heater becomes disabled, without a message. Please either give each field its own default, or correct the comment.
| runGasBit = RUN_GAS_680 | ||
| } | ||
| legacy.WriteRegister(d.bus, uint8(d.Address), REG_CTRL_GAS_1, | ||
| []byte{runGasBit | 0x00}) // nb_conv = 0 |
There was a problem hiding this comment.
| 0x00 has no effect. The comment gives the necessary information.
| return 16 | ||
| } | ||
|
|
||
| µs := overhead + |
There was a problem hiding this comment.
Please use an ASCII name here, for example us or durUS. The rest of the repo keeps identifiers ASCII.
| resHeatVal int8 | ||
| rangeSWErr int8 | ||
| // Running fine temperature value shared by T/P/H calculations | ||
| tFine int32 |
There was a problem hiding this comment.
tFine is per-measurement state, not calibration data. It also makes parseField write to d.calib, which is not obvious.
bme280 gives tFine to the calculation functions as a parameter. Please do the same here.
| const ( | ||
| CHIP_ID = 0x61 | ||
| VARIANT_BME680 = 0x00 | ||
| VARIANT_BME688 = 0x01 |
There was a problem hiding this comment.
These declarations have no user: VARIANT_BME688, GAS_MEAS_MSK, MEASURING_MSK, GAS_IDX_MSK, ModeSleep, and REG_MEAS_STATUS_0 (same address as REG_FIELD_0).
VARIANT_BME688 and ModeSleep become necessary if you add the high-range gas path. Please remove the others.
| connected := sensor.Connected() | ||
| if !connected { | ||
| println("BME688 not detected. Exiting...") | ||
| } | ||
|
|
||
| println("BME688 detected") |
There was a problem hiding this comment.
If the sensor does not answer, the example prints the message and then continues into the read loop. Please add a return.
2 participants
Hello!
This is a driver for the BME688 sensor that is included on the Pimoroni enviro indoor. I have tested this on an available enviro board and confirmed everything but the gas sensors are functional.
links:
https://github.com/pimoroni/enviro
https://shop.pimoroni.com/products/enviro-indoor