Changes to Intake also Juicy 3 #4

rohtumm · 2022-01-17T18:03:50Z

Intake code to control the motor that the compliant wheels are on as well as the drawbridge motor.

FlyN-Nick · 2022-01-17T18:33:02Z

What does the extend intake command do?

FlyN-Nick · 2022-01-17T18:35:03Z

Lines 19 and 23 of Intake.java, I suggest using this.motorName for clarity

FlyN-Nick · 2022-01-17T18:37:54Z

Lines 10 and 11 of Intake, should be public final.

FlyN-Nick · 2022-01-17T18:41:03Z

Lines 37-39 of Intake.java, the comment is unnecessary.

rohtumm · 2022-01-17T18:47:55Z

What does the extend intake command do?

rohtumm · 2022-01-17T18:49:32Z

What does the extend intake command do?

It extends the drawbridge which has the intake mechanism attached to it.

FlyN-Nick · 2022-01-17T20:54:31Z

src/main/java/org/usfirst/frc4904/robot/subsystems/Intake.java

+    public final static double DEFAULT_DRAWBRIDGE_MOTOR_SPEED = 0.0; //needs value
+    public final static double DEFAULT_OFF_SPEED = 0.0;
+
+    public Motor drawbridgeMotor;


FlyN-Nick · 2022-01-17T20:54:39Z

src/main/java/org/usfirst/frc4904/robot/subsystems/Intake.java

+    public final static double DEFAULT_OFF_SPEED = 0.0;
+
+    public Motor drawbridgeMotor;
+    public Motor axelMotor;


FlyN-Nick · 2022-01-17T20:54:49Z

src/main/java/org/usfirst/frc4904/robot/subsystems/Intake.java

+    }
+
+    public void setAxelIntakeSpeed(double speed) {
+        axelMotor.set(speed);


add this.

FlyN-Nick · 2022-01-17T20:55:00Z

src/main/java/org/usfirst/frc4904/robot/subsystems/Intake.java

+    }
+
+    public void setDrawBridgeSpeed(double speed) {
+        drawbridgeMotor.set(speed);


add this.

FlyN-Nick · 2022-01-17T20:55:10Z

src/main/java/org/usfirst/frc4904/robot/subsystems/Intake.java

+    public void stop() {
+        setSpeed(DEFAULT_OFF_SPEED, DEFAULT_OFF_SPEED); //maybe retract drawbridge?
+    }
+    // public void retractDrawBridge(double speed) {


delete comment

FlyN-Nick · 2022-01-17T20:57:45Z

src/main/java/org/usfirst/frc4904/robot/subsystems/Intake.java

+
+import org.usfirst.frc4904.standard.subsystems.motor.Motor;
+
+public class Intake {


Shouldn't this extend SubsystemBase?

Yes—all subsystems should extend SubsystemBase.

FlyN-Nick · 2022-01-17T21:00:08Z

src/main/java/org/usfirst/frc4904/robot/commands/ExtendIntake.java

+    public ExtendIntake(Intake intake, double targetSpeed) {
+        super();
+        this.intake = intake;
+        this.targetSpeed = targetSpeed;


You declare a variable targetSpeed, but you don't actually do anything with it. You need to call intake's setSpeed.

FlyN-Nick · 2022-01-17T21:00:51Z

src/main/java/org/usfirst/frc4904/robot/commands/ExtendIntake.java

@@ -0,0 +1,16 @@
+package org.usfirst.frc4904.robot.commands;


Preferably put this command in a subfolder of commands named "Intake".

FlyN-Nick · 2022-01-17T21:03:30Z

src/main/java/org/usfirst/frc4904/robot/commands/ExtendIntake.java

+public class ExtendIntake extends CommandBase {
+    protected final Intake intake;
+    protected double targetSpeed;
+    public ExtendIntake(Intake intake, double targetSpeed) {


The intake should not be a parameter of the constructor. Instead, use this.intake = RobotMap.Component.intake;. Additionally, for this to work, add the line addRequirements(RobotMap.Component.intake); as the last line of the constructor. You can checkout to the turret branch and look at the command TurnTurret as an example.

b-cho

Left some comments—please address and I can take another look.

b-cho · 2022-01-22T01:06:01Z

build.gradle

@@ -2,7 +2,7 @@ import edu.wpi.first.gradlerio.deploy.roborio.RoboRIO

 plugins {
    id "java"
-    id "edu.wpi.first.GradleRIO" version "2022.1.1"
+    id "edu.wpi.first.GradleRIO" version "2022.2.1"


Make sure that this matches the version on the driver station—probably leave it as 2022.1.1 for now.

Don't worry about changing this at the moment, though.

b-cho · 2022-01-22T01:06:55Z

src/main/java/org/usfirst/frc4904/robot/RobotMap.java

@@ -21,6 +32,7 @@
        }

        public static class Pneumatics {
+            public static final PCMPort DRAWBRIDGE_INTAKE_SOLENOID = new PCMPort(-1, PneumaticsModuleType.CTREPCM, -1, -1); //TODO: set port for drawbridge intake motor


No longer a motor (in the comment) :)

src/main/java/org/usfirst/frc4904/robot/RobotMap.java

b-cho · 2022-01-22T01:08:25Z

src/main/java/org/usfirst/frc4904/robot/commands/intake/AxleIntakeOff.java

+
+public class AxleIntakeOff extends MotorIdle {
+    public AxleIntakeOff() {
+        super("Stop Axle Intake", RobotMap.Component.intakeAxleMotor);


Generally the name argument should be in camel caps without spaces.

b-cho · 2022-01-22T01:08:37Z

src/main/java/org/usfirst/frc4904/robot/commands/intake/AxleIntakeOn.java

+
+public class AxleIntakeOn extends MotorConstant {
+    public AxleIntakeOn() {
+        super("Axel Intake On", RobotMap.Component.intakeAxleMotor, 0.5); // todo: needs motor speed


Generally the name argument should be in camel caps without spaces (see other).

b-cho · 2022-01-22T01:08:52Z

src/main/java/org/usfirst/frc4904/robot/commands/intake/ExtendIntake.java

+
+public class ExtendIntake extends SolenoidExtend { //It extends the drawbridge which has the intake mechanism attached to it.
+    public ExtendIntake() {
+        super("Extend Intake Drawbridge", RobotMap.Component.intakeDrawbridgeSolenoid);


Name argument should be in camel caps without spaces.

b-cho · 2022-01-22T01:09:38Z

src/main/java/org/usfirst/frc4904/robot/commands/intake/ExtendIntake.java

+import org.usfirst.frc4904.robot.RobotMap;
+import org.usfirst.frc4904.standard.commands.solenoid.SolenoidExtend;
+
+public class ExtendIntake extends SolenoidExtend { //It extends the drawbridge which has the intake mechanism attached to it.


Comments probably should go above the first line of the class, like this

// It extends the drawbridge which has the intake mechanism attached to it. public class ExtendIntake extends SolenoidExtend { ...

b-cho · 2022-01-22T01:09:58Z

src/main/java/org/usfirst/frc4904/robot/commands/intake/RetractIntake.java

+
+public class RetractIntake extends SolenoidRetract { //It extends the drawbridge which has the intake mechanism attached to it.
+    public RetractIntake() {
+        super("Retract Intake Drawbridge", RobotMap.Component.intakeDrawbridgeSolenoid);


Same as before—the name argument should be in camel caps without spaces.

b-cho · 2022-01-22T01:10:21Z

src/main/java/org/usfirst/frc4904/robot/subsystems/Intake.java

+
+import org.usfirst.frc4904.standard.subsystems.motor.Motor;
+
+public class Intake {


Yes—all subsystems should extend SubsystemBase.

b-cho · 2022-01-23T22:15:41Z

src/main/java/org/usfirst/frc4904/robot/RobotMap.java

@@ -70,5 +83,7 @@ public RobotMap() {
        HumanInput.Driver.xbox = new CustomXbox(Port.HumanInput.xboxController);
 		HumanInput.Operator.joystick = new CustomJoystick(Port.HumanInput.joystick);

+        Component.intakeDrawbridgeSolenoid = new SolenoidSubsystem("Intake Drawbridge Solenoid", false, SolenoidState.RETRACT, Port.Pneumatics.DRAWBRIDGE_INTAKE_SOLENOID.buildDoubleSolenoid());
+        Component.intakeAxleMotor = new Motor("Intake Motor", false, new CANTalonFX(Port.CANMotor.AXLE_INTAKE_MOTOR)); //TODO: check if CANTalonFX or SRX


Also this is a CANTalonFX—confirmed with the intake people on design.

b-cho

Looks good for now—leaving an approval here ✅

zbuster05 · 2022-01-29T01:10:51Z

src/main/java/org/usfirst/frc4904/robot/commands/intake/AxleIntakeOn.java

+ * Rotates the axle to intake the ball
+ */
+public class AxleIntakeOn extends MotorConstant {
+    public final static double DEFAULT_INTAKE_MOTOR_SPEED = 0.5; //TODO: needs value


Remember to add value

zbuster05

Looks good for now. We'll make sure to adjust constants and then after design review we should be able to merge this.

Changes to Intake also Juicy 3

56a60da

rohtumm requested review from b-cho and zbuster05 January 17, 2022 18:03

zbuster05 requested a review from FlyN-Nick January 17, 2022 18:05

rohtumm closed this Jan 17, 2022

rohtumm reopened this Jan 17, 2022

Fixed todos and added newline.

e1a2fd3

FlyN-Nick requested changes Jan 17, 2022

View reviewed changes

b-cho linked an issue Jan 22, 2022 that may be closed by this pull request

Intake mechanism #2

Open

9 tasks

Changed intake subsystem for solenoid and added commands

4f8eab7

b-cho requested changes Jan 22, 2022

View reviewed changes

aze12345 and others added 2 commits January 21, 2022 17:30

changed variable names deleted intake subsystem

a92e5f6

Remove unnecessary TODO

093cd98

BenCantCode requested a review from b-cho January 23, 2022 17:45

b-cho assigned aze12345, LukaK73, rohtumm, TheShishKabob and howliu19 Jan 23, 2022

b-cho reviewed Jan 23, 2022

View reviewed changes

b-cho previously approved these changes Jan 23, 2022

View reviewed changes

FlyN-Nick previously approved these changes Jan 23, 2022

View reviewed changes

zbuster05 reviewed Jan 29, 2022

View reviewed changes

zbuster05 previously approved these changes Jan 29, 2022

View reviewed changes

added secondary intake motor

c2b48e0

pluvii dismissed stale reviews from FlyN-Nick, zbuster05, and b-cho via c2b48e0 February 3, 2022 01:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to Intake also Juicy 3 #4

Changes to Intake also Juicy 3 #4

rohtumm commented Jan 17, 2022

FlyN-Nick commented Jan 17, 2022

FlyN-Nick commented Jan 17, 2022 •

edited

Loading

FlyN-Nick commented Jan 17, 2022

FlyN-Nick commented Jan 17, 2022

rohtumm commented Jan 17, 2022

rohtumm commented Jan 17, 2022

FlyN-Nick Jan 17, 2022

FlyN-Nick Jan 17, 2022

FlyN-Nick Jan 17, 2022

FlyN-Nick Jan 17, 2022

FlyN-Nick Jan 17, 2022

FlyN-Nick Jan 17, 2022

b-cho Jan 22, 2022

FlyN-Nick Jan 17, 2022

FlyN-Nick Jan 17, 2022

FlyN-Nick Jan 17, 2022

b-cho left a comment

b-cho Jan 22, 2022

b-cho Jan 22, 2022

b-cho Jan 22, 2022

b-cho Jan 22, 2022

b-cho Jan 22, 2022

b-cho Jan 22, 2022

b-cho Jan 22, 2022

b-cho Jan 22, 2022

b-cho Jan 22, 2022

b-cho Jan 23, 2022

zbuster05 Jan 29, 2022

b-cho left a comment

zbuster05 Jan 29, 2022

zbuster05 left a comment


		import org.usfirst.frc4904.standard.subsystems.motor.Motor;

		public class Intake {

Changes to Intake also Juicy 3 #4

Are you sure you want to change the base?

Changes to Intake also Juicy 3 #4

Conversation

rohtumm commented Jan 17, 2022

FlyN-Nick commented Jan 17, 2022

FlyN-Nick commented Jan 17, 2022 • edited Loading

FlyN-Nick commented Jan 17, 2022

FlyN-Nick commented Jan 17, 2022

rohtumm commented Jan 17, 2022

rohtumm commented Jan 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-cho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-cho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zbuster05 left a comment

Choose a reason for hiding this comment

FlyN-Nick commented Jan 17, 2022 •

edited

Loading