Magnetometer driver #176
Magnetometer driver #176
Conversation
mpkarpov-ui
left a comment
mpkarpov-ui
left a comment
There was a problem hiding this comment.
Overall looks good, some small changes would be nice, but nothing too big.
|
|
||
| ErrorCode MagnetometerSensor::init() { | ||
| if (!LIS3MDL.begin_SPI(LIS3MDL_CS)) { // Checks if sensor is connected | ||
| if (MMC5983.begin(MMC5983_CS) == false) { // Checks if sensor is connected |
There was a problem hiding this comment.
Any reason for this change? (Specifically the == false part)
| Y = ((double)cy - sf)/sf; | ||
| Z = ((double)cz - sf)/sf; | ||
|
|
||
| Magnetometer reading{X, Y, Z}; |
There was a problem hiding this comment.
I'm going to assume for this review that this is from the datasheet, but it does look like "magic math" to me... Could you add a doc comment?
| double mx; | ||
| double my; | ||
| double mz; |
There was a problem hiding this comment.
Discussed this, I think its OK. I want a second opinion though @MuhammadAli8209 @tjmcmanamen38
Mag isn't buffered to my knowledge, so worst case this just doubles the mag log size from 12 --> 24, which isnt world ending.
| adafruit/Adafruit LIS3MDL@^1.2.1 ; Magnetometer driver | ||
| adafruit/Adafruit LIS3MDL@^1.2.1 ; Old magnetometer driver, Adafruit_BNO08x fails or else | ||
| sparkfun/SparkFun MMC5983MA Magnetometer Arduino Library@^1.1.4 ; Magnetometer driver | ||
| sparkfun/SparkFun u-blox GNSS v3@^3.1.8 ; GPS |
There was a problem hiding this comment.
OK for now, maybe let's clean it up later.
Context (@MuhammadAli8209 ): Adafruit sensor libraries bring in a bunch of adafruit helper libraries that are needed for the BNO sensor. Since we're removing the BNO, we'll remove this dependency when we merge the LSM changes.
3 participants
Changed magnetometer driver, hoping these are all the changes