Skip to content
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

Fixed SoftwarePIDController checkTolerance and initTable #106

Merged
merged 1 commit into from
Feb 8, 2017
Merged

Fixed SoftwarePIDController checkTolerance and initTable #106

merged 1 commit into from
Feb 8, 2017

Conversation

rothso
Copy link
Contributor

@rothso rothso commented Feb 6, 2017

Hey again! My co-lead and I came across what we believe are two separate bugs in the SoftwarePIDController. Seeing as they're each relatively small to review, I hope you don't mind that I've combined them both into one PR.

  1. The checkTolerance method is inaccurate. It checks to see if the input is less than the setpoint minus the tolerance, which means basically any input less than the setpoint is considered in tolerance. This had the ugly effect of causing our controller commands to prematurely finish. I've thrown in a regression test that verifies the method now works as intended.

  2. The gains don't appear on the SmartDashboard. It looks like this.table = table is the culprit :) I compared this line to the corresponding line in the WPILib PIDController and I believe it should have been set to subtable.

* Added a unit test for verifying the checkTolerance logic
* Updated both SoftwarePIDController and the base method in Controller
* Fixed a problem where no gains would be sent to the SmartDashboard
@rhauch rhauch merged commit 2984158 into strongback:master Feb 8, 2017
@rhauch
Copy link
Member

rhauch commented Feb 8, 2017

@rothso Thanks for the fixes!

@rothso rothso deleted the pid-fixes branch February 8, 2017 17:23
CarlinWilliamson added a commit to Team3132/strongback-java that referenced this pull request Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants