Regression: Navio2 magnetometer failure

[nicolaerosia]

Hello,

I'm getting the following error and I don't have a magnetometer working anymore:

ERROR [sensors] Mag #0 fail:  STALE!

I've bisected this down to 5cbaaa6

[mhkabir]

@nicolaerosia would be good if you could debug it further. I don't have a Navio setup ready with me at the moment.

[nicolaerosia]

@mhkabir I'll investigate. Any ideas what could be the issue?

[mhkabir]

It's not obvious how the bisected commit breaks it. Are you sure that it's the correct breaking one?

[nicolaerosia]

Yes, I will check again even though I have double tested yesterday.

[mhkabir]

@nicolaerosia I'm guessing that one of the rotation calls fail because I see that all the values are zero as reported by @tstellanova.

This is where it probably fails : 5cbaaa6#diff-a4442851925fd43c16a781c4b830eb87R734. It never gets the rotation matrix.

[nicolaerosia]

I've rechecked, and this is the last working commit

commit 890c415ff2fc15d53cc59a0ab245083c6844d95a
Author: Anton Matosov <anton.matosov@gmail.com>
Date:   Tue Jun 6 23:21:32 2017 -0700

    Set acro to allowed max values

[mhkabir]

OK I think I got why it broke. POSIX needs an alternative to the IOCTL to detect external mags.

[LorenzMeier]

Yes, it should be all uORB topic internal (including the internal / external flag). We probably should introduce the concept for POSIX devices first, make sure it works there and then flick the rest of the system.

[bkueng]

I can confirm this is currently broken on RPi. The problem is that all _mag_device_id in here are 0. The failure comes from here, because there is no /dev/mag0. On RPi there is only /dev/imu0 and /dev/imu1. We have several options:

• make sure /dev/magX exists on POSIX
• extend checking for /dev/imu0
• use uorb only to get the driver id (temp compensation already does that). But for instance apply_accel_calibration needs the handle to set the calibration data (on POSIX, the DF wrappers read out the calib params themselves. But I don't see that they support CAL_MAG%i_ROT, so this should still be applied in voted_sensors_update)

[mhkabir]

@bkueng thanks. Would you be able to propose a PR with the last suggestion? We want to move away from ioctls as far as possible and use the uORB API more. As @LorenzMeier says, we'd need to add an internal/external flag in the uORB topic and fix voted_sensors_update to use uORB as needed. I don't have a RPi setup, and am not available for development for some time.

[bkueng]

I'd probably also go for the 3rd, but will need to look at it some more. I can do that.
Do you know how the CAL_MAG0_ROT is set? And how this is supposed to work? It's never set in the code, and I only see that if I set it manually, then QGC shows an external compass orientation field on Set Orientations.

[mhkabir]

It's set from QGroundControl if it's an external mag (this detection is probably broken on POSIX for ages or never existed). voted_sensors_update uses the parameter value to get the appropriate rotation matrix and applies it to the unrotated raw measurements from the sensor (from the sensor_mag topic)

For internal mags, the parameter is always set to -1, and voted_sensors_update just uses the board rotation matrix.

[bkueng]

How does QGC know it's external? I tested this with a pixracer & external mag attached, but the param was not set (I did a calibration).

[nicolaerosia]

note that df_mpu9250_wrapper already applies a rotation, should this be done elsewhere, I don't quite follow
https://github.com/PX4/Firmware/blob/master/posix-configs/rpi/px4.config#L13
https://github.com/PX4/Firmware/blob/master/src/platforms/posix/drivers/df_mpu9250_wrapper/df_mpu9250_wrapper.cpp#L869
https://github.com/PX4/Firmware/blob/master/src/platforms/posix/drivers/df_mpu9250_wrapper/df_mpu9250_wrapper.cpp#L612

[bkueng]

@nicolaerosia no, that is correct. The whole subject is quite complex & confusing. In addition to what we talked about above - the external rotation, which can be set by the user, there is also a rotation parameter supplied via CLI - the one you mention. This is used to make sure internal sensors are correctly rotated according to the labeling on the board. Which is why this is fixed, but can be different for different boards.

[nicolaerosia]

@bkueng ok, so we're applying two rotations, right? Maybe we could just multiply those, and get away applying a single rotation

[bkueng]

Right. But they're applied in 2 different places, and I think it makes sense to keep the one in the driver and the other in the common sensors module.