wpilibsuite / allwpilib

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

[wpiunit] Add Frequency and Measure.inverse() #6684

Closed WispySparks closed 1 month ago

WispySparks commented 4 months ago

Partially addresses #6683. I'm wondering if it would instead be preferrable to have an inverse() method on Unit that can be overridden by child units. So Velocity and Per would override it instead of having a reciprocal method and Time could override it to give back a frequency. This would then simplify Measure.inverse(). Do you think that's better than the way I've done it now?

WispySparks commented 4 months ago

Yeah I kinda want to not have Frequency be its own type and also use the divide method in inverse since it has dimensional analysis, I'm interested to hear what others think

bhall-ctre commented 4 months ago

and also use the divide method in inverse since it has dimensional analysis

I think there might be missing cases for:

But those should probably be fixed anyways, after which inverse() could just return Value.of(1).divide(meas).

Edit: Opened up a PR to fix these missing cases: https://github.com/wpilibsuite/allwpilib/pull/6696

PeterJohnson commented 1 month ago

Superseded by #6958.