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

Shooter commands #8

Open
wants to merge 102 commits into
base: testing
Choose a base branch
from
Open

Shooter commands #8

wants to merge 102 commits into from

Conversation

Exr0n
Copy link

@Exr0n Exr0n commented Jan 31, 2020

Everything has been checked and should be all good. Still need to do robot map stuff.

Exr0n and others added 30 commits January 20, 2020 12:18
renamed setMotorSpeed to setSpeed
created defaultOnSpeed and defaultOffSpeed
Replaced flywheel with original version
Wraps the lift belts and flippers only. Does not include runup belt.
No longer extends SubsystemBase
Changed some member names for clarity
make subsystem data members public
intake now wraps the intake rollers, the funnel motor, and the intake extension solenoid
added different default speed constants
added various speed setters
made subsystem data members public so commands can access them
add IDLE status to the enum
delete boxshot constant
update syncstatus for idle
Copy link
Member

@Daniel-E-B Daniel-E-B left a comment

Choose a reason for hiding this comment

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

please read through requested changes

}

public static class Component {
public static class Digital {
public static final int INDEXER_LIMIT_SWITCH = -1;
Copy link
Member

Choose a reason for hiding this comment

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

Where are the ports for hoodLowLimitSwitch and hoodHighLimitSwitch

public static class Flywheel {
public static final double P = 0;
public static final double I = 0;
public static final double D = 0;
Copy link
Member

Choose a reason for hiding this comment

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

wheres F

public static class Hood {
public static final double P = 0;
public static final double I = 0;
public static final double D = 0;
Copy link
Member

Choose a reason for hiding this comment

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

add F

Component.funnelMotor = new Motor("funnelMotor", new CANTalonSRX(Port.CANMotor.INTAKE_FUNNEL_MOTOR));
Component.liftBeltMotor = new Motor("liftBeltMotor", new CANTalonSRX(Port.CANMotor.LIFT_BELT_MOTOR));
Component.runUpBeltMotor = new Motor("runUpBeltMotor", new CANTalonSRX(Port.CANMotor.RUN_UP_BELT_MOTOR));
CANTalonFX flywheelATalon = new CANTalonFX(Port.CANMotor.FLYWHEEL_MOTOR_A); //TODO: Nicer way to do this?
Copy link
Member

Choose a reason for hiding this comment

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

you could make the Talons separately if you want as Components and pass them in but this is probably fine

Copy link
Member

Choose a reason for hiding this comment

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

I'm going to leave it as-is for now, but question: do we want to average the reading of the two encoders, or just take one?

Component.flipperSolenoid = new SolenoidSubsystem(Port.Pneumatics.FLIPPER_SOLENOID.buildDoubleSolenoid());
Component.shooterAimSolenoid = new SolenoidSubsystem(Port.Pneumatics.SHOOTER_AIM_SOLENOID.buildDoubleSolenoid());

// TODO: FIX MOTOR TYPES
Copy link
Member

Choose a reason for hiding this comment

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

does this still need to be done? If so let's do it because its all built

import org.usfirst.frc4904.robot.subsystems.Hood;

import edu.wpi.first.wpilibj2.command.CommandBase;
public class SetHoodAngle extends CommandBase {
Copy link
Member

Choose a reason for hiding this comment

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

should it be some sort of motor position constant?

Copy link
Member

Choose a reason for hiding this comment

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

yes, absolutely

@Override
public void initialize() {
hood.enableMotionController();
hood.setPosition(angle);
Copy link
Member

Choose a reason for hiding this comment

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

does this need to be done in execute?

public final double DEFAULT_SPEED = 0;
private final double LOWER_HOOD_ANGLE = 0; //TODO: Add this value
private final double RANGE_HOOD_ANGLES = 35.0;
private final double UPPER_HOOD_ANGLE = LOWER_HOOD_ANGLE + RANGE_HOOD_ANGLES; //TODO: Does having all of these theoretical constants negate the zeroing we're doing?
Copy link
Member

Choose a reason for hiding this comment

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

these constants do not negate zeroing. The zeroing is because although the encoder on the servo is absolute, "0" will be a different place every time

Copy link
Member

Choose a reason for hiding this comment

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

I know, I more was wondering if perhaps the angle difference between the lower and upper part of the hood could change over time (if teeth get damaged or something else). For now, we should leave it as is, however, I'd be curious to see what the actual hood rotation amount is.

*/
@Override
public void setPosition(double angle) {
double safePosition = servoRange.limitValue(hoodAngleToServoPosition(angle)); //TODO: Should we error if we are outside of this value?
Copy link
Member

Choose a reason for hiding this comment

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

Either re-zero the hood and send something to the dashboard or disable the hood entirely

Copy link
Member

Choose a reason for hiding this comment

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

re-zeroing could potentially be extremely dangerous if we're outside of the range

Copy link
Member

Choose a reason for hiding this comment

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

This is only ever called in reference to a setpoint, so I don't think it's a zeroing error. It just means that someone tried to set our hood angle to something impossible.

flywheel.setSpeed(speed);
}

// public void setFlywheelSpeedForDistance(double distance) {
Copy link
Member

Choose a reason for hiding this comment

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

why is this commented out? We will eventually need a method for this

Copy link
Member

@billwpierce billwpierce left a comment

Choose a reason for hiding this comment

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

Just some general comments now that we're getting closer to testing. Nobody specific has to deal with them, I just wanted to get them down somewhere before starting to modify things.

public static class CAN {
public static class CAN { // TODO: CHANGE CONSTS
public static final int HOOD_MOTOR = -1;
public static final int HOOD_ENCODER = -1;
Copy link
Member

Choose a reason for hiding this comment

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

No hood encoder—we'll use the encoder that's built into the falcon that's driving the hood.

public static class Pneumatics {
public static class Pneumatics { // TODO: CHANGE CONSTS
public static final PCMPort INTAKE_SOLENOID = new PCMPort(0, -1, -1);
public static final PCMPort FLIPPER_SOLENOID = new PCMPort(0, -1, -1);
Copy link
Member

Choose a reason for hiding this comment

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

We probably aren't using a flipper.

Comment on lines 57 to 58
public static final int HOOD_LOWER_LIMIT_SWITCH = -1;
public static final int HOOD_UPPER_LIMIT_SWITCH = -1;
Copy link
Member

Choose a reason for hiding this comment

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

It's currently one limit switch, with two different magnets.

}
}

public static class Hood {
Copy link
Member

Choose a reason for hiding this comment

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

Where are the other constants, for converting from motor spins to hood movement?

Comment on lines 117 to 120
public static final double P = 0.0;
public static final double I = 0.0;
public static final double D = 0.0;
public static final double F = 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.

should probably copy over the values that work


import org.usfirst.frc4904.standard.commands.motor.MotorConstant;

public class IndexOne extends SequentialCommandGroup {
Copy link
Member

Choose a reason for hiding this comment

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

IMO IndexOne is a misleading name, because we don't have that precise of control over our mechanisms.

import org.usfirst.frc4904.robot.subsystems.Hood;

import edu.wpi.first.wpilibj2.command.CommandBase;
public class SetHoodAngle extends CommandBase {
Copy link
Member

Choose a reason for hiding this comment

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

yes, absolutely

* @param shooter The shooter subsystem
* @param speed The target speed of the flywheel
*/
public Shoot(double speed) {
Copy link
Member

Choose a reason for hiding this comment

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

should be modified to also take in a hood angle

* towards the upper limit, - speed is towards the lower limit. The LimitType
* "true" is upper limit, "false" is lower limit.
*/
public Hood(CANTalonEncoder hoodEncoder, CustomDigitalLimitSwitch limitSwitch) {
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this have a falcon passed in, instead of a hood encoder?

return Math.abs(RobotMap.Component.shooter.flywheel.getTargetSpeed() - speed) > SPEED_TOLERANCE;
}), new WaitUntil(() -> {
return RobotMap.Component.shooter.flywheel.getStatus() == FlywheelStatus.AT_SPEED;
}), new IndexOne());
Copy link
Member

Choose a reason for hiding this comment

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

realistically, we won't be shooting—instead, we'll get it all into position, and then the operator will hit the buttons to index the balls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.