-
Notifications
You must be signed in to change notification settings - Fork 612
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
[wpimath] Add lastValue() method to filters #6351
Conversation
We should probably name this lastValue to avoid confusion.
…On Tue, Feb 6, 2024 at 1:15 PM N0tACyb0rg ***@***.***> wrote:
@N0tACyb0rg <https://github.com/N0tACyb0rg> requested review from
@wpilibsuite/wpilib on: #6351
<#6351> [wpimath] Add
getValue method for SlewRateLimiter as a code owner.
—
Reply to this email directly, view it on GitHub
<#6351 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFSPMR7WUT3FGQQALPBKVXDYSJXNBAVCNFSM6AAAAABC4OZ4DKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRG4ZDEMZUGEYDOMI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Something like that? Or should it just be lastValue? |
I prefer |
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.
I actually ended up prefering lastValue()
to getLastValue()
as well, changed it to that.
If we're going to add this, we should add it to all filter classes that can support it (i.e., they've already cached the latest output internally). LinearFilter would return |
Okay, I'll start working on it. |
I added |
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.
Now there's a mix of GetValue() and LastValue(). Which do we want?
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.
Sorry about that, brains on autopilot are fun. 😅
That should be the last thing. Let me know if it's good to go and I'll squash the commits. |
wpimath/src/main/java/edu/wpi/first/math/filter/SlewRateLimiter.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/filter/LinearFilter.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/filter/MedianFilter.java
Outdated
Show resolved
Hide resolved
I squashed the commits, they should be ready to merge now. |
We autosquash PRs before merge anyway. |
Adds the getValue method to return the current value of a SlewRateLimiter without recalculating the value.