Fix BMM150 blocking magnetometer read

[bjornhbengtsson]

Description

Fixes the blocking BMM150 magnetometer read path so it is consistent with the interrupt-driven read path.

Changes:

• Read all 8 BMM150 data bytes on A0002 Rev 2, including RHALL
• Replace logical && operations with bitwise & masks
• Sign-extend the packed 13-bit X/Y and 15-bit Z measurements
• Store the RHALL measurement in IMU_RAW
• Propagate all magnetometer read errors instead of checking only for timeout
• Preserve the six-byte read behavior for A0002 Rev 1

Issue Link

Related to SunDevilRocketry/Flight-Computer-Firmware#300

Testing

• Passes existing unit tests

• Unit tests modified

• Integration test performed

• git diff --check passed

• No existing host-side unit-test target compiles driver/imu/imu.c

• Hardware validation is still required

Other

This PR fixes the blocking imu_get_mag_xyz() function, which is not currently called by the flight firmware, but served as a useful first step in familiarizing myself with the magnetometer implementation. Further work for issue #300 will investigate and validate the interrupt-driven magnetometer path used during normal operation and determine how the retrieved magnetometer data can be used for sensor fusion.

https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmm150-ds001.pdf

https://getyarn.io/yarn-clip/23a5374e-cb47-431e-82a0-7a63da68f11f/gif

Reviewer Checklist

Standards

• Follows FCF Architectural Standards
• Follows SDR Coding Standards
• Code complexity/function Size is minimized
• Code is testable
• Code is readable and commented properly
• License terms are respected

Error Handling

• Potentially unsafe functions return a status code
• Error returns properly handled

Memory

• Stack allocated memory is scoped correctly
• Heap allocated memory is avoided
• Globally allocated memory is minimized except when necessary
• Pointers are used correctly
• Concurrency has been considered

Performance

• Rate limiters are respected
• Busy waiting is avoided
• "Delay" calls are not used in performance sensitive code

[bjornhbengtsson]

The automated GPS test currently fails while compiling mod/sensor/sensor.h because IMU_DATA is undefined in the test environment. The failure occurs before compiling the modified imu_get_mag_xyz() function and appears unrelated to this PR. The servo job was subsequently cancelled.

[ETSells]

Picking up this review myself as I am the SME for legacy IMU. Nick, you're still more than welcome to get eyes on it too.

This looks solid! Consider dropping the rev 1 conddefs, but other than that I'm happy with this!

Agreed that there's no worries about the test issue. That happens on the target branch as well, but will be fixed once FCF changes hit main.

[ETSells]

We are dropping support for the rev 1 IMU in #28. Let's consider dropping this path for rev 1 mag reads.

[ETSells]

OOF no wonder our mag reads on the blocking path always used to be missing precision on the lower bits lmaoooo! Good spot!

[ETSells]

nit: this compound is getting a bit hard to read. Consider using the styling that was here before i.e.

mag_x_raw = (int16_t)(   (uint16_t) regMag[1]                        << MAG_XY_MSB_BITSHIFT ) |
                     ( ( (uint16_t) regMag[0] & MAG_XY_LSB_BITMASK ) >> MAG_XY_LSB_BITSHIFT );

If you're not used to SDR's code styling it might seem trivial or not as readable, but it makes a huge difference to me as a reviewer and to readability throughout our codebase if everything is more uniform. The compound makes it a bit hard to track everything, but the extra spacing and forcing everything to two lines helps tell me how this works:

1. The byte order out of the raw registers is little endian
2. There is a bitmask applied to the least significant bits of each register
3. There is a shift applied to both the MSB and LSB
4. We bitwise OR the MSB and LSB, then store them into an int16_t that houses the scaled raw values.

[bjornhbengtsson]

Thanks for the review Eli! I removed the Rev 1 path from imu_get_mag_xyz(). I also reformatted the register unpacking and sign-extension logic to match SDR's style and make the bitmasking and shifting operations easier to follow.

[ETSells]

ship it

[bjornhbengtsson]