wpilibsuite / allwpilib

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

Units API Rewrite #6778

Open narmstro2020 opened 5 days ago

narmstro2020 commented 5 days ago

So this PR is based on feedback and discussion from PR #6710, #6696, #6684, #6676

I know some of those are mine. I will consider those dead and will leave them only for references to the feedback and notes.

I'm going to start with taking the existing units API and breaking it down into separate measurement type packages, angle, time, dimensionless, angularvelocity, etc.

Each will have an a class equivalent to the old Measure interface and ImmutableMeasure class which will handle immutable measures. (i.e. Angle)

Each will have a class that will handle the units of that type (i.e. AngleUnit)

Each class that handles Immutable measures will have a child class that handles mutable ones (i.e. MutableAngle).

At first (and probably at the end) many of the packages will be near duplicates of each other except for minor variations to handle division, multiplication, etc.

For the most part no generics will be used. They might come back in a limited form as this is further optimized, but no guarantees.

Every unit of a certain measurement type will have all of its derived units go back to the S.I. standard unit. As an example feet will be based on meters and inches will be based on meters. New units will not be based off of other ones unless that other one is the S.I. base (or derived as in the case of m/s). I've found it confusing when seeing the word base in the old API and not knowing if S.I. base or a unit that a new one is based on is used.

Units that are derived from the standard S.I. base units will be done so here as well much like the way AngularVelocity is done.

I'll not be focusing on UnitBuilder at this time. I'll effectively deprecate it. Though custom units of a type can still be created through the constructors and static methods of that type.

Of course feeback is welcome. I'll leave this in draft form until I've effectively replaced all of the existing units with the new setup.

WispySparks commented 5 days ago

A rewrite has begun here as well by @SamCarlberg

narmstro2020 commented 5 days ago

@WispySparks Well how about that. I didn't think to check his repos.
It looks he and I are on the same path in our way of thinking.

@SamCarlberg Sorry I didn't get back with you sooner on this. Looking at your work in progress it looks like we're on the same wavelength.

I guess I'll just leave this dead for now.

@SamCarlberg One suggestion is to take EnergyUnit and VoltageUnit out of BaseUnits

SamCarlberg commented 5 days ago

Yes, this is very similar. However, base types will be required to allow classes to be unit-agnostic when necessary:

class ConfigurableThing<U extends Unit> {
  Measure<? extends U> configuredValue;
}

var byDistance = new ConfigurableThing<DistanceUnit>();
byDistance.configuredValue = Feet.of(...);

var byTime = new ConfigurableThing<TimeUnit>();
byTime.configuredValue = Minutes.of(...);

Mutable types inheriting from an immutable base also means you can't get JVM optimizations from final fields, use records, or migrate to value types if project valhalla ever lands. This is why the approach I've been taking uses an interface with mutable and immutable implementations. It also opens up the possibility to codegen (most of) the implementation types.

narmstro2020 commented 5 days ago

Can't wait to see the finished product.

Sorry in my previous comment I just mentioned Energy and Voltage as being separated from Base types just to match up with SI