Stop the Foresight tuner on NaN results and runaways - #112
Open
menaleeperera wants to merge 1 commit into
Open
menaleeperera wants to merge 1 commit into
menaleeperera wants to merge 1 commit into
Conversation
A failed system-identification fit could hand NaN to the later steps: with fewer than two samples between 10% and 80% of the steady speed, linearFit returns a NaN slope, which the `== 0` check lets through. A NaN heading kP makes every power the braking steps send NaN, CachedMotor ignores NaN powers, and the motors keep their last power, so the robot drives off until someone stops it. - The three fits fail on fewer than two samples, or a NaN or wrong-sign slope. - Each step runs through step(): a failure stops the tuning with the step's name and reason. - Values the later steps drive with, and everything in the generated config, are checked: NaN, infinite, or not positive where they must be stops the tuning with a message. - The braking steps stop the robot if it gets 24 in past the test's ends or drifts 48 in across them.
|
We are having the exact same issue with our robot, with mecanum drivetrain with 435rpm Gobilda motors. The heading and direction identification are working, but the robot is travelling much farther than is stated. |
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
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.
Problem
On our mecanum robot (Swyft Drive V2, goBILDA Pinpoint), the Foresight tuner's Forward Braking and
Strafe Braking steps drove off at a constant power and never braked; we had to emergency-stop the
robot both times. This looks like the runaway in Pedro-Pathing/PedroPathing#179.
We traced it through the tuner:
steady speed. In our runs there were none: the first readings of each step were stale (the
Pinpoint was still recalibrating its IMU, which PinpointLocalizer 3.0.1 and the 1 s sleeps added here
now wait for).
linearFitreturns a NaN slope. The check islinReg[1] == 0,which NaN passes, so
tauand heading kP are NaN.they send is NaN.
CachedMotor.setPowerignores NaN powers and keeps the previous one. The motors stay at the step'sstarting power (0.7 forward, 1.0 strafe) through "drive" and "brake" alike.
The IMU wait fixes the trigger we hit, but any fit that fails the same way, or any other step that
produces NaN, still leads to a silent runaway. This PR makes those failures stop the tuning with a
message instead.
Changes
All in
ForesightTuner.java:fewer than two samples, or a NaN or wrong-sign slope, not only on a slope of exactly 0.
step(), so a failure stops the tuning with the step's nameand reason on the tuning page.
in the generated config are checked. A value that is NaN, infinite, or not positive where it must be
stops the tuning with a message naming the step and value, so NaN never reaches the motors or the
pasted config.
OVERRUN(24 in) past thetest's ends, or drifts
DRIFT(48 in) across the test's direction. We saw about 24 in of drift acrossfive strafe passes on a healthy run, hence 48.
No change to the measurements themselves when every step succeeds.
Testing
1 s sleeps. Before the IMU wait: the tuner stopped at the Heading Tuner with "only 0 usable samples"
instead of the robot driving off. With a wait added: all steps completed, every fit found enough
samples, and the braking steps ran normally.
revhub3.0.1,tuning1.0.1).