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] Cleanup and Enhancement of DCMotor and gearboxes #7435

Closed
wants to merge 8 commits into from

Conversation

narmstro2020
Copy link
Contributor

@narmstro2020 narmstro2020 commented Nov 26, 2024

So in an effort to clean up and enhance DCMotor and the whole concept of a gearbox I decided to do the following.

Summary of changes.

Java changes

  1. DCMotor is only focused on the constants for 1 motor and doesn't even know what a reduction is.
  2. The motor constants are internally stored in the Java units library. They are still public final. I questioned this, but realized that it is of no significant effort to call baseMagnitude() on any of those fields. If you are wanting those values then the units library should be pretty straightforward to navigate.
  3. The static methods for each known type have been replaced with an enum called KnownDCMotor. (I'm up for better name suggestions)
  4. Gearbox class that contains a DCMotor, number of motors, and a gearbox reduction.
  5. Constructor overloads for Gearbox exist for combos involving 1 motor with reduction of 1. multiple motors but reduction of 1. 1 motor with reduction other than 1 and full constructor.
  6. Gearbox reduction can still be changed as per the requirements of DifferentialDrive and related classes.
  7. getTorque, Voltage, Current, and AngularVelocity methods for both units and non units class usages.
  8. DCMotorSim renamed to GearboxSim. Simulating a simple single motor with no gearing would be handled with GearboxSim as well.
  9. LinearSystemId static methods reworked. All none drivetrain sims will eventualy use one of the kv, ka methods after calculating them in their own methods and then call idenfityPosition or identifyVelocity. createDCMotorSystem replaced with createGearboxSystem.

C++ changes.
... coming soon. Currently in production.

As per usual any feedback is welcome and appreciated.

@narmstro2020 narmstro2020 requested review from a team as code owners November 26, 2024 03:10
@narmstro2020 narmstro2020 changed the title [wpimath] Separating concerns for DCMotor and gearboxes [wpimath] Cleanup and Enhancement of DCMotor and gearboxes Nov 26, 2024
@narmstro2020
Copy link
Contributor Author

/format

@spacey-sooty
Copy link
Contributor

The static methods for each known type have been replaced with an enum called KnownDCMotor. (I'm up for better name suggestions)

Why? Factories seemed like a good approach to me

@narmstro2020
Copy link
Contributor Author

narmstro2020 commented Nov 27, 2024

The static methods for each known type have been replaced with an enum called KnownDCMotor. (I'm up for better name suggestions)

Why? Factories seemed like a good approach to me

Since they are constants enums seemed like a better choice for me as that's the primary purpose of an enum, to provide a simplified way of handling constants. Also once I separated out numMotors from DCMotor and integrated it into gearbox the static factory methods didn't seem necessary as they no longer had an input argument.

Plus adding a new motor would be super simple. Just add it to the next row of enum Values

@narmstro2020
Copy link
Contributor Author

I'm going to go ahead and close this one. May revisit later.

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