REVrobotics / SPARK-MAX-Examples

Example code for SPARK MAX
BSD 3-Clause "New" or "Revised" License
104 stars 104 forks source link

RevPhysicsSim Position Control doesn't work #25

Open jonathandao0 opened 2 years ago

jonathandao0 commented 2 years ago

Given this code example:

CANSparkMax m_motorA;
CANSparkMax m_motorB;

public void robotInit() {
    m_motorA =  new CANSparkMax(0, CANSparkMaxLowLevel.MotorType.kBrushless);
    m_motorB =  new CANSparkMax(0, CANSparkMaxLowLevel.MotorType.kBrushless);

    REVPhysicsSim.getInstance().addSparkMax(m_motorA, DCMotor.getNEO(1));
    REVPhysicsSim.getInstance().addSparkMax(m_motorB, DCMotor.getNEO(1));
}

doube positionSetpoint = 0;
public void robotPeriodic() {
    double velocitySetpoint = 5000;

    m_motorA.getPIDController().setReference(
          velocitySetpoint ,
          CANSparkMax.ControlType.kVelocity);

    m_motorB.getPIDController().setReference(
          positionSetpoint ,
          CANSparkMax.ControlType.kPosition);

    REVPhysicsSim.getInstance().run();
    positionSetpoint += 1;
}

When simulating this code in WPILib, m_motorA is correctly updated in simulation while m_motorB does not get updated. Digging into the SparkMaxSimProfile.class code, it does the following when the simulation runs:

    public void run() {
        double period = this.getPeriod();
        this._vel = this._spark.getEncoder().getVelocity();
        double position = this._spark.getEncoder().getPosition();
        double posFactor = this._spark.getEncoder().getPositionConversionFactor();
        this._spark.getEncoder().setPosition(position + this._vel * period / 60000.0D * posFactor);
    }

It doesn't look like setting a position setpoint is correctly updating the SparkMax in simulation. Will this be fixed in the 2023 RevLib? Another thing that would be helpful would be an API call in SparkMaxPIDController to get the calcuated PID output so that we can take the output and simulate the results manually.

viggy96 commented 11 months ago

I don't believe the math that's used here to set the encoder position is correct at all. Where is the "60000.0" coming from? You should just be doing this._spark.getEncoder().setPosition((position + this._vel * period) * posFactor).

viggy96 commented 11 months ago

Also there is no way to properly simulate mechanisms that use an absolute encoder, like a MAXSwerve Module.