Fix arduloop anti-windup fidelity gap vs real ArduPilot#10
Merged
Conversation
- arduloop's rate-PID anti-windup 'saturated' gating was architecturally present but never actually populated by any caller (always False), silently disabling anti-windup across all axes in simtests. - HeliRateController now self-computes per-axis saturation each tick, mirroring AP_MotorsHeli_Single::move_actuators: combined roll/pitch cyclic magnitude vs new HeliParams.CYC_MAX_cd (from H_CYC_MAX param, default 2500 centi-degrees), plus yaw output-limit saturation. Stored as self._prev_saturated and fed into the NEXT tick's update_all(limit=...) calls, replicating AP's one-cycle-delayed anti-windup feedback. - Removed the now-dead external 'saturated' parameter from HeliRateController.update() and GuidedAttitudeController.update() (no caller ever supplied it; clean cutover per repo convention). - Fixed a bug in AC_PID debug recording (d.SRate referenced a nonexistent self._output_slew_rate; now uses self._slew_limiter.get_slew_rate()). - Added simulation/analysis/arduloop_pid_replay.py: replays real SITL PID_TUNING desired/achieved samples through arduloop's own AC_PID to isolate controller-math divergence from physics/EKF drift. Confirmed P/D/FF terms already matched real AP almost exactly; I-term diverged (pitch especially) due to the anti-windup gap fixed here. - Extended telemetry schema (telemetry_columns.py, telemetry_csv.py, mock_ardupilot.py, mediator.py) and compare_pumping_sitl_simtest.py with PDmod/SRate columns end-to-end to support this diagnosis. Validated: full unit suite (467 passed), targeted arduloop/guided/heli/pid tests (55 passed). Re-ran SITL test_lua_flight_steady_sitl after the fix: still fails on max_tension (1202N vs 1000N limit), confirming this fix does not touch the real firmware's tension-spike behavior (that remains a separate, open issue) -- as expected since arduloop is simtest-only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
saturatedgating was architecturally present but never actually populated by any live caller (alwaysFalse), silently disabling anti-windup across all axes in simtests.HeliRateControllernow self-computes per-axis saturation each tick, mirroringAP_MotorsHeli_Single::move_actuators: combined roll/pitch cyclic magnitude vs newHeliParams.CYC_MAX_cd(fromH_CYC_MAXparam, default 2500 centi-degrees), plus yaw output-limit saturation. Stored asself._prev_saturatedand fed into the next tick'supdate_all(limit=...)calls, replicating AP's one-cycle-delayed anti-windup feedback.saturatedparameter fromHeliRateController.update()andGuidedAttitudeController.update()(clean cutover, no caller ever supplied it).AC_PIDdebug recording (d.SRatereferenced a nonexistentself._output_slew_rate; now usesself._slew_limiter.get_slew_rate()).simulation/analysis/arduloop_pid_replay.py: replays real SITLPID_TUNINGdesired/achieved samples through arduloop's ownAC_PIDto isolate controller-math divergence from physics/EKF drift.compare_pumping_sitl_simtest.pywithPDmod/SRatecolumns end-to-end.Validation
simulation/tests/unit).test_lua_flight_steady_sitl(real SITL firmware) after the fix: still fails onmax_tension(1202N vs 1000N limit) — confirms this fix does not (and should not) affect real-firmware behavior, since arduloop is simtest-only. The tension-spike issue remains open and separate.Notes
This branch also includes other in-flight working-tree changes present at commit time (see diff) in addition to the arduloop fidelity fix.