Skip to content
This repository has been archived by the owner on Oct 27, 2020. It is now read-only.

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

Open
GLRoylance opened this issue Jan 20, 2020 · 0 comments

Comments

@GLRoylance
Copy link

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  ... ;
}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant