FRC-1250 / Team-1250-Crescendo2024

MIT License
0 stars 0 forks source link

[REV] Absolute encoder rollover breaks position control continuity #36

Closed Rodney-Lewis closed 6 months ago

Rodney-Lewis commented 6 months ago

Description

While using position control on the rev speed controllers with an absolute encoder attached, the PID controller does not natively handle the encoder rollover (1.0 -> 0.0).

Problem scenario

Assume moving towards home is negative and the home position on an encoder will read 0.

If you attempt to go home and you're beyond your home position, the encoder will rollover to 1. However, the motor will continue in the negative direction as it attempts to get back to 0 by descending from 1 rather than ascending back into the rollover point.

Proposed fixed

The encoder offset should be set so that the rollover point does not exist within the robots normal range of operation. Offset - https://github.com/FRC-1250/Team-1250-Crescendo2024/blob/beta/src/main/java/frc/robot/subsystems/Shoulder.java#L42

Adding 0.5 to all positions and offsets would work for example. The additional value must be consistent in all locations.

qwbmo commented 6 months ago

I'd like us to test this with a very low current limit until we're sure the issue is stable. We verified that it will move with as low as 5 Amps on Wednesday. I'm concerned adjusting the offset won't keep the speed controller from getting confused when the actual sensor rolls over.

Also, can we set the offset such that 0 is pointing straight down (~20° past home)? We can calibrate at 0.25 (1/4 rev, or horizontal) and it'll make it easy for everyone to visualize.

Rodney-Lewis commented 6 months ago

Amp reduction, yes.

Offset value straight down, I think is also doable without much more effort. If the offset is a genuine concern, we can physically move the zero too likely you mentioned. We have to make repairs anyway.

Side note: We should consider setting the SparkMax to use human readable value like degrees while we're at it - https://codedocs.revrobotics.com/java/com/revrobotics/absoluteencoder#setPositionConversionFactor(double)

qwbmo commented 6 months ago

Side note: We should consider setting the SparkMax to use human readable value like degrees while we're at it -

even better!