bmi270: add driver for BMI270 6-axis IMU - #874
Conversation
Add I2C driver for the Bosch BMI270 inertial measurement unit, supporting configurable accelerometer (±2g/4g/8g/16g) and gyroscope (±125/250/500/1000/2000 dps) ranges. Includes: - Register definitions (registers.go) - Driver implementation with go:embed for config firmware (bmi270.go) - BMI270 config binary data (bmi270-config.bin) - Example usage (examples/bmi270/main.go)
deadprogram
left a comment
There was a problem hiding this comment.
Thanks for this driver. The structure follows the repo conventions and the register map agrees with the datasheet (CHIP_ID 0x24, INIT_ADDR nibble/byte split). I checked all the scale factors and they are correct:
- accel: 2G 61, 4G 122, 8G 244, 16G 488 microG/LSB
- gyro: 2000 60976, 1000 30488, 500 15244, 250 7622, 125 3811 micro-dps/LSB
The worst case product 32767 * 60976 = 1.998e9 stays inside int32, so there is no overflow.
I put some comments in the code. The most important one is the 8 KB RAM copy of the config blob.
Some smaller items:
- The ODR and the bandwidth are constant (ACC_CONF and GYR_CONF = 0xA8, 100 Hz). This is satisfactory for a first version, but a Config field could be useful later.
- The example mixes println and fmt.Printf. fmt adds much flash usage on small targets.
- The PR text says that you did the test on an M5Stack Core 1.3, but the smoketest builds for m5stack-core2. Please make sure that the pins in the example are correct for the target that the CI builds.
| "tinygo.org/x/drivers" | ||
| ) | ||
|
|
||
| //go:embed bmi270-config.bin |
There was a problem hiding this comment.
Please add a comment that gives the source of this binary and its license status. The bma42x driver does this for its config blobs (see bma42x/bma42x.go lines 29-37). This blob is a Bosch binary and it has no attribution now.
| } | ||
| time.Sleep(1 * time.Millisecond) | ||
|
|
||
| configBytes := []byte(bmi270ConfigData) |
There was a problem hiding this comment.
This makes an 8 KB copy of the firmware in RAM. The bma42x driver does special work to prevent this (unsafeStringToSlice, bma42x/bma42x.go lines 158-162).
Here the correction is easier, because you copy the chunk into wbuf. The copy built-in accepts a string source:
for i := 0; i < len(bmi270ConfigData); i += chunkSize {
...
n := copy(d.wbuf[1:], bmi270ConfigData[i:end])Then you can remove the configBytes variable.
There was a problem hiding this comment.
Updated to copy directly from the string using copy(), eliminating the 8 KB RAM copy. Thanks!
f8a25fb#diff-17f100c3d4763c5385c199aedcb9357aa29b378c1d6a6dcb10ccb40742c92d25R105-R122
| if err != nil { | ||
| return err | ||
| } | ||
| if status == 0x01 { |
There was a problem hiding this comment.
This comparison is too strict. It is not equal to 0x01 if axes_remap_error (bit 5) or odr_50hz_error (bit 6) is set, although the message field (bits [2:0]) shows init_ok. See datasheet section 5.3.2.
Use status&0x07 == 0x01.
| } | ||
|
|
||
| func (d *Device) Configure(config Config) error { | ||
| if config.AccelRange != 0 { |
There was a problem hiding this comment.
Accel2G and Gyro2000DPS are both 0x00, thus config.AccelRange != 0 and config.GyroRange != 0 are always false for the default values and the else branches do the same thing. You can assign the two fields directly.
| return err | ||
| } | ||
|
|
||
| var rangeVal byte |
There was a problem hiding this comment.
The AccelRange constants are equal to the register values, thus this switch is an identity map. You can write byte(d.accelRange). The same is applicable to the gyro switch below.
| reg_GYR_CONF = 0x42 | ||
| reg_GYR_RANGE = 0x43 | ||
| reg_INIT_CTRL = 0x59 | ||
| reg_INIT_ADDR_0 = 0x5B |
There was a problem hiding this comment.
reg_INIT_ADDR_1 is not used, because the 2-byte write to INIT_ADDR_0 increments the address automatically. This is satisfactory, but a short comment can help the next reader.
- Add Bosch Sensortec source/license attribution for the embedded bmi270-config.bin firmware blob. - Avoid an 8 KB RAM copy of the config blob by copying chunks directly from the embedded string into wbuf. - Check only the message field of INTERNAL_STATUS (status&0x07 == 0x01) per datasheet section 5.3.2, instead of the full register value. - Drop redundant AccelRange/GyroRange zero checks in Configure, since Accel2G and Gyro2000DPS are both 0x00. - Replace the identity-map switch blocks with direct byte() casts. - Add a comment on reg_INIT_ADDR_1 noting that writing 2 bytes to reg_INIT_ADDR_0 automatically updates it. - Example: use println with integer mg/mdps values instead of fmt.Printf to reduce flash usage on small targets.
|
Thank you for the feedback! I have updated the PR to address all the review comments and verified that it works as expected on the M5Stack Core2 v1.3. |
Sorry, the PR text was wrong. I use M5Stack Core2 v1.3, and fixed the text. |
Summary
Add I2C driver for the Bosch BMI270 6-axis inertial measurement unit.
Features
Files
Testing
test M5Stack Core 2 v1.3
the test program is below:
https://github.com/t-morisawa/m5stack-tinygo/blob/668ff91/tinygo/bmi270-monitor/main.go
Closes #872