Currently the ExpansionHubPidConstants class contains three feedback constants (kP, kI, kD), three feedforward constants (kS, kV, kA), and a continuous input status, minimum, and maximum. The ExpansionHubMotor class contains two instances of the pid constants class, one for position, and one for velocity, that are each separately modifiable. These values are published over NT to the topic /rhsp/{expansion hub usb}/motor{port}/pid/{type}/{constant} (where {type} is either position or velocity).
However, using velocity feedforward for position is mathematically incorrect, and continuous input makes no sense for velocity feedback.
Thus I think there should be two separate constants classes:
ExpansionHubPositionConstants that contains kP, kI, kD, kS, continuous, continuousMin, and continuousMax constants andExpansionHubVelocityConstants that contains kP, kI, kD, kS, kV, kA constants
This will also require a change to the expansion hub service, as the different constants classes will be publishing different things over NT now.
Currently the
ExpansionHubPidConstantsclass contains three feedback constants (kP, kI, kD), three feedforward constants (kS, kV, kA), and a continuous input status, minimum, and maximum. TheExpansionHubMotorclass contains two instances of the pid constants class, one for position, and one for velocity, that are each separately modifiable. These values are published over NT to the topic/rhsp/{expansion hub usb}/motor{port}/pid/{type}/{constant}(where{type}is either position or velocity).However, using velocity feedforward for position is mathematically incorrect, and continuous input makes no sense for velocity feedback.
Thus I think there should be two separate constants classes:
ExpansionHubPositionConstantsthat contains kP, kI, kD, kS, continuous, continuousMin, and continuousMax constants andExpansionHubVelocityConstantsthat contains kP, kI, kD, kS, kV, kA constantsThis will also require a change to the expansion hub service, as the different constants classes will be publishing different things over NT now.