search

tinygo-org / drivers

TinyGo drivers for sensors, displays, wireless adaptors, and other devices that use I2C, SPI, GPIO, ADC, and UART interfaces.
https://tinygo.org
BSD 3-Clause "New" or "Revised" License
607 stars 191 forks source link

Use int16 for ADXL345 readings #656

Closed nmaggioni closed 7 months ago

nmaggioni commented 7 months ago

The ADXL345 accelerometer outputs readings as 16-bit integers (see the first page of its datasheet), and thus parsing them as 32-bit ones skews the results.

Here is the example code running on an RP2040 with the current implementation and giving out of range readings (the sensor is sitting facing downwards and slightly tilted towards the Y axis):

X: 20 Y: 448 Z: 261212
X (raw): 5 Y (raw): 112 Z (raw): 65303

And here with proper typecasting, with higher values now in the expected µg (micro-gravity) range:

X: 12 Y: 432 Z: -940
X (raw): 3 Y (raw): 108 Z (raw): -235

Given the comments in the source code itself, I guess that this driver porting error was previously masked by the limited bitness of other microcontrollers?

While I was at it I deleted the error return param from ReadAcceleration since I didn't see anything prone to throwing in its codepath, but I'm wondering if https://github.com/tinygo-org/drivers/pull/46#issue-437786421 is still applicable and it should be kept instead. CI gave me the answer :)

deadprogram commented 7 months ago

Thanks for the fixes @nmaggioni now going to squash/merge.

aykevl commented 7 months ago

Given the comments in the source code itself, I guess that this driver porting error was previously masked by the limited bitness of other microcontrollers?

No, integers are all the same on every microcontroller. The only one that might be different are int, uint, and uintptr: int and uint are either 32-bit or 64-bit (but almost always 32-bit on microcontrollers), uintptr is the size of a pointer but is rarely used.

(Just wanted to clear something up that might cause confusion).