Skip to content

Shooter Subsystem#42

Open
Aadhya-Sure wants to merge 28 commits intodevelopfrom
shooter-develop-2
Open

Shooter Subsystem#42
Aadhya-Sure wants to merge 28 commits intodevelopfrom
shooter-develop-2

Conversation

@Aadhya-Sure
Copy link

Removed state machine to simplify code flow since theres only really two modes and two of the states can be combined into a singular method
Initialized and configured 4 motors (tested only 3 because we don't have agitator hopper motor)
Added CAN ID constants for motors
Created methods to control shooter motors based on current RPM and linked them to commands (startShooter, stopShooter)
Binded commands to the A button on the operator controller

Tested both shooter motors and feeder motor on prototype. Still need to PID tune.

aquaseals and others added 24 commits February 7, 2026 15:47
…r variable,s will replace wiht acutal values later
@Aadhya-Sure Aadhya-Sure changed the title Shooter develop 2 Shooter Subsytem Feb 21, 2026
@Aadhya-Sure Aadhya-Sure changed the title Shooter Subsytem Shooter Subsystem Feb 21, 2026
Copy link
Contributor

@GabbyGenereux GabbyGenereux left a comment

Choose a reason for hiding this comment

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

Please remove all the files under /.gradle from this PR, as they shouldn't be committed.

@@ -0,0 +1,6 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

this shouldn't be committed either. We need to keep the projectYear and currentLanguage set, as it is now in the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: All java classes should be "PascalCase" (aka. first letter of every word should be capitalized) and methods should be camelCase (aka. first letter of every word, except for the first one, should be capitalized)

import java.util.concurrent.TimeUnit;


/** An example command that uses an example subsystem. */
Copy link
Contributor

@GabbyGenereux GabbyGenereux Feb 28, 2026

Choose a reason for hiding this comment

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

lets update the comments with something more applicable to the current command/class

// Returns true when the command should end.
@Override
public boolean isFinished() {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this should return true when the command should end, and we just return false, does that mean the command never ends?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comments apply here, rename the class and redo the comments so they are more descriptive of the code in the file.

// Returns true when the command should end.
@Override
public boolean isFinished() {
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Opposite question than the one I posed in startShooter, if we return true when the command should end, and we just return true right away, does the command every actually run?

Copy link

Choose a reason for hiding this comment

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

It looks to me that they have separated the start and stop into separate commands. Starting the shooting never ends until stop shooter is called. What happens if both commands are run at the same time (it could happen if they end up being assigned to different buttons).

I think they could probably be combined into a single command.

Choose a reason for hiding this comment

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

the button bindings for both commands are set such that they are both binded to one button. when you press/hold the button, startShooter runs and when you let go of the button, stopShooter is called. so basically stopShooter is continuously called until the button is pressed.

// Determines which way the motor spins
shooterConfig.inverted(false);
shooterMotor2.configure(
shooterConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: we should indent these properly to help with readability.

followerConfig.follow(shooterMotor1);
shooterMotor2.configure(followerConfig, SparkBase.ResetMode.kResetSafeParameters, SparkBase.PersistMode.kPersistParameters);


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Lets remove this extra whitespace as well

}

public void testMotor() { // purely for testing
System.out.println(shooterMotor1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets remove this for now. You'll have to add it back locally when you want to test, but I'd rather than than have rouge print statements hanging out in the code.

}

/**

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good place to put a comment explaining what this method does, and what it returns.

//indexerMotor.set(getDesiredVoltage()/2);
}

//@Override
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be removed?

public Robot() {
// Instantiate our RobotContainer. This will perform all our button bindings, and put our
// autonomous chooser on the dashboard.
System.out.println("shooter system initialized");
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove print statements, if we want to log things we should use a separate logging system.

m_encoder = shooterMotor1.getEncoder();
SparkMaxConfig shooterConfig = new SparkMaxConfig();
// Determines which way the motor spins
shooterConfig.inverted(false);
Copy link

Choose a reason for hiding this comment

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

You could make this a static variable instead of having to hunt through the code for it. Depending on how the motor is wired, then may need to be true or false. Since this is configuration related, it would be good practise to define configuration constant data together. For example

private static boolean INVERTED = false;

SparkBase.ResetMode.kResetSafeParameters,
SparkBase.PersistMode.kPersistParameters
);
shooterMotor1.setCANTimeout(500);//Units in miliseconds
Copy link

Choose a reason for hiding this comment

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

This timeout value could also be defined as a constant value... The other option for constant values is that you could allow them to be set via network tables... That can be really helpful during testing because it means you can dynamically tweak your variables without have to recompile and redeploy all the time.

//@SuppressWarnings("resource")
shooterMotor2 = new SparkMax(Constants.shooterConstants.MOTOR2_ID, MotorType.kBrushless);
// Determines which way the motor spins
shooterConfig.inverted(false);
Copy link

Choose a reason for hiding this comment

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

Similar comment as before, use a separate constant so they can be tweaked in one place.

@aquaseals aquaseals changed the base branch from main to develop March 5, 2026 01:03
aquaseals and others added 3 commits March 4, 2026 20:37
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.

5 participants