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

Create a SwerveModule class #32

Open
danielbrownmsm opened this issue Jan 2, 2023 · 1 comment
Open

Create a SwerveModule class #32

danielbrownmsm opened this issue Jan 2, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@danielbrownmsm
Copy link
Collaborator

Summary
Initially in issue #11 (Add Swerve Drive Support), I didn't think we needed a SwerveModule class, as we could go without it, and I didn't really feel like writing one/know what would be in one. Coding further into it though, especially using the WPILib swerve drive support and following the example code, a lot of repetitive and not-very-fun-to-read code could be simplified using a SwerveModule class. The class should have getters and setters for all the WPILib swerve odometry and kinematics stuff (SwerveModuleState, SwerveModulePosition, etc), PID controllers for turn and steer, parameter loading support, and sensible default values.

Unsure whether to write a specific unit test for the SwerveModule class, as that seems like it would get tested in the SwerveDrive subsystem tests anyways. Might be worth it though.

Work Required

  • Create a SwerveModule class
  • Rewrite SwerveDrive subsystem to use the Swerve Module class (although this is more Add Swerve Drive Support #11's domain)

Verification

  • CI passes
  • Code is sufficiently commented and readable
@danielbrownmsm danielbrownmsm added the enhancement New feature or request label Jan 2, 2023
@danielbrownmsm danielbrownmsm added this to the Enhanced Functionality milestone Jan 2, 2023
@danielbrownmsm danielbrownmsm self-assigned this Jan 2, 2023
@jkleiber
Copy link
Contributor

jkleiber commented Jan 3, 2023

Just as a note, its probably easier to write unit tests for the SwerveModule class than the entire subsystem, and imo would be quite valuable since subsystems are harder to test (due to some functions being real time)

@danielbrownmsm danielbrownmsm linked a pull request Jan 4, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants