-
Notifications
You must be signed in to change notification settings - Fork 0
Intake Subsystem #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Intake Subsystem #15
Conversation
|
Butches Comments: GeneralAdd support for the sysid() commands. There will be commands for quasistatic and for dynamic both forward and reverse The set pivot velocity capability will not be needed, but does not hurt anything. IntakeSubsystem.javaIn the method isIntakeDeployed() you compare the actual angle of the intake to the deployed angle. However, the Same basic comment for isIntakeStowed() IntakeIOHardware.javaThe can bus for the intake will likely not be the can bus for the drivebase (which is CompTunerConstants.kCANBus). The PID controllers generally have P, I, D, V, A, G, and S parameters. Many of these are zero, but lets put the code On the forward and reverse soft limits, I would have different constants from the deployedAngle and stowedAngle. The reason For the status signals, all of them are 50 Hz except the pivot angle which is 20 Hz. However the only two that we might need In the setRollerVoltage you take the Voltage value in the method argument (volts) and extract it as a double (volts.in(Volts)) |
blaze-developer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here
| //Current Limit Configurations | ||
| pivotConfigs.CurrentLimits.StatorCurrentLimit = IntakeConstants.currentLimit; | ||
| pivotConfigs.CurrentLimits.StatorCurrentLimitEnable = true; | ||
| pivotMotor.getConfigurator().apply(pivotConfigs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pivotMotor.getConfigurator().apply(pivotConfigs); |
When applying configs, it is recommended / has been useful in the past to use the tryUntilOk() method from PheonixUtil. This trys the configuration up to a specified number of times until it returns an OK status signal.
We should also only have to run this configuration once per motor after the configuration has been entirely created, this call should be removed, and the next one should be changed to tryUntilOk and kept.
| pivotConfigs.Slot0.kD= IntakeConstants.pivotKD; | ||
| pivotConfigs.Slot0.kV= IntakeConstants.pivotKV; | ||
| pivotConfigs.Slot0.kI= IntakeConstants.pivotKI; | ||
| pivotMotor.getConfigurator().apply(pivotConfigs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pivotMotor.getConfigurator().apply(pivotConfigs); |
Same as above
| pivotConfigs.SoftwareLimitSwitch.ForwardSoftLimitThreshold= IntakeConstants.deployedAngle.in(Degrees); | ||
| pivotConfigs.SoftwareLimitSwitch.ReverseSoftLimitEnable= true; | ||
| pivotConfigs.SoftwareLimitSwitch.ReverseSoftLimitThreshold= IntakeConstants.stowedAngle.in(Degrees); | ||
| pivotMotor.getConfigurator().apply(pivotConfigs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pivotMotor.getConfigurator().apply(pivotConfigs); |
Same as above, remove this redundant call.
| pivotConfigs.MotionMagic.MotionMagicCruiseVelocity= IntakeConstants.pivotCruiseVelocity.in(DegreesPerSecond); | ||
| pivotConfigs.MotionMagic.MotionMagicAcceleration= IntakeConstants.pivotCruiseAcceleration.in(DegreesPerSecondPerSecond); | ||
| pivotConfigs.MotionMagic.MotionMagicJerk= IntakeConstants.pivotMaxJerk; | ||
| pivotMotor.getConfigurator().apply(pivotConfigs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pivotMotor.getConfigurator().apply(pivotConfigs); | |
| tryUntilOk(5, () -> pivotMotor.getConfigurator().apply(pivotConfigs, 0.25)); |
Same as above, keep this as it is the last one and after pivotConfigs has been configured fully.
| //Current Limit Configurations | ||
| rollerConfigs.CurrentLimits.StatorCurrentLimit= IntakeConstants.currentLimit; | ||
| rollerConfigs.CurrentLimits.StatorCurrentLimitEnable= true; | ||
| rollerMotor.getConfigurator().apply(rollerConfigs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| rollerMotor.getConfigurator().apply(rollerConfigs); |
Same as above.
| rollerConfigs.Slot0.kD= IntakeConstants.rollerKD; | ||
| rollerConfigs.Slot0.kV= IntakeConstants.rollerKV; | ||
| rollerConfigs.Slot0.kI= IntakeConstants.rollerKI; | ||
| rollerMotor.getConfigurator().apply(rollerConfigs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| rollerMotor.getConfigurator().apply(rollerConfigs); | |
| tryUntilOk(5, () -> rollerMotor.getConfigurator().apply(rollerConfigs, 0.25)); |
Same as above, keep this one.
| rollerCurrentAmpsSignal = rollerMotor.getSupplyCurrent(); | ||
| pivotCurrentAmpsSignal = pivotMotor.getSupplyCurrent(); | ||
|
|
||
| BaseStatusSignal.setUpdateFrequencyForAll(50.0, rollerAngularVelocitySignal, rollerAppliedVoltsSignal, rollerCurrentAmpsSignal, pivotAngularVelocitySignal, pivotAppliedVoltsSignal, pivotCurrentAmpsSignal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can also use the tryUntilOk method.
| Logger.recordOutput("Intake/PivotAngle", inputs.PivotAngle); | ||
| Logger.recordOutput("Intake/RollerVoltage", inputs.RollerAppliedVolts); | ||
| Logger.recordOutput("Intake/PivotAngularVelocity", inputs.PivotAngularVelocity); | ||
| Logger.recordOutput("Intake/RollerAngularVelocity", inputs.RollerAngularVelocity); | ||
| Logger.recordOutput("Intake/RollerCurrentAmps", inputs.RollerCurrentAmps); | ||
| Logger.recordOutput("Intake/PivotCurrentAmps", inputs.PivotCurrentAmps); | ||
| Logger.recordOutput("Intake/PivotAppliedVolts", inputs.PivotAppliedVolts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Logger.recordOutput("Intake/PivotAngle", inputs.PivotAngle); | |
| Logger.recordOutput("Intake/RollerVoltage", inputs.RollerAppliedVolts); | |
| Logger.recordOutput("Intake/PivotAngularVelocity", inputs.PivotAngularVelocity); | |
| Logger.recordOutput("Intake/RollerAngularVelocity", inputs.RollerAngularVelocity); | |
| Logger.recordOutput("Intake/RollerCurrentAmps", inputs.RollerCurrentAmps); | |
| Logger.recordOutput("Intake/PivotCurrentAmps", inputs.PivotCurrentAmps); | |
| Logger.recordOutput("Intake/PivotAppliedVolts", inputs.PivotAppliedVolts); |
We don't really need to log these as outputs, they are already logged periodically as inputs, I think we should save on log file space and remove these calls.
| public void setRollerVoltage(Voltage volts) { | ||
| io.setRollerVoltage(volts); | ||
| } | ||
|
|
||
| public void setPivotAngle(Angle angle) { | ||
| io.setPivotAngle(angle); | ||
| } | ||
|
|
||
| public void stopRoller(){ | ||
| io.setRollerVoltage(Volts.of(0)); | ||
| } | ||
|
|
||
| public void stowIntake(){ | ||
| setPivotAngle(IntakeConstants.stowedAngle); | ||
| } | ||
|
|
||
| public void deployIntake(){ | ||
| setPivotAngle(IntakeConstants.deployedAngle); | ||
| } | ||
|
|
||
| public void setRollerVelocity(AngularVelocity velocity) { | ||
| io.setRollerVelocity(velocity); | ||
| } | ||
|
|
||
| public void setPivotVelocity(AngularVelocity velocity) { | ||
| io.setPivotVelocity(velocity); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should either be primarily exposed as Commands or command options should also be exposed. Either I can go in and do this, or you can if you feel up to it.
| import edu.wpi.first.units.measure.AngularVelocity; | ||
| import edu.wpi.first.units.measure.Angle; | ||
|
|
||
| public class IntakeCommands { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The majority of these, if not all of them, can be moved directly into the IntakeSubsystem, meaning its easier around the codebase to reference these commands, for example, instead of writing:
IntakeCommands.stowIntakeCommand(intake_)We could write
intake_.stow()When we start to involve multiple subsystems, factories like this become more useful.
| null, //Default ramp rate of the voltage of 1 V/s, according to Phoenix 6 documentation | ||
| stepVoltage_pivot, | ||
| timeOut_pivot, | ||
| (state) -> SignalLogger.writeString("state", state.toString()) //Logging the state of the routine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using the SignalLogger, AdvantageKit can be used here described in this documention..
We don't use the CTRE signal logger that saves log files, meaning we need to do it in our wpilog.
| null, //Default ramp rate of the voltage of 1 V/s, according to Phoenix 6 documentation | ||
| stepVoltage_roller, | ||
| timeOut_roller, | ||
| (state) -> SignalLogger.writeString("state", state.toString()) //Logging the state of the routine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
No description provided.