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

[wpiunits] Improves usage ergonomics and add more unit types #6710

Closed
wants to merge 18 commits into from

Conversation

narmstro2020
Copy link
Contributor

@narmstro2020 narmstro2020 commented Jun 7, 2024

So the main inspiration for this reorganization was to provide a means of creating measures in this form
Measure<LinearAcceleration> measure = RadiansPerSecondPerSecond.of(3.0)
instead of
Measure<Velocity<Velocity<Distance>>> measure = RadiansPerSecondPerSecond.of(3)
which from some loose feedback I've received is a major deterrent for teams/groups/students utilizing this API.

This removes the Velocity class entirely with its functionality being taken up by the new derived classes and the Per type.

Also Power, Energy, and Voltage have been removed as base units since they are derived from others.
The BaseUnits class is more representative now of the actual SI base unit quantities. With Angle and Dimensionless still in
the class for Java language purposes.

In order to make this work please understand the following caveats.

  1. If wanting to make an adhoc (on the fly) unit from a product or quotient of unit types use Mult and Per respectively.
  2. The unit types have or can have multiple times and per methods useful for generating units of the new types.
  3. Types that are derived from either a product or quotient of other types have a 3rd constructor that calls upon a static combine method similar to the combine method in Mult and Per to handle unit type creation.
  4. Static cache variables similar to those found in Mult and Per are added in these new derived types to allow for the combine static methods in those classes to function.
  5. I originally intended to subclass Mult and Per so as not to have code duplication in regards to , but the Java language spec doesn't really allow for this in the current implementation of Mult and Per. There could be a simple fix for this that I haven't discovered yet.
  6. The times and divide methods in the Measure class have been modified to take into account the lack of the Velocity class.
  7. The times method in the Measure class now checks commutativity of the units.

Summary of the new unit types and how they are derived. Sorry if this is appears to be a lesson in physics.

  • AngularVelocity is derived as a quotient of Angle and Time
  • AngularAcceleration is derived as a quotient of AngularVelocity and Time
  • LinearVelocity is derived as a quotient of Distance and Time
  • LinearAcceleration is derived as a quotient of LinearVelocity and Time
  • Force is derived as a product of Mass and LinearAcceleration
  • Energy is derived as a product of Force and Distance
  • Power is derived as a quotient of Energy and Time
  • Voltage is derived as a quotient of Power and Current
  • LinearMomentum is derived as a product of Mass and LinearVelocity
  • AngularMomentum is derived as a product of Distance and LinearMomentum
  • MomentOfInertia is derived as a quotient of AngularMomentum and AngularVelocity
  • Torque is derived as a product of MomentOfInertia and AngularMomentum. (This solved a problem with Energy and Torque having the same unit composition and is a reasonably valid alternative from a physics standpoint).

This document from NIST (National Institute of Standards and Technology) was very helpful in this work

Just to note I have a PR #6676 that has yet to be merged and attempts to add force and torque units. These have been included in this PR and in fact was the inspiration for this PR. I'm leaving PR #6676 open until told what to do with it.

This Measure type for torque in PR #6676 was Measure<Mult<Mult<Mass, Velocity<Velocity<Distance>>>>

As always feedback is requested and appreciated.

EDIT: Changes made. Originally Trapezoid profile had its Velocity constructor removed and replaced with one for both linear and angular velocity. Unfortunately Java's type erasure prevented this as a direct constructor. At the time I removed this constructor entirely. I have now added it back in as two static methods so as to avoid Java type erasure conflicts.

@narmstro2020 narmstro2020 requested review from a team as code owners June 7, 2024 15:34
@narmstro2020
Copy link
Contributor Author

narmstro2020 commented Jun 7, 2024

@PeterJohnson
I've noticed this check has been added that I've failed.
Comment on PR for robotpy / comment (pull_request)

What am I missing for it?

EDIT: also CMake is failing which is interesting since no C++ files were altered and this is a fresh pull from main.

@Gold856
Copy link
Contributor

Gold856 commented Jun 7, 2024

Both the failures are unrelated to your PR, you can ignore them.

Comment on PR for robotpy / comment (pull_request) is failing because you modified a file under wpilibNewCommands and it's supposed to tell you to make a corresponding PR for robotpy.

CMake Windows is failing because it's having problems building the dependencies.

@PeterJohnson
Copy link
Member

Superseded by #6958.

@narmstro2020 narmstro2020 deleted the UnitsReorganization branch August 29, 2024 20:49
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.

3 participants