-
Notifications
You must be signed in to change notification settings - Fork 606
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 full state support to LinearSystemId functions #6554
[wpimath] Add full state support to LinearSystemId functions #6554
Conversation
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.
While what I've written works in principle, would it be better to modify the KalmanFilter, LQR, and LinearSystemLoop classes to take in the LinearSystem and extract what they need instead?
Those classes can take any linear system, so they can't know what to slice unless the user tells them. At that point, the user might as well do the slicing via the LinearSystem method to separate concerns.
wpilibj/src/main/java/edu/wpi/first/wpilibj/simulation/SingleJointedArmSim.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/system/LinearSystem.java
Outdated
Show resolved
Hide resolved
Thanks for the help @calcmogul |
…ck on c++ slice method. started
@calcmogul |
Declare the parameter pack like this: https://godbolt.org/z/aoz7YYjfo. Use "Array of indices" from https://eigen.tuxfamily.org/dox/group__TutorialSlicingIndexing.html to slice the matrices. If you want to sort and deduplicate the indices first, copy them into a std::array via |
@calcmogul |
I got it to compile, but the precondition checks need some work still. |
@calcmogul |
If the user adds a duplicate index, the function uses the wrong output dimension for the return type (too large). We should probably just throw an error on duplicates. |
/format |
/format |
Simple checks for duplication. |
How is this one looking. Anything else I need to do? |
wpimath/src/main/java/edu/wpi/first/math/system/LinearSystem.java
Outdated
Show resolved
Hide resolved
Thank you. Requested changes made and committed. |
wpimath/src/main/java/edu/wpi/first/math/system/LinearSystem.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Tyler Veness <calcmogul@gmail.com>
Co-authored-by: Tyler Veness <calcmogul@gmail.com>
/format |
Thanks again. |
…uite#6554) Co-authored-by: Tyler Veness <calcmogul@gmail.com>
Addresses issue #6513
C++ version needs to be done, but I need some feedback before proceeding.
This relies on LinearSystem providing a function to trim the C and D matrices to a single row for use in the KalmanFilter, LinearQuadraticRegular, and LinearSystemLoop classes.
The trimmed version of the LinearSystem is passed into these classes.
This would add more of a implementation burden on users who wish to use this in the classes.
While what I've written works in principle, would it be better to modify the KalmanFilter, LQR, and LinearSystemLoop classes to take in the LinearSystem and extract what they need instead?
DCMotorSim, ElevatorSim, and SingleJointedArmSim have been modified to use the velocity output of the LinearSystem instead the state.