wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.03k stars 607 forks source link

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

Open narmstro2020 opened 3 weeks ago

narmstro2020 commented 3 weeks ago

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.

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 commented 3 weeks ago

@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 commented 3 weeks ago

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.