Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

ValueProvider observers and PID fixes#53

Merged
rcahoon merged 2 commits into
pid-extensionsfrom
rcahoon/pid-extensions
Mar 11, 2024
Merged

ValueProvider observers and PID fixes#53
rcahoon merged 2 commits into
pid-extensionsfrom
rcahoon/pid-extensions

Conversation

@rcahoon

@rcahoon rcahoon commented Feb 29, 2024

Copy link
Copy Markdown
Member

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]

@rcahoon rcahoon requested a review from dejabot February 29, 2024 04:54
@rcahoon rcahoon force-pushed the rcahoon/pid-extensions branch from b1a7e4f to ce431ab Compare February 29, 2024 08:42
@rcahoon rcahoon force-pushed the rcahoon/pid-extensions branch from ce431ab to 8d2604e Compare February 29, 2024 09:39

@dejabot dejabot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

awesome!

Comment thread src/main/java/com/team766/config/AbstractConfigValue.java
configPrefix += ".";
}
this.pidController = PIDController.loadFromConfig(configPrefix + "pid.");
this.pidController = new PIDController();

Copy link
Copy Markdown
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
Copy Markdown
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

Comment thread src/main/java/com/team766/hal/PIDSlotHelper.java Outdated
Comment thread src/main/java/com/team766/hal/PIDSlotHelper.java Outdated
@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
@rcahoon rcahoon deleted the rcahoon/pid-extensions branch April 13, 2024 20:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants