feat: add MPU-9250 / RAK1905 9-axis sensor support - #10556
Conversation
The RAK12034 (BMX160) WisBlock module is end-of-life. RAK's official replacement is the 6-axis RAK12033, which drops the magnetometer entirely. RAK1905 (InvenSense MPU-9250) is the closest 9-axis WisBlock module still in production and is a natural drop-in for users who want to keep tilt-compensated compass functionality. This adds a minimal direct-I2C driver — no new library dependency, ~5KB flash on rak4631. The MPU-6500 accel/gyro die is driven directly; the AK8963 magnetometer die is exposed via the MPU's I2C bypass mode and talked to at 0x0C. Per-axis Fuse-ROM ASA sensitivity factors and ST2 overflow are honored. Hard-iron calibration is persisted to /prefs/compass_mpu9250.dat, matching the BMX160 / ICM-20948 pattern. Detection disambiguates MPU-6050 from MPU-9250/9255 by reading WHO_AM_I (0x75) after the existing register-0x00 fallback path that already covers ICM-20948 / BMI270 / BMX160 / SEN5X. MPU-6050 paths are unchanged.
Detection found the chip and set the device type correctly, but ScanI2C::firstAccelerometer() iterates a hard-coded list that didn't include MPU9250 — so accelerometer_found stayed unset and the AccelerometerThread was never constructed for the new sensor.
github-actions
Bot
added
the
hardware-support
Compass-only fusion is stateless, so any dynamic acceleration during device rotation immediately showed up as overshoot on the displayed heading. Add a per-axis exponential moving average on the raw accel and mag samples before tilt compensation. Accel uses alpha=0.15 to keep the gravity reference stable; mag uses alpha=0.20 for slightly faster response. Time constant is well under half a second so the needle still tracks deliberate rotation, but small jitters and rotation-induced spikes are damped.
FusionCompassCalculateHeading assumes the sensor's Z axis is world-up. That holds when the WisBlock baseboard is mounted flat, but fails on cases where the baseboard sits vertical (LCD perpendicular to the board) — chip Z is then horizontal and tilt-comp becomes unstable. Add a build-time MPU9250_UP_AXIS selector with five options: PZ (default, baseboard flat), PX, NX, PY, NY. Implementation rotates both accel and mag via FusionAxesSwap so the rest of the heading pipeline still sees Z as up. The temporary default is PX while testing on a vertical RAK case; will be reverted to PZ before the PR is marked ready, with vertical case users opting in via variant.h or build flags.
The previous commit shipped with PX as the default while validating the remap on a vertical-mount RAK case. PZ matches the assumption baked into BMX160 / ICM20948 (chip Z = world up), so leaving the default at PZ keeps existing flat-mount users untouched. Vertical mount users opt in by setting MPU9250_UP_AXIS in their variant.h or via build_flags in platformio.ini.
|
Heads up - opened a design discussion about a related limitation I found while working on this: the existing compass code (here and in the BMX160 / ICM20948 drivers) assumes the sensor chip is mounted with Z = up, which breaks tilt compensation in vertical-mount cases. This PR ships with a build-time workaround (MPU9250_UP_AXIS), but the proper runtime-configurable solution is out of scope for this PR. Discussion: #10557 Doesn't block review of this PR, just flagging so reviewers know the orientation question is being tracked separately rather than ignored. |
…05-magnetometer # Conflicts: # src/detect/ScanI2C.cpp # src/motion/AccelerometerThread.h
…05-magnetometer # Conflicts: # src/detect/ScanI2C.cpp
There was a problem hiding this comment.
Pull request overview
Adds first-party support for the InvenSense MPU-9250/MPU-9255 (RAK1905 WisBlock) motion sensor, including scan-time disambiguation from other devices that share the same I2C addresses, and a new direct-I2C motion driver that provides tilt-compensated compass heading and persisted hard-iron calibration.
Changes:
- Add a new
MPU9250Sensordirect-I2C driver (accel + AK8963 mag) with Fusion-based heading and persisted calibration. - Update I2C scanning and accelerometer selection to recognize
MPU9250viaWHO_AM_Iand instantiate the new driver. - Add address constants for MPU-9250 and AK8963 to shared configuration.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/motion/MPU9250Sensor.h |
Declares the new MPU-9250/9255 motion sensor driver interface and calibration/filter state. |
src/motion/MPU9250Sensor.cpp |
Implements MPU-6500 + AK8963 init, sensor reads, tilt-compensated heading, and calibration persistence. |
src/motion/AccelerometerThread.h |
Wires the new ScanI2C::DeviceType::MPU9250 to instantiate MPU9250Sensor. |
src/detect/ScanI2CTwoWire.cpp |
Adds WHO_AM_I-based disambiguation to detect MPU-9250/9255 vs MPU-6050 on shared addresses. |
src/detect/ScanI2C.h |
Extends the DeviceType enum with MPU9250. |
src/detect/ScanI2C.cpp |
Includes MPU9250 in the accelerometer preference list. |
src/configuration.h |
Adds I2C address defines for MPU-9250 and AK8963. |
Firmware Size Report22 targets | vs
Show 17 more target(s)
Updated for c8610a1 |
Move the MPU-9250/9255 WHO_AM_I disambiguation ahead of the BMX160-by-address fallback in ScanI2CTwoWire. Previously an MPU-9250 strapped to 0x69 (== BMX160_ADDR) was caught by the BMX160 address shortcut and misidentified, preventing the new driver from being selected (flagged in Copilot review). Also reflow the accelerometer preference list in ScanI2C.cpp to satisfy clang-format (Trunk Check).
jp-bennett
left a comment
jp-bennett
left a comment
There was a problem hiding this comment.
Looks OK to me. I'll have to check if I have one of these to test.
|
Thank you @jp-bennett |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds MPU9250 support by defining I2C addresses, extending device detection, implementing MPU9250/AK8963 sensing and heading logic, and wiring the driver into ChangesMPU9250 sensor support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ScanI2CTwoWire
participant ScanI2C
participant AccelerometerThread
participant MPU9250Sensor
participant MPU6500
participant AK8963
participant FusionCompass
ScanI2CTwoWire->>ScanI2CTwoWire: read WHO_AM_I
ScanI2CTwoWire->>ScanI2C: identify MPU9250 or MPU9255
AccelerometerThread->>MPU9250Sensor: construct and init
MPU9250Sensor->>MPU6500: configure MPU-6500
MPU9250Sensor->>AK8963: configure magnetometer
loop periodic sampling
AccelerometerThread->>MPU9250Sensor: runOnce
MPU9250Sensor->>MPU6500: read accelerometer
MPU9250Sensor->>AK8963: read magnetometer
MPU9250Sensor->>FusionCompass: calculate heading
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/detect/ScanI2CTwoWire.cpp (1)
878-902: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWHO_AM_I disambiguation logic looks correct.
WHO_AM_I values (0x68 MPU-6050, 0x71 MPU-9250, 0x73 MPU-9255) at register 0x75 match known chip datasheets, and running this check before the BMX160-by-address fallback correctly avoids misidentifying an MPU-9250 strapped to 0x69 as a BMX160.
One nit: the explanatory comment at Lines 878-882 spans 5 lines, exceeding the one-to-two-line comment guideline for this file type.
✏️ Suggested comment trim
- // MPU-6050 and MPU-9250/9255 share I2C addresses (0x68/0x69), and chip ID - // register 0x00 reads 0x00 on MPU-9250. Disambiguate via WHO_AM_I (0x75): - // MPU-6050 -> 0x68, MPU-9250 -> 0x71, MPU-9255 -> 0x73 - // This must run before the BMX160-by-address fallback below, otherwise an - // MPU-9250 strapped to 0x69 (== BMX160_ADDR) is misidentified as a BMX160. + // Disambiguate shared 0x68/0x69 addresses via WHO_AM_I (0x75); must run + // before the BMX160-by-address fallback to avoid misidentifying MPU-9250@0x69.As per coding guidelines, "Keep code comments minimal: one or two lines at most, only explain the why when it is not obvious, and avoid multi-paragraph explanatory comments."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/detect/ScanI2CTwoWire.cpp` around lines 878 - 902, Trim the multi-line explanatory comment in ScanI2CTwoWire’s WHO_AM_I disambiguation block so it fits the file’s one-to-two-line comment guideline. Keep only the essential rationale near the registerValue check and the BMX160_ADDR fallback, preserving the behavior in the MPU9250/MPU6050 detection logic while shortening the comment.Source: Coding guidelines
src/motion/MPU9250Sensor.h (1)
12-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim the long explanatory comments.
Several new comments exceed the project’s “one or two lines” C++ comment style. Keep the non-obvious axis/calibration rationale, but move module history and datasheet-level detail to docs or a shorter summary.
As per coding guidelines, “Keep code comments minimal: one or two lines at most, only explain the why when it is not obvious, and avoid multi-paragraph explanatory comments.”
Also applies to: 33-44, 51-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/motion/MPU9250Sensor.h` around lines 12 - 26, The new header comments in MPU9250Sensor.h are too verbose and exceed the project’s one- or two-line comment style. Shorten the explanatory blocks near the axis constants and the MPU-9250 module description, keeping only the non-obvious rationale needed for MPU9250_UP_AXIS and the driver setup in MPU9250Sensor while moving history, replacement context, and datasheet-level details to external docs or a brief summary.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/motion/MPU9250Sensor.cpp`:
- Around line 279-294: The MPU9250 axis-remap logic in MPU9250Sensor should fail
fast for invalid MPU9250_UP_AXIS values instead of silently doing nothing when
none of the FusionAxesSwap branches match. Add an explicit compile-time check
around the existing MPU9250_UP_AXIS selection so only the known
MPU9250_UP_AXIS_PX, _NX, _PY, _NY, and default MPU9250_UP_AXIS_PZ values are
accepted, and make the fallback/default case unambiguous in this section of
MPU9250Sensor.cpp.
- Around line 226-235: The calibration flow in MPU9250Sensor::update currently
exits early when readSensors(accel, mag) fails, which prevents
finishCalibrationIfExpired() from running and can leave doCalibration stuck on;
restructure the update path so calibration expiry is checked even when fresh
sensor data is missing, while still skipping updateCalibrationExtrema() when mag
data is unavailable. Keep the logic centered around readSensors,
beginCalibrationDisplay, updateCalibrationExtrema, and
finishCalibrationIfExpired so the timeout can close the calibration window
regardless of missed AK8963 samples.
- Around line 107-118: The MPU9250Sensor::init setup path ignores several
writeRegister() failures, so the sensor can report success even when DLPF,
accel/gyro range, bypass, or AK8963 configuration did not apply. Update
MPU9250Sensor::init() to check every required writeRegister() call in the
configuration sequence (including the later magnetometer setup writes as noted
in the comment) and return false immediately on any failure. Make sure
AccelerometerThread::init() only proceeds when MPU9250Sensor::init() fully
succeeds so a partially configured driver is not kept alive.
---
Nitpick comments:
In `@src/detect/ScanI2CTwoWire.cpp`:
- Around line 878-902: Trim the multi-line explanatory comment in
ScanI2CTwoWire’s WHO_AM_I disambiguation block so it fits the file’s
one-to-two-line comment guideline. Keep only the essential rationale near the
registerValue check and the BMX160_ADDR fallback, preserving the behavior in the
MPU9250/MPU6050 detection logic while shortening the comment.
In `@src/motion/MPU9250Sensor.h`:
- Around line 12-26: The new header comments in MPU9250Sensor.h are too verbose
and exceed the project’s one- or two-line comment style. Shorten the explanatory
blocks near the axis constants and the MPU-9250 module description, keeping only
the non-obvious rationale needed for MPU9250_UP_AXIS and the driver setup in
MPU9250Sensor while moving history, replacement context, and datasheet-level
details to external docs or a brief summary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 75828694-0481-4f66-bf56-931001ab82cd
📒 Files selected for processing (7)
src/configuration.hsrc/detect/ScanI2C.cppsrc/detect/ScanI2C.hsrc/detect/ScanI2CTwoWire.cppsrc/motion/AccelerometerThread.hsrc/motion/MPU9250Sensor.cppsrc/motion/MPU9250Sensor.h
- initMPU6500/initAK8963: return false on any required writeRegister() failure so a partially configured driver is never kept alive - runOnce: run finishCalibrationIfExpired() even when readSensors() fails, so missed AK8963 samples cannot stall the calibration window open - axis remap: add explicit MPU9250_UP_AXIS_PZ branch and #error default so an invalid MPU9250_UP_AXIS build flag fails at compile time
Documents all ten MPU9250Sensor member functions to satisfy the CodeRabbit docstring-coverage check (was 12.5%, threshold 80%). Comment-only change.
Shorten the WHO_AM_I disambiguation comment and the MPU9250Sensor.h class / EMA comments to the repo's one-to-two-line guideline, keeping the non-obvious rationale (detection ordering, axis-remap, filtering). Comment-only.
Replace Unicode em-dashes with ASCII hyphens (ascii-dash linter) and apply
clang-format 16 in MPU9250Sensor.{cpp,h}. Formatting-only.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
…05-magnetometer # Conflicts: # src/detect/ScanI2CTwoWire.cpp
|
Hey @jp-bennett |
develop switched to the upstream meshtastic/Fusion library (meshtastic#10724), which renamed the axes-remap API. Update MPU9250Sensor to match the other sensors: - FusionAxesSwap -> FusionRemap - FusionAxesAlignment* -> FusionRemapAlignment* - FusionCompassCalculateHeading(conv, a, m) -> FusionCompass(a, m, conv)
…ter' into feat/mpu9250-rak1905-magnetometer
|
Sorry for the ping again, @jp-bennett could you trigger CI again? Some builds were failing and I fixed them. |
|
Hey @jp-bennett, could you please trigger CI again? |
The file re-declared the global screen as a raw pointer (extern graphics::Screen *screen), conflicting with the real declaration in main.h (extern std::unique_ptr<graphics::Screen> screen). Build variants whose include chain pulled in main.h failed with 'conflicting declaration'. Include main.h instead; if (screen) and screen->setHeading() work unchanged on the unique_ptr.
|
Hey @jp-bennett, sorry for another ping. I found a bug with main.h and fixed it. Could you please trigger CI again? |
|
Hey @jp-bennett |
Conflict: src/detect/ScanI2C.cpp firstAccelerometer() probe list. develop added SC7A20 and moved QMA6100P last; this branch appended MPU9250. Resolved as a union keeping develop's ordering with MPU9250 appended (14 entries) and bumped firstOfOrNONE count 13 -> 14.
Summary
Adds support for the InvenSense MPU-9250 (RAK1905 WisBlock module): a 9-axis sensor that combines a 3-axis accelerometer, 3-axis gyroscope, and 3-axis magnetometer in one package. The driver provides tilt-compensated magnetic heading via the existing Fusion library, with hard-iron calibration persisted to flash - matching the BMX160 and ICM-20948 patterns.
Implemented as a minimal direct-I2C driver with no new library dependency. Flash impact on RAK4631: roughly +1 KB.
Motivation
The RAK12034 (BMX160) is end-of-life. RAK's official replacement, RAK12033 (IIM-42652), is 6-axis only and drops the magnetometer entirely. For users who want tilt-compensated compass on a WisBlock-based device, the closest still-in- production option is the RAK1905 (MPU-9250).
Detection
MPU-9250 shares I2C addresses (0x68/0x69) with MPU-6050, ICM-20948, BMX160, BMI270, SEN5X. "ScanI2CTwoWire" now reads WHO_AM_I (0x75) after the existing register-0x00 disambiguation:
No regressions to the existing detection paths.
Testing
Verified on a RAK4631 + RAK19007 baseboard + RAK1905:
References
Related WisBlock modules
MPU-9250 chip-level documentation (TDK InvenSense)
Reference implementations
my, mx, -mzto align the AK8963 magnetometer frame with the MPU-6500 accel frame).🤝 Attestations
Video of testing: https://photos.app.goo.gl/Vkcj1HB4mcSKeKBi8
Summary by CodeRabbit
New Features
Bug Fixes