pivotpivotpivot #4
Conversation
samperlmutter
left a comment
samperlmutter
left a comment
There was a problem hiding this comment.
i'm not sure why this is reformatting everything? it doesn't look like the formatter has changed. i can't approve this in case it causes a bunch more format errors later. can you try and build the code on your computer, then commit all of that?
gowri-k2010
requested review from
samperlmutter
and removed request for
samperlmutter
…ivotpivotpivot # Conflicts: # build.gradle # src/main/java/frc/robot/RobotContainer.java # src/main/java/frc/robot/subsystems/drive/Drive.java
samperlmutter
left a comment
samperlmutter
left a comment
There was a problem hiding this comment.
looks pretty good for the most part. the arm is the most complete. pivot missing some things. nothing is simulated either
| private TalonFXConfiguration motorConfig() { | ||
| return new TalonFXConfiguration() | ||
| .withSlot0( | ||
| new Slot0Configs() | ||
| .withGravityType(GravityTypeValue.Arm_Cosine) | ||
| .withKD(0.0) | ||
| .withKP(0.0) | ||
| .withKI(0.0) | ||
| .withKG(0.0) | ||
| .withKS(0.0) | ||
| .withKV(0.0)) | ||
| .withCurrentLimits( | ||
| new CurrentLimitsConfigs() | ||
| .withStatorCurrentLimit(0.0) | ||
| .withSupplyCurrentLimit(0.0) | ||
| .withStatorCurrentLimitEnable(true) | ||
| .withSupplyCurrentLimitEnable(true)); | ||
| } |
There was a problem hiding this comment.
this should be moved to a PivotConfig class
| PivotDown((0.0)), | ||
| PivotUp((0.0)), | ||
| position(0), | ||
| PivotStowed((0.25)); |
There was a problem hiding this comment.
not sure what the position enum is for. these also don't really all need Pivot at the beginning since they are on the PivotPos enum.
Consider the difference between referencing them:
PivotPos.PivotDown
vs
PivotPos.Down
| .withMagnetOffset(magnetOffset) | ||
| .withAbsoluteSensorDiscontinuityPoint(0.625)); | ||
|
|
||
| // public static final int ARM_KRAKEN_ID = 21; |
There was a problem hiding this comment.
looks like you can delete all this commented out code
Co-authored-by: Sam Perlmutter <sam.perlmutter227@gmail.com>
Co-authored-by: Sam Perlmutter <sam.perlmutter227@gmail.com>
…ivotpivotpivot
gowri-k2010
requested review from
samperlmutter
and removed request for
samperlmutter
…otpivot # Conflicts: # src/main/java/frc/robot/RobotContainer.java
taran-duba
left a comment
taran-duba
left a comment
There was a problem hiding this comment.
looks much better! still a few changes left to do
taran-duba
left a comment
taran-duba
left a comment
There was a problem hiding this comment.
almost there! i think you may have accidentally resolved one of my previous comments before actually making the change so please address that!
samperlmutter
left a comment
samperlmutter
left a comment
There was a problem hiding this comment.
looking pretty good. few clean up stuff left
| } | ||
|
|
||
| public Command setPositionCommand(PivotPos pivotPos) { | ||
| return runEnd(() -> setPosition(pivotPos), io::stop); |
There was a problem hiding this comment.
I would advise you to use a control request. You're not moving the motor!
| } | ||
|
|
||
| public Command applyPowerCommand(double power) { | ||
| return runEnd(() -> io.applyPower(power), io::stop); |
|
|
||
| public interface PivotIO { | ||
| @AutoLog | ||
| class PivotIOinput { |
| } | ||
|
|
||
| public Command setPositionCommand(PivotPos pivotPos) { | ||
| return run(() -> setPosition(pivotPos)).finallyDo(io::stop); |
There was a problem hiding this comment.
This can be startEnd since the control request needs to be sent only once. In fact, for MotionMagicTorqueCurrentFOC, once you pass in a position to go to, it will stay at that position, so you don't need to run io.stop() at the end. You could just use runOnce instead
| } | ||
|
|
||
| public Command applyPowerCommand(double power) { | ||
| return run(() -> io.applyPower(power)).finallyDo(io::stop); |
There was a problem hiding this comment.
Same thing. You only need to send a control request once. Either startEnd or runOnce will do.
|
|
||
| @Override | ||
| public void updateInputs(PivotIOinput inputs) { | ||
| inputs.pos = (double) pivotCan.getAbsolutePosition().getValueAsDouble(); |
|
|
||
| @Override | ||
| public void applyPower(double power) { | ||
| pivotMotor.setControl(new DutyCycleOut(power)); |
There was a problem hiding this comment.
Please create a single instance of DutyCycleOut and reuse that instance instead, as you did with MotionMagicTorqueCurrentFOC
6 participants
Pull Request Template
There may be some issues with this tbh, I had some trouble w my device, will make further changes for sure
Type of Change
Subsystem(s) Modified
Description of Changes
Additional Context
Checklist
Screenshots/Videos