-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Made convenience functions for tuning a CustomPIDController on SmartDashboard #197
base: main
Are you sure you want to change the base?
Changes from all commits
68569af
94a0a10
bd4066b
96f155c
592631d
ed9a6f1
ed6f952
ca4d886
9e575f6
ff15bcd
c02d92f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
#Sun Jan 06 15:07:52 PST 2019 | ||
gradle.version=5.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<projectDescription> | ||
<name>standard</name> | ||
<comment>Project standard created by Buildship.</comment> | ||
<projects> | ||
</projects> | ||
<buildSpec> | ||
<buildCommand> | ||
<name>org.eclipse.buildship.core.gradleprojectbuilder</name> | ||
<arguments> | ||
</arguments> | ||
</buildCommand> | ||
</buildSpec> | ||
<natures> | ||
<nature>org.eclipse.buildship.core.gradleprojectnature</nature> | ||
</natures> | ||
</projectDescription> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
connection.project.dir= | ||
eclipse.preferences.version=1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"currentLanguage": "none", | ||
"enableCppIntellisense": false, | ||
"projectYear": "none", | ||
"teamNumber": 4904 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
plugins { | ||
id "java" | ||
id "edu.wpi.first.GradleRIO" version "2019.1.1" | ||
} | ||
|
||
def ROBOT_MAIN_CLASS = "frc.robot.Main" | ||
|
||
// Define my targets (RoboRIO) and artifacts (deployable files) | ||
// This is added by GradleRIO's backing project EmbeddedTools. | ||
deploy { | ||
targets { | ||
roboRIO("roborio") { | ||
// Team number is loaded either from the .wpilib/wpilib_preferences.json | ||
// or from command line. If not found an exception will be thrown. | ||
// You can use getTeamOrDefault(team) instead of getTeamNumber if you | ||
// want to store a team number in this file. | ||
team = frc.getTeamNumber() | ||
} | ||
} | ||
artifacts { | ||
frcJavaArtifact('frcJava') { | ||
targets << "roborio" | ||
// Debug can be overridden by command line, for use with VSCode | ||
debug = frc.getDebugOrDefault(false) | ||
} | ||
// Built in artifact to deploy arbitrary files to the roboRIO. | ||
fileTreeArtifact('frcStaticFileDeploy') { | ||
// The directory below is the local directory to deploy | ||
files = fileTree(dir: 'src/main/deploy') | ||
// Deploy to RoboRIO target, into /home/lvuser/deploy | ||
targets << "roborio" | ||
directory = '/home/lvuser/deploy' | ||
} | ||
} | ||
} | ||
|
||
// Set this to true to enable desktop support. | ||
def includeDesktopSupport = false | ||
|
||
// Maven central needed for JUnit | ||
repositories { | ||
mavenCentral() | ||
} | ||
|
||
// Defining my dependencies. In this case, WPILib (+ friends), and vendor libraries. | ||
// Also defines JUnit 4. | ||
dependencies { | ||
compile wpi.deps.wpilib() | ||
compile wpi.deps.vendor.java() | ||
nativeZip wpi.deps.vendor.jni(wpi.platforms.roborio) | ||
nativeDesktopZip wpi.deps.vendor.jni(wpi.platforms.desktop) | ||
testCompile 'junit:junit:4.12' | ||
} | ||
|
||
// Setting up my Jar File. In this case, adding all libraries into the main jar ('fat jar') | ||
// in order to make them all available at runtime. Also adding the manifest so WPILib | ||
// knows where to look for our Robot Class. | ||
jar { | ||
from { configurations.compile.collect { it.isDirectory() ? it : zipTree(it) } } | ||
manifest edu.wpi.first.gradlerio.GradleRIOPlugin.javaManifest(ROBOT_MAIN_CLASS) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Manifest-Version: 1.0 | ||
Main-Class: frc.robot.Main | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
import org.usfirst.frc4904.standard.custom.sensors.PIDSensor; | ||
import edu.wpi.first.wpilibj.PIDSource; | ||
import edu.wpi.first.wpilibj.PIDSourceType; | ||
import edu.wpi.first.wpilibj.smartdashboard.SmartDashboard; | ||
import edu.wpi.first.hal.util.BoundaryException; | ||
|
||
/** | ||
|
@@ -28,6 +29,7 @@ public class CustomPIDController extends MotionController { | |
protected double lastErrorDerivative; | ||
protected double derivativeTolerance; | ||
protected double minimumNominalOutput = 0.0; | ||
protected static final String DEFAULT_SMARTDASHBOARD_PREFIX = "PID"; | ||
|
||
/** | ||
* An extremely basic PID controller. | ||
|
@@ -421,12 +423,77 @@ public double get() { | |
} | ||
} | ||
|
||
/** | ||
* Put the PID controller to SmartDashboard for tuning. | ||
* Puts the error, setpoint, sensor value, and output. | ||
* This method adds a very small random value to everything | ||
* so that graphs show properly on SmartDashboard. | ||
* | ||
* @param prefix | ||
* The prefix to use when putting things on SmartDashboard. | ||
*/ | ||
@Override | ||
public void putToSmartDashboard(String prefix) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be added as an abstract function to MotionController (same for the other functions). |
||
double noise = (Math.random() - 0.5) * 0.0000001; // Generate very small noise centered at zero | ||
SmartDashboard.putNumber(prefix + "_Error", getError() + noise); | ||
SmartDashboard.putNumber(prefix + "_Setpoint", getSetpoint() + noise); | ||
SmartDashboard.putNumber(prefix + "_Sensor", getSensorValue() + noise); | ||
SmartDashboard.putNumber(prefix + "_Output", get() + noise); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, did we decide that the mutating effects of calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we have decided to fix that in the future. It should not make too much of a difference, as it does calculate time deltas every time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool -- and the sensor updates fast enough that each few ticks will probably have a different value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is on the list to be fixed. (In fact I have a list of PID changes we need to make that various programmers will be doing before SVR) |
||
} | ||
|
||
/** | ||
* Put the PID controller to SmartDashboard for tuning. | ||
* Puts the error, setpoint, sensor value, and output. | ||
* This method adds a very small random value to everything | ||
* so that graphs show properly on SmartDashboard. | ||
*/ | ||
@Override | ||
public void putToSmartDashboard() { | ||
putToSmartDashboard(CustomPIDController.DEFAULT_SMARTDASHBOARD_PREFIX); | ||
} | ||
|
||
/** | ||
* Update the PID controller from SmartDashboard for tuning. | ||
* Gets the P, I, D, and F constants from SmartDashboard, | ||
* or puts the current ones there if there's no constants. | ||
* | ||
* @param prefix | ||
* The prefix to use when putting things on SmartDashboard. | ||
*/ | ||
@Override | ||
public void updateFromSmartDashboard(String prefix) { | ||
// If we're missing any constants on SmartDashboard, put the current ones | ||
if (!(SmartDashboard.containsKey(prefix + "_P") && SmartDashboard.containsKey(prefix + "_I") | ||
&& SmartDashboard.containsKey(prefix + "_D") && SmartDashboard.containsKey(prefix + "_F"))) { | ||
SmartDashboard.putNumber(prefix + "_P", getP()); | ||
SmartDashboard.putNumber(prefix + "_I", getI()); | ||
SmartDashboard.putNumber(prefix + "_D", getD()); | ||
SmartDashboard.putNumber(prefix + "_F", getF()); | ||
return; | ||
} | ||
setPIDF(SmartDashboard.getNumber(prefix + "_P", getP()), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want to use the two-argument version with a default value? We've done this in the past while testing (with default 0). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a good way of doing it, it will default to the previous P/I/D/F value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is what I'm doing...? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes -- I have... room to improve. |
||
SmartDashboard.getNumber(prefix + "_I", getI()), | ||
SmartDashboard.getNumber(prefix + "_D", getD()), | ||
SmartDashboard.getNumber(prefix + "_F", getF())); | ||
} | ||
|
||
/** | ||
* Update the PID controller from SmartDashboard for tuning. | ||
* Gets the P, I, D, and F constants from SmartDashboard, | ||
* or puts the current ones there if there's no constants. | ||
*/ | ||
@Override | ||
public void updateFromSmartDashboard() { | ||
updateFromSmartDashboard(CustomPIDController.DEFAULT_SMARTDASHBOARD_PREFIX); | ||
} | ||
|
||
public boolean derivativeOnTarget() { | ||
return derivativeTolerance == 0 || Math.abs(lastErrorDerivative) < derivativeTolerance; | ||
} | ||
|
||
@Override | ||
public boolean onTarget() { | ||
return super.onTarget() && derivativeOnTarget(); | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
distributionBase=GRADLE_USER_HOME | ||
distributionPath=permwrapper/dists | ||
distributionUrl=https\://services.gradle.org/distributions/gradle-5.0-bin.zip | ||
zipStoreBase=GRADLE_USER_HOME | ||
zipStorePath=permwrapper/dists |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this actually works, I think it is great. I would watch out for improper nonalphanumeric character parsing in NetworkTables though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we change it to this? It's pretty jenk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This did not exist before. It is for the bang (!) bang (!) controller. We should make a shebang controller at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As funny as that is it should probably be
BangBang
orBANGBANG