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

[wpilib] DCMotorSim cleanup/enhancement #7021

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

narmstro2020
Copy link
Contributor

@narmstro2020 narmstro2020 commented Sep 2, 2024

This PR reworks DCMotorSim similar to what was done with FlywheelSim in PR #6629

@narmstro2020 narmstro2020 requested a review from a team as a code owner September 2, 2024 01:59
@narmstro2020 narmstro2020 changed the title [wpilib] FlywheelSim cleanup/ehancement [wpilib] DCMotorSim cleanup/ehancement Sep 2, 2024
@narmstro2020 narmstro2020 changed the title [wpilib] DCMotorSim cleanup/ehancement [wpilib] DCMotorSim cleanup/enhancement Sep 2, 2024
@narmstro2020
Copy link
Contributor Author

What's the issue here with the failing test?

@calcmogul
Copy link
Member

calcmogul commented Sep 3, 2024

That test doesn't use the classes you modified, so the 2 inch error in y is due to the randomness injected into the test, not due to your PR. We should hardcode the random seed for testing purposes in a separate PR.

@narmstro2020
Copy link
Contributor Author

That test doesn't use the classes you modified, so the 2 inch error in y is due to the randomness injected into the test, not due to your PR. We should hardcode the random seed for testing purposes in a separate PR.

So nothing I need to do about the test for this PR?

@narmstro2020
Copy link
Contributor Author

Does anything need to be modified here?

@narmstro2020
Copy link
Contributor Author

/format

@narmstro2020
Copy link
Contributor Author

@calcmogul Are there any more changes to be made on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants