[peterbarker]

Summary

Fix the vertical controller oscillating between zero and full throttle
whenever vibration compensation is active. The NEU to NED conversion
(aa69e79) dropped the / GRAVITY_MSS from the throttle-per-acceleration
gain in AC_PosControl::get_throttle_with_vibration_override(). This made the
vibration-override feed-forward and integrator gains about 9.8x too large.
Copter-4.7.0 and 4.7.1 are affected. 4.6.x is not.

Classification & Testing (check all that apply and add your own)

• Checked by a human programmer
• Non-functional change
• No-binary change
• Infrastructure change (e.g. unit tests, helper scripts)
• Automated test(s) verify changes (e.g. unit test, autotest)
• Tested manually, description below (e.g. SITL)
• Tested on hardware
• Logs attached
• Logs available on request

The new autotest Copter.VibrationCompensationThrottle takes off in LOITER,
adds an accelerometer Z bias (the same trigger as VibrationFailsafe), and
waits for "Vibration compensation ON". It then records VFR_HUD throttle at
10Hz for 30s and fails if more than 20 sample-to-sample steps exceed 15%. The
faked bias makes the EKF reset its height estimate about every 13s. That
causes a few large steps even when the controller is behaving correctly, so
the test allows some.

Three runs per build:

Build	Steps > 15% (of ~300)	Result
master	51, 44, 44	FAIL
this PR	5, 5, 5, 6	PASS
54f1b3d (just before the NEU to NED series)	6, 5, 5	PASS

The parent of aa69e79 does not compile because it is mid-series, so I
used the commit just before the series as the "before regression" build.
The existing VibrationFailsafe test also passes with this PR.

Description

The bug was found while analysing a flight log from a quad on 4.8.0-dev
(log available on request). A battery failsafe RTL was followed one second
later by "Vibration compensation ON" (VibeZ ~40, heavy clipping). From then
until landing, CTUN.ThO pulsed between 0 and 1 with a period of about
1-1.5s, and PSCD.TAD swung by about ±40 m/s².

Recomputing the throttle from the logged target acceleration with the
current formula (hover - 0.25 * hover * TAD - I, MOT_THST_HOVER=0.125)
reproduces the logged ThO closely. With / GRAVITY_MSS restored, the same
targets produce commands of only about 0 to 0.23:

t (s)	PSCD.TAD	CTUN.ThO	current formula	with / g
324.55	-25.8	1.00	0.90	0.18
325.65	-41.9	1.00	1.40 (sat)	0.23
326.05	+43.3	0.00	-1.26 (sat)	-0.05 (sat)
370.85	-12.8	0.51	0.49	0.13

With the correct gain, the vibration-mode velocity loop has an effective
bandwidth of about 0.25 * PSC_D_VEL_P = 1.25 rad/s. The 9.8x error raises
that to about 12 rad/s. That is right at the 2Hz throttle cutoff, before
adding motor lag and the lag in the vertical velocity estimate used in
high-vibration mode, so the loop becomes unstable. Hover throttle cancels
out of the loop gain, so the problem affects every airframe.

Before the NEU to NED series, the other scalings in this function were
(thr / 100) * 1000 on the integrator and * 0.001 on the output. Those
cancel each other and were correctly removed. Only the gravity divisor was
lost. The fix restores it, which returns the gains to their 4.6 values in
SI units.

Commits:

1. AC_AttitudeControl: restore gravity scaling in vibration override throttle
2. autotest: add test for throttle oscillation under vibration compensation

This should be considered for backport to 4.7.

The bit which broke it:

aa69e79#diff-9fe7f1dfc4ef9b28922f55104d879c4b78656bc70293cde53f3c114dad535eeeL1456

(the variable name there loses unit-correctness and doesn't get it back)

[github-actions]

Global Size Summary (Total Flash delta in bytes)

Built from 280b87a rebased onto e21a6ae

Board	Periph	tracker	blimp	bootloader	copter	heli	iofw	plane	rover	sub
DrotekP3Pro	_	_	_	*	_	_	_	_	_	_
Durandal	_	*	*	_	+8	+8	_	+8	*	+8
Hitec-Airspeed	*	_	_	_	_	_	_	_	_	_
KakuteF7	_	*	*	_	+8	+8	_	+8	*	+8
MambaH743v4	_	*	*	_	+8	+8	_	+8	*	+8
MatekF405-CAN	_	_	_	*	_	_	_	_	_	_
MatekF405	_	*	*	_	+8	+8	_	+8	*	+8
MatekH743-bdshot	_	*	*	_	+8	+8	_	+8	*	+8
Pixhawk1-1M	_	*	*	_	+8	+8	_	+8	*	+8
disco	_	*	*	_	+8	+8	_	+8	*	+12
f103-GPS	*	_	_	_	_	_	_	_	_	_

[andyp1per]

Yikes! Good catch

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-10-04)

Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.
Verdict: COMMENT

Reviewed at head 2ad11caf92.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/ardupilot/34607/1.html#pr34607

Thanks, this is a real regression and the fix looks right. In the NED conversion (aa69e79), / GRAVITY_MSS was dropped together with the cm and x1000 factors. That left the vibration-compensation feedforward and integrator about 9.8 times too hot. Restoring it (AC_PosControl.cpp:1472) brings both terms back to the pre-conversion gains. The new autotest fails reliably with the line reverted (45-49 large steps against a limit of 20) and passes with it in (4-6).

One small, nonblocking suggestion for the test: it never checks that VFR_HUD samples were actually received (arducopter.py:2074). As written, zero or very few samples pass. A guard such as if len(steps) < 200: raise NotAchievedException(...), like the 'insufficient samples' check at arducopter.py:17769, would stop telemetry problems from turning into a silent pass.

For maintainers: the regression is in Copter-4.7.0/4.7.1, so the AC_AttitudeControl commit looks like a 4.7 backport candidate.

[rmackay9]

Oh my, I suspect this may be the cause of this support discussion where Sokerer talks about an alternating throttle

[peterbarker]

Oh my, I suspect this may be the cause of this support discussion where Sokerer talks about an alternating throttle

Exactly what I saw on my vehicle.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-10-05)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: REQUEST CHANGES

Reviewed at head 06817b997f.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/ardupilot/34607/2.html#pr34607

The vibration fix is still right. The gravity divisor at AC_PosControl.cpp:1472 is unchanged, and thanks for adding the minimum-sample guard to the test (arducopter.py:2075). The tests pass in SITL at 06817b9.

The PR has since picked up six AP_AHRS/autotest commits, and one of them breaks builds with DCM compiled out. AP_AHRS.cpp:1336 returns EKFType::DCM outside an #if AP_AHRS_DCM_ENABLED guard. A SITL copter build with --define AP_AHRS_DCM_ENABLED=0 fails there ("'DCM' is not a member of 'AP_AHRS::EKFType'"), while merge-base builds and links. Wrapping that if (prefer_dcm_over(...)) block in #if AP_AHRS_DCM_ENABLED fixes it.

Smaller points, none blocking:

• The description still lists only the two vibration commits and asks for a 4.7 backport. Could the AHRS refactor go in its own PR, or at least be described here? Then the fix can be reviewed and backported on its own.
• The NOT_INITIALISED bit set for unstarted EKFs (AP_AHRS_NavEKF3.cpp:235) can't change selection, because backend_healthy_for_active() is only reached after the started early return. The commit message and comment overstate what it does.
• The fallback table and loop are duplicated at AP_AHRS.cpp:1261 and AP_AHRS.cpp:1509. A small shared helper would keep a single preference order.

[AP-Review]

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: ACCEPT

Reviewed at head 280b87aab1.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/ardupilot/34607/3.html#pr34607

Thanks for dropping the AHRS refactor from this PR. The branch is now just the two vibration-fix commits, patch-identical to the ones reviewed before, so the earlier DCM-disabled build break and the AHRS side points no longer apply.

The fix still looks right: restoring / GRAVITY_MSS at AC_PosControl.cpp:1472 puts the vibration-override gains back where they were before the NED conversion. The new test passes at 280b87a in SITL (3-5 large steps out of 300) and fails with the fix reverted (46). flake8 and the branch checks are clean. This looks ready, and still a good 4.7 backport candidate.

Not checked: hardware or flight logs, the full Copter suite, and CI, which was still pending at this head.

[andyp1per]

I'm in two minds about the need to add an autotest here, I mean adding one for a single line change is overkill. Also I have always seen throttle oscillation with vibration compensation, even on older builds - this just makes it 10 time worse (literally). So I am not even sure the test is that correct.

[rmackay9]

Thanks very much for the fix! I agree in general with @andyp1per's comment that we may be overdoing the CI tests but on the other hand this is a rarely used part of the code so having a good autotest for this area is probably a good idea.

[rmackay9]

This is included in 4.7.3-beta1