-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
accumulated calibration fixes and improvements (mostly magnetometer) #15235
Conversation
3360b62
to
d6e754e
Compare
0eb2166
to
ac94518
Compare
5e1e347
to
32b245c
Compare
7eff315
to
6d2d07f
Compare
- print calibration status when finished
- print each calibration when finished
- skip rotations that are identical or very close - compute mean squared error (MSE) to skip a sqrt
- ROTATION_PITCH_9_YAW_180 and ROTATION_ROLL_90_PITCH_68_YAW_293 are kept if manually configured
- this keeps the string within a single Mavlink STATUSTEXT - fixes #14829
- improve status print
- reduced now that calibration uses uORB::Subscription - can likely be reduced further (perhaps < 8) with additional testing
cbc224a
to
7832870
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good on my side, I tried a few orientations again and the detection works really well.
Maybe only point for later to improve: you currently don't need to perform a full rotation to complete one axis; doing some small left-right rotations is enough to pass to the next orientation, which isn't ideal.
Not ideal but not a regression, tricking it was already easy before. |
It's a bit more obvious now because you can wiggle in place and force it through faster. It's also slightly harder though as it must accept samples from all mags in the same update. I found that as long as it was the full 6 orientation calibration it's actually pretty hard to mess up even if you squander a few sides completely without rotating. We definitely should encourage people to use the full 6 orientation unless it's not possible, and that's one of the reasons I tried to make it much faster. I also considered running little attitude estimators in place and really requiring proper clean rotations, but that had implications for auto rotation later that I didn't have time to work through. |
Thanks everyone! |
Good timing, I just started testing it and I noticed:
|
I believe this is QGC doing a full param sync after calibration. I'm going to start keeping a closer eye on mavlink performance and latency to see if we can do anything about this and other issues. In particular for manual control I don't yet know if it's the QGC stick broadcast getting blocked or the PX4 mavlink module not receiving/processing them properly.
This is an intentional reset of those slots. After you reboot they won't be present. I also plan to update the parameter system at some point so that we're not storing a reset parameter at all. Thanks for testing. |
This pull request combines a few mag sensor pipeline and calibration related improvements and fixes that started to overlap.
New functionality
new helperI'll bring this in later.systemcmds/mag_test
that simply runs and prints the heading of each sensor_mag instance until you quitAlong the way I discovered a number of potential bugs and the need to properly carry forward additional data in calibration "slots" that led to the consolidation of these changes. One problem was again the potential inconsistency between sensor instances (uORB, /dev/magX, calibration slots) that can shift if sensor ordering changes (external mags changed or startup script).
EDIT (@MaEtUgR): fixes #14829