Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pr mag cal fix #7287

Merged
merged 12 commits into from May 26, 2017

Conversation

Projects
None yet
6 participants
@mhkabir
Copy link
Member

commented May 24, 2017

This PR :

  • Fixes a bad assumption in sensor calibration code, which assumed that uORB instance ID == the ID of the file descriptor (e.g /dev/sensor_gyroX) for a sensor. This would lead to offsets being applied to the wrong sensor depending on the race between driver instantiation and uORB advertisement. This was hidden so far because of the startup order of sensors on non-v4Pro and non-v5 boards.
  • Enables temperature compensation and boot-calibration for HMC5x83 mags running on v4Pro and v5 hardware, which should improve performance in general.
  • Fixes MPU9250 mag driver to properly publish it's device ID.
  • Fixes LIS3MDL mag driver to properly publish it's device ID.
  • Adds checks to the sensor calibration code which will fail the calibration if there are inconsistencies in device IDs (usually pointing to a buggy sensor driver) so that we can catch them out and fix them. These problems have remained hidden so far because nothing was forcing a sanity check.

This needs :

  • Test flights on Pixhawk 1 / Pixhawk Mini, Pixhawk 2.1, Pixracer and Pixhawk Pro. Full parameter reset and redo all calibrations before flying.
  • Bench tests on Snapdragon Flight / Bebop / RPi to make sure that calibration still works as normal.

@mhkabir mhkabir force-pushed the pr-mag-cal-fix branch 2 times, most recently to 7fbf2ba May 24, 2017

@LorenzMeier

This comment has been minimized.

Copy link
Member

commented May 24, 2017

Awesome work!

orb_unsubscribe(worker_data.subs[cur_accel]);
}

#elif defined(__PX4_QURT) || defined(__PX4_POSIX_RPI) || defined(__PX4_POSIX_BEBOP)

This comment has been minimized.

Copy link
@bkueng

bkueng May 24, 2017

Member

This can be simplified to #else

This comment has been minimized.

Copy link
@mhkabir

mhkabir May 25, 2017

Author Member

Done. Also extended to include Excelsior boards.

Side note : These defines are yuck.

This comment has been minimized.

Copy link
@bkueng

bkueng May 26, 2017

Member

How about #ifdef __PX4_NUTTX?

This comment has been minimized.

Copy link
@mhkabir

mhkabir May 26, 2017

Author Member

Done. :)

@mrpollo mrpollo added this to the Release v1.6.0 milestone May 24, 2017

@mhkabir

This comment has been minimized.

Copy link
Member Author

commented May 24, 2017

We still need the fix is more places, I think. Flight test on Pixhawk Pro with correct mag orientation showed toiletbowling, so it seems that there is something still wrong. Will keep working on this.

@LorenzMeier

This comment has been minimized.

Copy link
Member

commented May 25, 2017

Can you please share a log? I don't think that validating it using a flight says anything about correctness - it just says that the system you flew wasn't right.

It could be that the calibration data was wrongly assigned and is now correctly, etc. Please try to check separately for correctness and we'll handle any churn (e.g. a required re-calibration) separately iff required.

@mhkabir

This comment has been minimized.

Copy link
Member Author

commented May 25, 2017

More locations in the code needed the fix. See the commit I just pushed : 6abcfd3

However, since now we only set the rotation matrix on startup, it would require a reboot after calibration. I added the @reboot_required flags to the params, but this might need some more support on QGC calibrator side to properly propagate up to the user.

@mhkabir mhkabir force-pushed the pr-mag-cal-fix branch 2 times, most recently from 073c14b May 25, 2017

mhkabir added some commits May 23, 2017

rc.sensors : start external compass on v4Pro and v5 with temperature …
…compensation and self-calibration at startup

@mhkabir mhkabir force-pushed the pr-mag-cal-fix branch May 26, 2017

@Avysuarez

This comment has been minimized.

Copy link

commented May 26, 2017

@mhkabir

This comment has been minimized.

Copy link
Member Author

commented May 26, 2017

@Avysuarez @PX4TestFlights OK, looks good. Can you test on v4 hardware? Normally it would not tell us much because v1-v4 hardware didn't expose the bug, but I will give you a custom binary in that case, which will force that situation. It would be good to fly with the board and external compass rotated (on yaw) with respect to the frame (any rotation should do). Make sure you reset params and recalibrate, setting the correct orientations during calibration.

Here is the binary for Pixracer :
px4fmu-v4_default.px4.zip

@santiago3dr

This comment has been minimized.

@santiago3dr

This comment has been minimized.

Copy link

commented May 26, 2017

@mhkabir mhkabir force-pushed the pr-mag-cal-fix branch to 9f943fa May 26, 2017

@mhkabir

This comment has been minimized.

Copy link
Member Author

commented May 26, 2017

@santiago3dr @Avysuarez Thanks for the testing. Looks good!

@LorenzMeier Good to merge after CI passes, and we make sure that QGC asks for a reboot after setting board/compass rotation.

@mhkabir

This comment has been minimized.

Copy link
Member Author

commented May 26, 2017

@jywilson @mcharleb Can you please test this on Snapdragon? Please do a parameter reset and recalibrate all sensors to make sure that the calibration routine still functions properly. A bench test will be enough.

@LorenzMeier LorenzMeier merged commit ed5cf9f into master May 26, 2017

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/sitl/pr Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details

@LorenzMeier LorenzMeier deleted the pr-mag-cal-fix branch May 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.