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

Deep copy swerve module states before returning them #5123

Closed

Conversation

person4268
Copy link
Contributor

Fixes #5029. I don't mind taking a different approach if needed.

@person4268 person4268 requested a review from a team as a code owner February 21, 2023 05:44
@person4268
Copy link
Contributor Author

person4268 commented Feb 21, 2023

The CI failures seem to be network errors and unrelated to this PR. (./gradlew :wpimath:check builds and works fine on my machine)

@hadley31
Copy link
Contributor

Based on discussion from #5134, I'd suggest repurposing this PR to remove the member variable entirely, since it is not used anywhere, and would remove the need to deep copy.

@person4268
Copy link
Contributor Author

Based on discussion from #5134, I'd suggest repurposing this PR to remove the member variable entirely, since it is not used anywhere, and would remove the need to deep copy.

I'm tired, so I may be wrong, but isn't the member used to persist angle in case you command an all-zero ChassisSpeeds? That's why it exists, iirc.

@hadley31
Copy link
Contributor

Ahh, I do see that now. However, I would argue that should be handled by the user, rather than the library, since it could possibly be used to calculate speeds other than the current robot state

@Starlight220
Copy link
Member

If it's not used anywhere, what caused the issue that prompted this PR in the first place?

@calcmogul
Copy link
Member

Superseded by #5398.

@calcmogul calcmogul closed this Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants