-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: intake IO and commands #6
base: main
Are you sure you want to change the base?
Conversation
@@ -28,4 +28,23 @@ public enum Mode { | |||
/** Replaying from a log file. */ | |||
REPLAY | |||
} | |||
|
|||
public static class IntakeConstants { |
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.
This year, we want constants in their own files by subsystem to reduce merge conflicts
i.e. robot2025/subsystems/intake/IntakeConstants.java
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.
This year we want to use factory methods as much as possible, rather than separate command classes
Something like
public Command commandSetAngle(Rotation2d angle) {
return Commands.runOnce(...)
}
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.
Ditto
import org.littletonrobotics.junction.Logger; | ||
import org.team1540.robot2025.Constants; | ||
|
||
public class Intake extends SubsystemBase { |
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.
I would call this whole subsystem the coral intake
public void setSpeed(double speed) { | ||
io.setSpeed(speed); | ||
} | ||
|
||
public void setPosition(Rotation2d rotations) { | ||
io.setPosition(rotations); | ||
} |
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.
I believe the intake has three functions:
Pivot (falcon)
Spin (falcon)
Funnel (the vertical rollers; neo)
The method names should reflect that
private final TalonFX topFalcon = new TalonFX(Constants.IntakeConstants.TOP_FALCON_ID); | ||
|
||
// controls height of intake | ||
private final TalonFX bottomFalcon = new TalonFX(Constants.IntakeConstants.BOTTOM_FALCON_ID); |
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.
Rather than top and bottom, labeling these (and related fields) with what they do would be clearer.
// controls height of intake | ||
private final TalonFX bottomFalcon = new TalonFX(Constants.IntakeConstants.BOTTOM_FALCON_ID); | ||
|
||
private final PositionVoltage motorRequest = new PositionVoltage(0).withSlot(0); |
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.
Might want to be MotionMagicVoltage
// topFalcon.setInverted(false); | ||
// bottomFalcon.setInverted(true); | ||
// neo.setInverted(false); | ||
|
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.
You will want current limits, and you will want to configure the second motor (and maybe the neo)
public void setSpeed(double speed) { | ||
topFalcon.set(speed); | ||
neo.set(speed); | ||
// told by Ryan that they will always be in sync, will separate if needed |
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.
This would make sense to aggregate at the subsystem level, not the IO level.
No description provided.