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

ValueProvider observers and PID fixes #53

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

rcahoon
Copy link
Member

@rcahoon rcahoon commented Feb 29, 2024

Description

Add a system to register for notifications when a ValueProvider's value changes. Use it in PID gains for MotorControllers

Also a few assorted fixes for #49

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Be detailed so that your code reviewer can understand exactly how much and what kinds of testing were done, and which might still be worthwhile to do.

  • Unit tests: [Add your description here]
  • Simulator testing: [Add your description here]
  • On-robot bench testing: [Add your description here]
  • On-robot field testing: [Add your description here]

Copy link
Contributor

@dejabot dejabot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!

configPrefix += ".";
}
this.pidController = PIDController.loadFromConfig(configPrefix + "pid.");
this.pidController = new PIDController();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not read this from the config file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the other MotorControllers don't read their PID gains from the config file.

This is now accomplished (more generically and more correctly) by configurePID in RobotProvider

src/main/java/com/team766/hal/PIDSlotHelper.java Outdated Show resolved Hide resolved
src/main/java/com/team766/hal/PIDSlotHelper.java Outdated Show resolved Hide resolved
@rcahoon rcahoon closed this Mar 1, 2024
@rcahoon rcahoon reopened this Mar 1, 2024
@rcahoon rcahoon force-pushed the rcahoon/pid-extensions branch 6 times, most recently from edda3f7 to 619e804 Compare March 1, 2024 15:40
@rcahoon rcahoon merged commit 28a84c7 into pid-extensions Mar 11, 2024
4 checks passed
@rcahoon rcahoon deleted the rcahoon/pid-extensions branch April 13, 2024 20:51
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