allwpilib
allwpilib copied to clipboard
wpilibsuite
Rewrite DutyCycleEncoder and AnalogEncoder
The existing AnalogEncoder and DutyCycleEncoder classes are massive footguns. They automatically include rollover support, which is often not what people want. Additionally, DutyCycle's rollover support is completely broken and cannot be trusted. It also cannot be easily fixed even at the FPGA level.
For consistency's sake it may also be better to make the offset value "the voltage/position measurement where you would like 0 to be" and subtract the offset from the measured value - this is typically more intuitive to find (even though it's the same number multiplied by -1) and would match the convention used by DutyCycleEncoder currently, as well as many swerve templates/libraries
Clarification: What's the difference between AnalogPotentiometer and this version of AnalogEncoder?
It took me some time to realize that expectedZero is the offset. I understand that this naming is unambiguous whether the offset is done before or after scaling, but I think the word "offset" should still be there in the parameter description and maybe other places.
What's the difference between AnalogPotentiometer and this version of AnalogEncoder
They're basically the same thing at this point. We'd probably deprecate AnalogPotentiometer since actual potentiometers are rarely used anymore.
As for the Zero, Naming things is hard. Some people think offset, some think zero. I've seen it about 50/50 split.
As for the Zero, Naming things is hard. Some people think offset, some think zero. I've seen it about 50/50 split.
So whatever the parameter name is, the description should include the other name as well.
Maybe extract a base AnalogSensor<U> class that accepts a U(volt) conversion function and handles offset+scaling (likely by using the above function) and then it can be used for unit-aware classes of any analog sensor? We can add intuitively-named subclasses for common use cases (encoders, potentiometers, ultrasonics -- #4894, pressure sensors, and so on). This can be a great step for what was desired in #5033.
Maybe extract a base
AnalogSensor<U>class that accepts aU(volt)conversion function and handles offset+scaling (likely by using the above function) and then it can be used for unit-aware classes of any analog sensor? We can add intuitively-named subclasses for common use cases (encoders, potentiometers, ultrasonics -- #4894, pressure sensors, and so on). This can be a great step for what was desired in #5033.
This should all be done later. All this PR should be is fixing up the low level API.