Skip to content
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

Intake and indexer #19

Merged
merged 15 commits into from
Feb 3, 2024
Merged

Intake and indexer #19

merged 15 commits into from
Feb 3, 2024

Conversation

AdleyJ
Copy link
Member

@AdleyJ AdleyJ commented Feb 1, 2024

No description provided.

@AdleyJ AdleyJ requested a review from a team as a code owner February 1, 2024 23:28
@AdleyJ AdleyJ linked an issue Feb 1, 2024 that may be closed by this pull request
@rutmanz rutmanz changed the base branch from main to staging February 1, 2024 23:30
public Command feedToAmp() {
return Commands.sequence(
Commands.runOnce(() -> indexerIO.setFeederVelocity(-600), this),
Commands.waitSeconds(5),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could wait for X time after the note stops being seen (waitUntil), just to make sure it gets out before stopping

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sequences though I might make them their own classes

return Commands.sequence(
Commands.runOnce(() -> indexerIO.setFeederVelocity(1200), this),
Commands.waitSeconds(1),
Commands.runOnce(() -> indexerIO.setFeederVelocity(0), this)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might make this not just end after a second. This will only be used in other sequences anyway so maybe just having it return the command to set feeder velocity to 1200 would be good

public class IndexerIOSim implements IndexerIO {


private final DCMotorSim intakeSim = new DCMotorSim(DCMotor.getNEO(1), INTAKE_GEAR_RATIO,0.025);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is 0.025 doing here? (it should probably be in constants file)

public class IndexerIOSparkMax implements IndexerIO {
private final CANSparkMax intakeMotor = new CANSparkMax(INTAKE_ID, CANSparkLowLevel.MotorType.kBrushless);
private final CANSparkMax feederMotor = new CANSparkMax(FEEDER_ID, CANSparkLowLevel.MotorType.kBrushless);
private final DigitalInput indexerBeamBreak = new DigitalInput(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constants file for digitalinput id

Comment on lines 16 to 17
public double setpointRPM;
public double feederPositionError;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setpoint logging should happen in the subsystem, not the IO inputs. Also, why are you logging the position error? Isn't the feeder only velocity controlled?

public class Indexer extends SubsystemBase {
private final IndexerIO indexerIO;
private final IndexerIOInputsAutoLogged indexerInputs = new IndexerIOInputsAutoLogged();
private final boolean TUNING = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a constant called tuningMode in the constants file, use that instead of this


default void setFeederVelocity(double velocity) {}

default void configurePID(double p, double i, double d) {}
Copy link
Member

@mimizh2418 mimizh2418 Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be called configureFeederPID since the intake isn't running a control loop

private boolean isClosedLoop = true;
private double intakeVoltage = 0.0;
private double feederVoltage = 0.0;
private double feederVelocityRPS = 0.0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably called feederSetpointRPS, feederVelocityRPS implies that this is the actual feeder velocity, which this is not.

@rutmanz rutmanz merged commit b2b1c7e into staging Feb 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Indexer / Intake
4 participants