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

[wpimath] Add feedforward constant constructor to ElevatorSim #5823

Merged
merged 82 commits into from
Nov 2, 2023

Conversation

narmstro2020
Copy link
Contributor

Adds a subclass of ElevatorSim that uses kG, kV, and kA from sysId to simulate an Elevator.

@narmstro2020
Copy link
Contributor Author

I don't understand what the cpp error is. Something about units, but the units should fine.

@narmstro2020
Copy link
Contributor Author

/format

@narmstro2020
Copy link
Contributor Author

/format

@@ -75,9 +85,11 @@ units::ampere_t ElevatorSim::GetCurrentDraw() const {
// is spinning 10x faster than the output.

// v = r w, so w = v / r
Copy link
Member

Choose a reason for hiding this comment

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

This comment doesn't belong to the newly added code.

@narmstro2020
Copy link
Contributor Author

/format

@narmstro2020
Copy link
Contributor Author

I think this is done. Is there anything else I need to finish?

@calcmogul
Copy link
Member

calcmogul commented Nov 2, 2023

Should that breaking change be made in other sim classes?

@narmstro2020
Copy link
Contributor Author

Does that change affect other sim classes?

Not yet. I have other PR's waiting for those. But I haven't submitted yet.

@calcmogul calcmogul added the breaking Introduces a breaking change. label Nov 2, 2023
Copy link
Member

@calcmogul calcmogul left a comment

Choose a reason for hiding this comment

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

Note that this PR makes a breaking change: the gearing and drum radius arguments were removed from the constructor. This makes the simulation class easier to use since that information is already inherent in the linear system argument.

@PeterJohnson PeterJohnson merged commit ddc8db6 into wpilibsuite:main Nov 2, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Introduces a breaking change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants