FIRST-Tech-Challenge / SkyStone

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

setVelocity() should be setTargetVelocity(); there should be a getTargetVelocity() #205

Open GLRoylance opened 4 years ago

GLRoylance commented 4 years ago

The position model is reasonable. DcMotor.getCurrentPosition() returns the current position as read by the encoder. One can set a desired position with DcMotor.setTargetPostion(int ticks); that causes the controller to seek the target position. There is no guarantee that the motor will ever reach that position. One can learn the target position by invoking DcMotor.getTargetPosition().

DcMotorEx offers PIDF position control using the same methods, but its PIDF velocity control does not follow the pattern.

DcMotorEx.getVelocity() returns the current velocity (angular rate) in ticks per second. That method is analogous to getCurrentPosition(). For parallelism, the method should be renamed to getCurrentVelocity().

/* rename */
int getCurrentVelocity() {
  ... existing code
}

/** @deprecated */
int getVelocity() {
  return getCurrentVelocity();
}

DcMotorEx.setVelocity(double angularRate) is not analogous. It does not set the actual velocity but rather sets a target velocity. To follow the pattern, the method should be renamed setTargetVelocity(double angularRate).

/* rename */
void setTargetVelocity(double angularRate () {
  ... existing code
}

/** @deprecated */
void setVelocity(double angularRate) {
  setTargetVelocity(angularRate);
}

DcMotorEx does not have a method to discover the target position. There should be a DcMotorEx.getTargetVelocity() method.

double getTargetVelocity() {
  return  ... ;
}