Wizard: MaxSteer as invariant raw counts, MaxPWM = 2 × MinPWM default#390
Draft
Xyntexx wants to merge 2 commits into
Draft
Wizard: MaxSteer as invariant raw counts, MaxPWM = 2 × MinPWM default#390Xyntexx wants to merge 2 commits into
Xyntexx wants to merge 2 commits into
Conversation
The legacy MaxSteerAngle was an int field on AutoSteerConfig storing degrees. The wizard measured a plateau angle from PGN 253 (post-CPD- conversion) and saved that degree value, so running CPD calibration later left the stored max-steer wrong by ratio actualCPD/defaultCPD. Bench-test on AgOpenGPS-Official#388 confirmed this — operators running the wizard in the default order (CPD after max-steer) got a max-steer setting that clipped guidance corrections far inside the real mechanical limit. Switch the storage primitive to raw WAS counts and make MaxSteerAngle (degrees) a computed view of MaxSteerRawCounts / CountsPerDegree. Setting MaxSteerAngle by degrees back-converts to raw counts at the current CPD, so legacy callers (JSON profile load, config dialog spinner) keep working. CPD changes raise PropertyChanged on MaxSteerAngle so the config dialog display refreshes the moment the operator finishes the CPD circle test. MaxSteeringAngleStepViewModel: - Adds MaxSteerRawCounts property alongside MaxSteerAngle. - After plateau capture, computes chosenDeg = min(left, right) x 0.9 then writes MaxSteerRawCounts = chosenDeg x currentCPD. Both the degree view and the raw-counts capture are exposed for the result panel. - OnLeaving persists MaxSteerRawCounts (the source of truth); the config's computed MaxSteerAngle is derived on read.
Two related fixes in the Phase A Kp ramp: N. After the ramp's success branch captures MinPwm from PGN 253, the step also seeds ConfigStore.AutoSteer.MaxPwm to min(2 x MinPwm, 255). Operators previously had to discover the default MaxPwm themselves in the config dialog; the 2x rule of thumb gives them a reasonable starting point that's high enough for the firmware to actually drive the wheel under load, low enough to avoid runaway tuning. Persist-at-capture: moved the MinPwm / InvertMotor save out of OnLeaving and inline at the success branch. The old OnLeaving block was gated on CalibrationCompleted, but RunKpRampAsync never set that flag (Phase B used to; that's split into MaxSteeringAngleStepViewModel now). Result: operators who ran the ramp and then pressed Next saw their Kp-ramp results silently dropped on the floor. Saving inline at the success branch fixes the loss and reads more naturally as a single 'capture + persist' step. Also cleaned up a stale OnLeaving write to autoSteer.MaxSteerAngle. The vestigial _maxSteerAngle field in this VM (left over from the split) defaulted to 0; if CalibrationCompleted ever did get set, the save block would have clobbered the operator's real max-steer calibration with that zero. Removed the line; MaxSteer persistence lives entirely in MaxSteeringAngleStepViewModel now. Tests in MaxSteerRawCountsAndMaxPwmTests: - MotorCal_AfterCapture_SetsMaxPwmToDoubleMinPwm — MinPwm=35 leads to MaxPwm=70. - MotorCal_AfterCapture_CapsMaxPwmAt255 — MinPwm=200 caps MaxPwm at the byte ceiling rather than overflowing. - Companion M tests (raw-counts invariance, plateau capture) live in the same fixture for cohesion.
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.
Two related calibration refinements from bench-test of the wizard.
Issue M — MaxSteer stored as raw counts, degrees computed from current CPD (`ca6fea45`)
The wizard's Max Steering Angle step read `ActualSteerAngle` (post-CPD-conversion degrees) and saved it as `MaxSteerAngle` (also degrees). If CPD circle test ran after max-steer (or didn't run at all), the stored max-angle was wrong by the ratio `actualCPD / defaultCPD` — operator's wizard order shouldn't matter that way.
Net: operator can run CPD circle BEFORE or AFTER max-steer — final `MaxSteerAngle` is correct either way, because the raw-counts capture is CPD-invariant.
Issue N — MaxPwm default = 2 × MinPwm after motor cal (`0404bacc`)
After the Kp-ramp captures MinPwm, also write `MaxPwm = Math.Min(2 × MinPwm, 255)`. Sensible default; operator can tune in the AutoSteer config dialog if needed.
While in that area, found and fixed two real bugs:
The result panel now correctly shows both MinPwm and the seeded MaxPwm.
Tests
5 new in `MaxSteerRawCountsAndMaxPwmTests`:
Full Services suite: 903 pass, 1 fail (pre-existing Windows file-lock flake, unrelated), 2 skip.
Non-spec note
The develop motor-cal VM still carries dead Phase B fields (`MaxSteerAngle` property, `MeasuringMaxAngle`/`Complete` enum values, `IsPhaseB0/B1`, `IsComplete`) that aren't wired to AXAML anymore. Harmless but visible. Worth a small follow-up to delete the carcass; left alone here to keep the diff focused on M+N.
Test plan