hybridgroup / gobot

Golang framework for robotics, drones, and the Internet of Things (IoT)
https://gobot.io
Other
8.9k stars 1.04k forks source link

HMC5883L driver returns wrong values #830

Closed gen2thomas closed 1 year ago

gen2thomas commented 2 years ago

According to the datasheet section "Data Output X Registers A and B" the values should be in 2s complement form +0xF800 ... 0x07FF (-2048 ... +2047). In case of an overflow, the value gets "-4096 (0xF000)".

In my first test case, the values of X,Y, Z jump in a high range. All values can exceed 2048 and don't lock to -4096 for a strong magnetic field.

A first investigation shows, that a missing byte-swap after reading the MSB, LSB data with "ReadWordData()" could be the root cause.

I will provide a fix after some further tests, maybe together with #805 .

gen2thomas commented 2 years ago

Following a spontaneous idea, I investigate all calls of "ReadWordData()" and all implementations. Normally the function should return the bytes in the expected order, otherwise we would not need this function. Also it is surprising, that there is no related bug entry since the initial implementation of the HMC5883L driver with #781 in April 2021.

I found usage of "ReadWordData()" for the following devices:

These implementations can be found:

note: After reading the SMBus specification, I come to the conclusion, that the order is defined as LSB, MSB. All platforms, except firmata and digispark use this implementation.

So I assume that the following drivers will not work at the moment (independent of the used platform):

Note for TSL2561: Because the documentation is somewhat confusing about the read order (stored order is LSB, MSB), I found the arduino implementation from adafruit, which reads the values sequential (without set the address for the second value) and use the first value (little address) as LSB, this means do not swap. The same for the arduino library of sparkfun. So our driver should work fine.

gen2thomas commented 2 years ago

I have tested the HMC5883L driver today with my digispark platform and the same problem happen. So I looked again to the digispark driver implementation, and I was wrong with my statement that the bytes become swapped, because it reads 2 bytes and the first becomes the low byte, the second the high byte. I have corrected my comments above.

gen2thomas commented 2 years ago

To clarify, how a word read "normally" works, I installed my mcp2221 USB-to-UART/I2C serial converter and connected the HMC5883L to it. Than I initializes the device and read the data for X in different ways:

This means, that also the commonly used i2cget binary will return a word as LSB, MSB (little endian), which matches the implementation of all our platforms. So the HMC5883L driver (and maybe the CCS811) needs to be fixed (store values as big endian).

deadprogram commented 2 years ago

Looks like something must have gotten mixed up at some point. Thanks for finding this issue!

gen2thomas commented 1 year ago

part of release v2.0.0