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.

Sign up for GitHub

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

Jump to bottom

[wpimath] Add setters for Feedforward gains #7784

Merged
merged 1 commit into from
Feb 14, 2025
Merged

[wpimath] Add setters for Feedforward gains #7784

merged 1 commit into from
Feb 14, 2025
+189 −11

Conversation

spacey-sooty
Copy link
Contributor

No description provided.

@spacey-sooty spacey-sooty requested a review from a team as a code owner February 14, 2025 03:51
@spacey-sooty spacey-sooty changed the title [wpimath] Add setters to Feedforward gains [wpimath] Add setters for Feedforward gains Feb 14, 2025
@github-actions github-actions bot added the component: wpimath Math library label Feb 14, 2025
@calcmogul
Copy link
Member

Why is your system's feedforward gains changing over time?

@spacey-sooty
Copy link
Contributor Author

Why is your system's feedforward gains changing over time?

It isn't. This allows tuning gains over NT without being locked into Sendable.

@calcmogul
Copy link
Member

Then we should document that these functions should only be used for online gain tuning. Any other use is incorrect.

@spacey-sooty
Copy link
Contributor Author

Then we should document that these functions should only be used for online gain tuning. Any other use is incorrect.

The PID controller setters don't have this?

@spacey-sooty spacey-sooty force-pushed the ff-setters branch 3 times, most recently from 52ad9be to d254f2a Compare February 14, 2025 04:12
@calcmogul
Copy link
Member

calcmogul commented Feb 14, 2025 •
edited
Loading

The PID controller setters don't have this?

We should add it then. Feedback gain scheduling is a symptom of poorly modeling your system, given that most FRC systems are linear.

@PeterJohnson
Copy link
Member

I think we can merge this as-is and debate the comment side of things later?

@spacey-sooty
Copy link
Contributor Author

spacey-sooty commented Feb 14, 2025 •
edited
Loading

I think we can merge this as-is and debate the comment side of things later?

Would be nice if this makes it into the next release, I PRd this after needing these functions for a tuning impl

@PeterJohnson PeterJohnson merged commit 4d126b1 into wpilibsuite:main Feb 14, 2025
45 checks passed
@spacey-sooty spacey-sooty deleted the ff-setters branch February 14, 2025 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calcmogul calcmogul calcmogul requested changes

@PeterJohnson PeterJohnson PeterJohnson approved these changes

Assignees
No one assigned
Labels
component: wpimath Math library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spacey-sooty @calcmogul @PeterJohnson