Changes from previous PR #8
Conversation
- `DTSendable` forces subclasses to implement `AutoCloseable` - `DTSendable` convenience methods to limit NetworkTables entry refresh rate - Remove `CURRENTLIMITTYPE` generic parameter from `DTMotor` and change to a single `int` parameter - Add type-specific `configCurrentLimit()` methods to `DTTalonFX` and `DTNeo` - Rename `DTMotor::internal()` to 'getMotorImpl()` - Rename 'DTMotorFaults::internal()` to 'getFaultsImpl()` - Add `DTMotor::getMaxVelocity()` and `getStallTorque()` methods to interface - Bugfixes
| @@ -1,7 +1,7 @@ | |||
| package org.victorrobotics.frc.dtlib.actuator.motor; | |||
|
|
|||
| public interface DTMotorFaults<FAULT_TYPE> { | |||
| FAULT_TYPE internal(); | |||
| FAULT_TYPE getFaultsImpl(); | |||
There was a problem hiding this comment.
Not sure this is needed since the motor fault objects both provide the same methods.
There was a problem hiding this comment.
Maybe, but in some cases it might be nice to get something more specific. I suppose you probably could do DTMotor.getMotorImpl().[faults-method] though.
There was a problem hiding this comment.
Sorry, I wasn't clear. I meant that the parameter wasn't needed because both concrete fault classes have the same methods. Having the internal fault object doesn't buy you anything.
| new SupplyCurrentLimitConfiguration(true, maxSupplyCurrent, maxSupplyCurrent, 0)); | ||
| } | ||
|
|
||
| public void configCurrentLimit(SupplyCurrentLimitConfiguration currentConfig) { |
There was a problem hiding this comment.
This method could take the limits directly and construct the SupplyCurrentLimitConfiguration object.
| @@ -197,7 +206,7 @@ public double getEncoderPosition() { | |||
|
|
|||
| @Override | |||
| public double getEncoderVelocity() { | |||
| return internal.getSelectedSensorVelocity() / TICKS_PER_REV * 10; | |||
| return internal.getSelectedSensorVelocity() / TICKS_PER_REV * 10 * SECONDS_PER_MINUTE; | |||
There was a problem hiding this comment.
This returns the velocity in RPM, which is an important capability. Consider renaming this to something like getMotorRPM or getRPM.
There was a problem hiding this comment.
How about getEncoderVelocityRPM or getVelocityRPM? I don’t think getRPM is specific enough (could just be commanded velocity), and the word “motor” shouldn’t appear in a DTMotor method.
DTSendableforces subclasses to implementAutoCloseableDTSendableconvenience methods to limit NetworkTables entry refresh rateCURRENTLIMITTYPEgeneric parameter fromDTMotorand change to a singleintparameterconfigCurrentLimit()methods toDTTalonFXandDTNeoDTMotor::internal()to 'getMotorImpl()`to 'getFaultsImpl()DTMotor::getMaxVelocity()andgetStallTorque()methods to interface