ICM20948 integration into MPU9250 driver

[flochir]

I condensed the history of #9362 in this new PR to make a clear review of the changes easier and get up to date with the master branch.

The data delivered by the ICM looks good. Set Rotation 6/270° Yaw for the external magnetometer for a Here GNSS mounted in flight direction (I kept the chips own coordinate system to not obfuscate it by rotating it fitting to a specific device). rc.sensors is modified to activate the whole IMU in the Here GNSS (on Pixhawk 2/2.1) for testing purposes. To use only the magnetometer: mpu9250 -X -R 6 -M start

I'm thinking about improvements to the bus_options[] array and the associated bulk of device path definitions - they increased a lot with the new bus options added through the master branch while I was working on this. I would still need to add some to make the set complete. In the end the path strings are redundant information in bus_options[] and can be generated from bus, bus type and device type.

I'll add a test flight log as soon as possible.

[LorenzMeier]

@bkueng Could you look into this?

[flochir]

I'm a bit lost what to do about the insufficient flash size on px4fmu-v2 reported by jenkins.

[flochir]

These are two short indoor test flights with the ICM as full IMU (in the GNSS, attached to the top of the cube).

Together with the internal I2C MPU and lsm303d (SPI MPU9250 disabled):
https://logs.px4.io/plot_app?log=060d1829-af7b-4629-8b5c-be03b9f17926

ICM as only IMU:
https://logs.px4.io/plot_app?log=089075b2-60cd-4524-8e42-beb667e557d7

These were done quickly on an old DJI flamewheel configuration with pooor tuning, all I could do today because of heavy rain outside - so don't wonder ;-)

The flights themselves were pretty short (about 30 seconds) because of the surroundings. Outdoors I also have no problems with the magnetometer.

The ICM only configuration gave me oscillations on roll and nick that the multi-sensor-flight didn't show. They are nicely visible in the log however, so probably not the sensor.
This is not the best data, but maybe something sticks out that hints at remaining driver problems - please give me a hint if you see something. I'll post more data soon.

[DanielePettenuzzo]

@flochir I'm trying to start the ICM with the -M flag because I just want to use the magnetometer, but when I do I get a hardfault. Do you see the same behaviour? If I remove the -M flag and start it normally it works, although I get also accel and gyro, which I don't want.

[DanielePettenuzzo]

@flochir I did this pr to your branch fixing the issue that caused the hardfault: flochir#1
The accel and gyro ring buffers were used but not initialized. There could be more of these around, but this was the main issue.

[flochir]

@DanielePettenuzzo
I don't get a hardfault, can you give me more information? Are you on the Pixhawk 2.1, too? Can you analyse and give me the code position it fails on?

I've got another problem with the -M switch however. Currently the -M switch just disables the publishing of accel and gyro data (naive assumption that would suffice...). However the calibration routine still detects the presence of the sensors and complains that they're there, but have never published. Any hint what would be the best way to set the two sensors to disabled/not create the device files and still instantiate the objects?

edit: posted before I read your second comment

[flochir]

Thanks for the fix!

[flochir]

Here's the accelerometer calibration log with active icm in magnetometer only mode:

Start the individual calibration steps by clicking one of the buttons to the left.
[cal] calibration started: 2 accel
Accel 0 (ID 1468681) no matching uORB devid
[cal] calibration failed: reading sensor

Doing this right from my point of view would mean moving the accelerometer CDEV to its own subclass like the gyro, then not initializing it in the first place. But maybe that would be better done with a (really needed I think) thorough rework of the whole driver, maybe when adapting it to the new driver base classes in the pipeline.

So any hints on how to prevent accel/gyro from being used completely, and doing this "cleanly", as intermediate solution?

[flochir]

I did two 10 minute flights with the icm working as the only magnetometer without issues.

As a workaround for the not yet working "-M" switch/calibration problem, I loaded the ICM driver last (as the fourth accel/gyro, so it didn't get calibrated). The other magnetometers had been disabled, so the flight was with the ICM as magnetometer only and the adapted mpu9250 driver working on the two integrated MPUs.

[DanielePettenuzzo]

@flochir I will have another look tomorrow. I need to have a look at what the calibration checks

[DanielePettenuzzo]

@flochir the calibration uses the accel0, accel1, accel2. This device is created here: https://github.com/PX4/Firmware/blob/0450b5d0cac452b3e24091ef3248bc6f7a899560/src/drivers/imu/mpu9250/mpu9250.cpp#L429 and we can add an if statement also here. This should solve the calibration issue although I still don't like that the mpu9250 class creates the mpu9250_icm_accel_ext device although it is not used. I will think of a solution tomorrow.

[DanielePettenuzzo]

@flochir flochir#2 this should solve your calibration issue. Now the only remaining issue is that the mpu9250_icm_accel_ext device is still created by the mpu9250 class even when it's mag only. This doesn't seem to have any side effects but it should be fixed. I will think of how we can restructure the driver in such a way that we can get rid of all these if(!_magnetometer_only) statements.

[DanielePettenuzzo]

I also tested with the mpu9250 on a pixracer and everything seems to work.

[flochir]

Great, thanks again! I'm currently trying to write a separate driver node for the accelerometer, like there is for the gyro, so we don't need to create the device.

[flochir]

Done, now it calbrates with -M and without creating the accel/gyro devices.

[DanielePettenuzzo]

@flochir everything works for you now? I tested it and if I start accel, gyro and mag then everything works, but with the -M flag I get a reboot when I start the driver. Do you also see this behaviour? What board are you using? I'm using pixhawk 4. I wonder if the hardfault triggering is different between stem32f4 and stm32f7 microcontrollers.

[flochir]

No hardfault here:

NuttShell (NSH)
nsh> mpu9250 -X -M start
INFO  [mpu9250] Bus probed: 2
MPU9250_I2C on I2C bus 1 at 0x69 (bus: 100 KHz, max: 400 KHz)
AK8963_I2C on I2C bus 1 at 0x0c (bus: 100 KHz, max: 400 KHz)
nsh> mpu9250 -X info
state @ 2001fc80
Device type:20948
mpu9250_read: 991 events, 1138886us elapsed, 1149.23us avg, min 1134us max 1267us 18.360us rms
mpu9250_acc_read: 0 events
mpu9250_gyro_read: 0 events
mpu9250_bad_trans: 0 events
mpu9250_bad_reg: 0 events
mpu9250_good_trans: 0 events
mpu9250_reset: 0 events
mpu9250_dupe: 0 events
mag queue       2/96 (1/0 @ 2001dc60)
temperature: 21.0
nsh>

Did you check where the hardfault is triggered?

Did you try the ICM as full IMU? When I did I had quite some oscillations with about 1 or 2 Hz in flight and a bit of weird behaviour running "handheld" at high thrust, while the sensor data, sampling rate etc. looked ok. Not entirely sure yet if this couldn't be a driver issue after all.

[mwingerson]

Has there been an update on this?

Thanks!

[jonathan84clark]

The code being merged in is significantly out-of-date with master. If you pull master then try to merge the driver in you will run into build failures.
The driver appears to be returning data however when I use it with QGroundControl I am not able to calibrate using just the Here Plus GPS, it doesn't recognize that I am moving the Here Plus.
If I calibrate with the internal mag it does not recognize the compass heading changing when I just rotate the Here Plus GPS
I am using a Pixhawk 2
The driver is not functional. My team and I have spent several days trying to get it to work without luck.
I also noticed that the driver for the icm20948 is tightly coupled with the ak09916 driver.

[flochir]

@mwingerson Sorry, no updates yet, I was out of the country.
@jonathan84clark I will rebase it soon.
Did you adjust ROMFS/px4fmu_common/init.d/rc.sensors? If you didn't, can you disable all the other Acc/Gyro/Mag sensors, remove the "-M" flag from "mpu9250 -X -M -R 6 start" and try again? This would run the ICM as the full (and only) IMU. It would be interesting if it reacts then and if you are maybe able to calibrate.

I also noticed that the driver for the icm20948 is tightly coupled with the ak09916 driver.

Yes - can you elaborate? The driver treats the AK09916 as a subsystem of ICM20948, just as the original MPU9250 driver did for AK8963.

[flochir]

@DanielePettenuzzo Did you get any insights on your setup?

[jonathan84clark]

@flochir
What I mean by tightly coupled is that the icm20948 driver relies completely on the ak09916 driver. At the time I wrote that it looked like an incorrect design decision however after more research I have found it may not be incorrect. I didn't realize that the ak09916 was a separate die that was bussed to the icm20948. That makes this driver significantly more difficult to develop for PX4 (in my opinion). I really don't understand why they designed it like that. I could go on and on about the design problems I see with the icm20948 and the here plus (including the fact that the Here+ people claim it works in PX4 yet the driver doesn't exist yet). But I will not subject you to comments from the peanut gallery.

[jonathan84clark]

@flochir
What I saw when I used your driver is it failed calibration, even though it appeared to be returning valid data using the info command.
The drone engineer at my company asked me to implement a driver myself based on the hmc5883 driver. Right now my driver is working correctly except that I am having calibration issues. QGround Control indicates that the mag readings from the ak09916 are inconsistent with the internal mags.
I actually figured out a way to uncouple the icm20948 and ak09916 drivers. Basically they exist as two separate drivers however all the icm20948 does is set the I2C bus as pass-through. Then the ak09916 driver talks through that bus. They both need to be started for it to work. I felt that my design fit in better with the PX4 framework.
I am going to continue the work on my driver but if you can get yours working, approved and into master that would be fantastic.

[flochir]

The Here GNSS used to work with PX4, as long as they equipped it with the hmc5883. Maybe that was the info you got. They switched to ICM20948 when the HMC was discontinued.

[flochir]

@jonathan84clark
I started out similar. Take a look at #9345, maybe you can glean something. It's not finished however and was abandoned for this attempt to integrate full ICM support into the MPU driver (see discussion).

Btw. if you get inconsistent mags check if the sensor rotation is correct.

[jonathan84clark]

@flochir
Thanks for the help!

[jonathan84clark]

@flochir
I am trying to validate that my code is working. I was wondering if you could take a look at it. I was also wondering if there are other members of the community who I should run this by. I am getting data, QGround Control no longer reports inconsistent mag readings. But I really want to make sure things are as they should be.
Here is my fork https://github.com/jonathan84clark/Firmware
I am developing on the icm2048Core_v2 branch
Thanks,

Jonathan L Clark

[DanielePettenuzzo]

@flochir I'll spend some time on it tomorrow

[dagar]

This should now fit into px4fmu-v2. I've gutted a number of things we can live without.

Let's do one final test pass so that we can finally get this in.

Has this been tested across all the different cases we care about?

• mpu9250 SPI IMU (pixracer, pixhawk 2, or pixhawk 3 pro)
• mpu9250 I2C IMU (crazyflie?)
• ICM20948 I2C mag only (Here GPS) be sure to try several non default rotations

[dagar]

@flochir I pushed the cleanup commit to your branch again (it was lost in your last push).

[DanielePettenuzzo]

@dagar for the mpu9250 SPI IMU (pixracer, pixhawk 2, or pixhawk 3 pro) testing would it be possible to get someone from the testing team to try it? By the way does the testing team have a crazyflie? We can try several rotations for the here GPS in some non default rotations.

[flochir]

@dagar Sorry, I got used too much to force pushing on this I guess.

[dagar]

@DanielePettenuzzo no, they don't have a crazyflie. Perhaps you or @RomanBapst could try it quickly?

[flochir]

@dagar @DanielePettenuzzo
Did a few test flights with Pixhawk 2.1, with one internal mpu9250 on "external" SPI as only IMU and the icm20948 in magnetometer only mode. I varied the rotations of the external mag, but did not have the opportunity to rotate the flight control/internal IMU this time.

External mag in standard/no rotation

External mag rotated YAW 270

External mag rotated YAW 180

External mag rotated ROLL 90

[dannyfpv]

tested on pixhawk v4 pro
https://review.px4.io/plot_app?log=30056c32-68f8-433a-b688-ad727b0efe34

[mrpollo]

@dagar should we order a few Crazyflie's?

[flochir]

Rebased, only touched rc.sensors. @dagar, I put the mpu9250 driver call for icm20948 into the "Begin Optional drivers" section. Is this the rigth place?

[dagar]

Testing looks good, let's finally get this in. We really need to get the basic driver structure under control, this could have been a much simpler process.

Thanks everyone