Conversation
The movement threshold of the gyro zero calibration was a fixed number of raw sensor counts. A gyro configured for a higher sensitivity reports more counts for the same physical rotation, so its standard deviation exceeded the threshold while the board was perfectly still and the calibration restarted forever. Express the threshold as a rotation rate and convert it into sensor counts using the scale reported by the driver. At the default 16.4 LSB/dps the effective threshold stays at 32 counts. Fixes iNavFlight#10650
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoScale gyro calibration threshold by sensor sensitivity
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can switch off images and animations for a plain-text comment |
|
Note: #11932 fixes |
|
@sensei-hacker Looked at #11932 - they do not touch, in either sense. Different functions, ~50 lines apart in
No shared hunk, so they apply cleanly in either order. No shared state either. Worth merging #11932 regardless - passing a parameter and then ignoring it is the kind of thing that bites exactly once, later. Still unbuilt here: the firmware CI has not been released for this PR, so the threshold change has not been compiled upstream. The arithmetic is |
The movement threshold is a number of raw counts, and raw counts mean different rotation rates on different parts - a dual-IMU board is free to pair two unrelated sensors. Passing the constant unchanged asked the secondary for the same number rather than the same physical stillness, so a more sensitive secondary could fail a calibration the primary passes. Converting through both scales asks for the same stillness. Where the two sensors are the same part the scales cancel and the value is exactly the old constant, so nothing changes on the boards this has been tested on. This is the same class of bug as iNavFlight#11905, which fixes it for the primary by moving the constant into dps. That change and this one both edit gyroStartCalibration(); once it lands, both call sites become the same expression over each sensor's own scale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Asks each gyro for its own scale by index instead of hardcoding the primary. A dual-IMU board may pair two unrelated parts, so the primary's sensitivity is the wrong reference for a secondary sensor. Requested on iNavFlight#11933, which adds a second calibration call site in this function.
|
I have approved the CI here, which may make testing on hardware slightly easier. |
|
RAM / Flash usage vs. base commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #11905 249 targets built. Find your board's
|
2 participants
Problem
@and-sh reported in #10650 that the gyro zero calibration never completes when the gyro is configured for a higher sensitivity than the default. He hit it on a custom board with an ICM45686 while testing different full-scale ranges, and pointed out that the standard deviation is computed from raw readings while the threshold does not depend on the gyro scale. Fixes #10650.
Cause
src/main/sensors/sensors.h:46definesCALIBRATING_GYRO_MORON_THRESHOLD 32in raw counts, andsrc/main/sensors/gyro.c:380passes it unchanged tozeroCalibrationStartV().performGyroCalibration()(gyro.c:403-405) feedsgyroADCRaw[]into the calibrator, andsrc/main/common/calibration.c:159compares the raw-count standard deviation against that fixed number, restarting the window whenever it is exceeded. A gyro with more LSB per dps shows a larger raw deviation for the same physical stillness, so every window fails.Change
sensors.hreplaces the constant withCALIBRATING_GYRO_MORON_THRESHOLD_DPS (32.0f / 16.4f), i.e. 1.95 dps, which is what 32 LSB meant at the default 16.4 LSB/dps.gyroStartCalibration()ingyro.cdivides that bygyroDev[0].scaleand passes the result as the threshold. Drivers withscale = 1.0f / 16.4fkeep exactly 32 LSB; LSM6DXX Gen-V (scale = 0.070f) moves to 27.9 LSB and the fake gyro (0.0625f) to 31.2 LSB.Test
Not run on hardware or SITL. Cause verified by reading
sensors.h:46,gyro.c:380-405andcalibration.c:154-171on maintenance-10.x. Not compiled: no fork CI run exists forfd6d88e, and the upstream "Build firmware" run (https://github.com/iNavFlight/inav/actions/runs/34512924844) is waiting for approval with no jobs.sensors/gyro.cis compiled byflight_imu_unittest(src/test/unit/CMakeLists.txt:20-23); no test references the old macro. Compiled for all targets and the four SITL builds on the fork, green: https://github.com/Raffi1202/inav/actions/runs/34770676315Flash / RAM
Builds clean on all targets. No size comparison yet: the fork build has no baseline for this branch, and the upstream size report runs once CI is released for this PR.
Docs
No documentation change needed: no setting or user-visible procedure changes;
docs/Buzzer.md:15describes the calibration restart on movement and stays correct.Interaction with #11933
#11933 (dual-gyro blackbox logging, targeting
master) adds a secondzeroCalibrationStartV()call in the same function,gyroStartCalibration(), and it passesCALIBRATING_GYRO_MORON_THRESHOLD- the constant this PR removes. Whichever of the two landssecond will either fail to build or leave the secondary gyro on the unscaled threshold, which is
the bug this PR fixes. Raised by @MrScothh on #11933; noted here so whoever merges first sees it.
The clean end state is one expression per sensor over its own
gyroDev[n].scale, since a dual-IMUboard may pair two unrelated parts and the primary's scale is the wrong reference for the
secondary.