lsm303agr: accelerometer not working as expected with microbit V1.5 and V2.2

[breml]

Today I tried the lsm303agr driver from latest release (tinygo.org/x/drivers v0.23.0) and the accelerometer did not provide the expected values. I used the the code from the example as a foundation.

After some debugging, I found the code in https://github.com/tinygo-org/drivers/issues/125#issuecomment-647614939, which worked for me as expected. To me, it looks like the difference is on line https://github.com/tinygo-org/drivers/blob/release/lsm303agr/lsm303agr.go#L140 where the current driver has

ReadRegister(uint8(d.AccelAddress), ACCEL_OUT_X_L_A, data)

and the working code from the before mentioned issue comment uses:

ReadRegister(LSM303_ADDRESS_ACCEL, LSM303_REGISTER_ACCEL_OUT_X_L_A | 0x80, data)

The difference is the | 0x80. I had a look at the datasheet (https://www.st.com/resource/en/datasheet/lsm303agr.pdf), but I was not able to figure out, what the effect of setting the most significant bit with | 0x80 is.

I made an exact copy of the lsm303agr driver in my project and only added | 0x80, which fixed the accelerometer for me on both, microbit V1.5 and V2.2.

Additionally, I also don't know, if it is neccessary to set this bit for the reading of the magnetometer and the temperature is necessary as well.

[breml]

polite CC: @ysoldak, @alankrantas, @conejoninja

[ysoldak]

Hm, interesting. Seem to be my bad, a typo in a refactoring PR.

Could you please try and checkout either "dev" branch or "v0.23.0" tag and replace ACCEL_OUT_X_L_A with ACCEL_OUT_X_H_A on line 140?

Same for magnetometer, line 193: MAG_OUT_X_L_M -> MAG_OUT_X_H_M And temperature, line 223: OUT_TEMP_L_A -> OUT_TEMP_H_A

I don't have IMU myself, can't test.

[breml]

@ysoldak I quickly tested what you suggested for the accelerometer (line 140) and this does not work.

The value for the constant ACCEL_OUT_X_L_A is 0x28 and for ACCEL_OUT_X_H_A it is 0x29.

As mentioned in the issue, the fix that does work is, if I change the value to ACCEL_OUT_X_L_A|0x80, resulting in 0xA8.

[breml]

The example in https://github.com/tinygo-org/drivers/issues/125#issuecomment-647614939 does refer to the Adafruit implementation, which does use the | 0x80 as well (see: https://github.com/adafruit/Adafruit_LSM303/blob/master/Adafruit_LSM303.cpp#L40). But I have not found any documentation on why this is necessary. I can just confirm on my two microbit devices, that this does the trick.

[ysoldak]

I see, yes, refactoring PR actually did not change behaviour, it was like this always, from the very first version of the driver.

No idea why that | 0x80 would be needed or what it does (effectively changes register number, by why? 🤷‍♂️)

[breml]

I am not totally sure, but in the data sheet on page 39, it says, that for read operations, an additional bit must be set (see table 24 + 25). The confusing thing is, that there the least significant bit is set where the above mentioned fix with | 0x80 does set the most significant bit. But this can well be due to differences in bit order.

[ysoldak]

No, that bit is about I2C address, not register number.

[ysoldak]

Here, I found it, on page 38:

The I2C embedded inside the LSM303AGR behaves like a slave device and the following protocol must be adhered to. After the start condition (ST) a slave address is sent, once a slave acknowledge (SAK) has been returned, an 8-bit subaddress (SUB) is transmitted: the 7 LSb represent the actual register address while the MSB enables address auto increment. If the MSb of the SUB field is 1, the SUB (register address) is automatically increased to allow multiple data read/writes.

So | 0x80 enables multibyte read, which we indeed use.

~The driver was broken from the very first version, wonder nobody reported this earlier.~ Refactoring PR did broke it by introducing multibyte read.

[ysoldak]

@breml you want to make a fix PR?

[breml]

I am glad you fund the right explanation in the data sheet. Yes, I am happy to create a PR, hopefully tomorrow.

[ysoldak]

Great! But please, add a good comment on why this | 0x80 is needed or make a self-explanatory const.

[breml]

@ysoldak Yes, will do.

I think the LSM303AGR worked until the refactoring, because before, every byte was read individually (e.g. https://github.com/tinygo-org/drivers/pull/368/files#diff-a17120368c28202ceb30b483401c06a512bf3702efd6e1e1098e34e6f2789c12L117-L122). I just wonder, if the LSM303AGR is the only case, that needs to be fixed. Judging from #368, I could not see other cases, where before multiple read calls have been made. That being said, I only own a device with the LSM303AGR, other cases I am not able to test.

[alankrantas]

Well...I don't have the v2.2 micro:bits but I have v1.5 and v2s, which has the same type of LSM303AGR and is known to work with previous versions of TinyGo. Can help testing if no one else have them.

[ysoldak]

@breml Yes, you are right. Introduction of multibyte read broke it. I believe it just works for other LSM sensors, I have them and tested. Partly explains how this happened -- would not imagine multibyte read must be separately enabled.

@alankrantas yes please 🙏