FIRST-Tech-Challenge / SkyStone

FTC SDK
https://www.firstinspires.org/robotics/ftc/what-is-first-tech-challenge
275 stars 1.04k forks source link

ticksPerRev, gearing, maxRPM incorrect when using goBilda 435 RPM motors #178

Open shishghate opened 4 years ago

shishghate commented 4 years ago

We have been playing around with RoadRunner and noticed that the motor configuration information for the goBilda motors is the following:

@MotorType(ticksPerRev=2786, gearing=99.5, maxRPM=60, orientation=Rotation.CCW)
@DeviceProperties(xmlTag="goBILDA5202SeriesMotor", name="GoBILDA 5202 series", builtIn = true)
@DistributorInfo(distributor="goBILDA_distributor", model="goBILDA-5202", url="https://www.gobilda.com/5202-series-yellow-jacket-planetary-gear-motors/")
@ExpansionHubPIDFVelocityParams(P=2.0, I=0.5, F=11.1)
@ExpansionHubPIDFPositionParams(P=5.0)
public interface GoBILDA5202Series
    {
    }

Those values are for a 60RPM motor, but we are using the 435's that came with the strafer kit. That motor documentation says the ticksPerRev are 383.6, gearing is 13.7, and maxRPM is 435. Is there logic someplace in the code base that does this conversion? If not, just like there are different Neverrest motors listed in the motor list, should there be more goBilda definitions? Here is what we came up with for the 435, but we have no idea if the PID values are correct:

@MotorType(ticksPerRev=383.6, gearing=13.7, maxRPM=435, orientation= Rotation.CCW)
@DeviceProperties(xmlTag="goBILDA5202SeriesMotor435", name="GoBILDA 5202 series435", builtIn = true)
@DistributorInfo(distributor="goBILDA_distributor", model="goBILDA-5202", url="https://www.gobilda.com/5202-series-yellow-jacket-planetary-gear-motors/")
@ExpansionHubPIDFVelocityParams(P=2.0, I=0.5, F=11.1)
@ExpansionHubPIDFPositionParams(P=5.0)
public interface GoBILDA5202Series435 {
}

Would this also mess up the RUN_TO_POSITION calculations? Any clarification would be appreciated.

shishghate commented 4 years ago

I just opened the Neverrest 40 and 20 definitions and the values for the MotorType are different for each definition. Does this mean there should be different definitions for each goBilda motor?

@MotorType(ticksPerRev=1120, gearing=40, maxRPM=160, orientation=Rotation.CCW)
@MotorType(ticksPerRev=560, gearing=20, maxRPM=315, orientation=Rotation.CW)

Would this also affect RUN_USING_ENCODERS and RUN_TO_POSITION?

gearsincorg commented 4 years ago

The only time the SDK code uses the RPM and maxRPM values is to determine the Max Counts per second for a motor. (maxCPS = maxRPM * ticksPerRev / 60) So... since all of the goBilda Yellowjacket motors use the same motor/encoder housing, it does not matter which one is listed in the motor parameters, the macCPS per second are the same for ALL the different speeds. The one chosen to include was 60 RPM (or one rev per second).... Just to pick an easy standard.

The only reason that there are two different Neverest motors listed is that they use spur gearboxes that have opposite output shaft rotations, (notice the CW and CCW annotations). This allows the software to maintain the same rotation direction if the two types are interchanged.

But... Since the Yellow Jacket motors are planetary gearboxes, they all rotate in the same direction.

shishghate commented 4 years ago

Interesting. So one could not use the getTicksPerRev() on the goBilda motor definition to compute distance? We would have to hard code that value based on the motor we have chosen? Wouldn't it be better off just having a maxCPS parameter?

gearsincorg commented 4 years ago

That's true... maxCPS is probably a good suggestion.

Originally there were very few motor types to deal with. The mass proliferation of gear ratios has made the current Motor Configuration method unwieldy.

shishghate commented 4 years ago

I agree is has become unwieldy, but there is code out there now that depends on those values being accurate. What would be the best way to handle this?

TimVargo commented 4 years ago

Create a variable called Legacy, and default this to TRUE. If Legacy is true then use current values and methods. If it is false then use new (corrected) values and methods. Existing code will continue to use legacy stuff, new code can explicitly set Legacy to be FALSE.