Jonathan #6

lotan23 · 2022-09-09T10:03:30Z

Create height, Joysticpower commands

This reverts commit 86ddd8e.

GaiaZano05

Graet job Jonathan!

GaiaZano05 · 2022-09-09T10:30:01Z

src/main/java/frc/robot/Constants.java

@@ -22,6 +22,7 @@ public static final class Gripper {
        public static final Boolean VOLT = true;
        public static final int SAT = 12;
        public static final double TICKS_PER_METER = 2048/(Math.PI*Math.PI*0.066/10);
+        public static final double OFFSET = 0.03;
    }

 }


Cntrl+alt+l

GaiaZano05 · 2022-09-09T10:30:30Z

src/main/java/frc/robot/Constants.java

@@ -22,6 +22,7 @@ public static final class Gripper {
        public static final Boolean VOLT = true;
        public static final int SAT = 12;
        public static final double TICKS_PER_METER = 2048/(Math.PI*Math.PI*0.066/10);


GaiaZano05 · 2022-09-09T10:30:55Z

src/main/java/frc/robot/Constants.java

@@ -22,6 +22,7 @@ public static final class Gripper {
        public static final Boolean VOLT = true;
        public static final int SAT = 12;
        public static final double TICKS_PER_METER = 2048/(Math.PI*Math.PI*0.066/10);
+        public static final double OFFSET = 0.03;


GaiaZano05 · 2022-09-09T10:37:15Z

src/main/java/frc/robot/subsystems/Elevator/Elevator.java

+                /*
+                 * @param value
+                 * @return
+                 */


Delete lines.

GaiaZano05 · 2022-09-09T10:37:41Z

src/main/java/frc/robot/subsystems/Elevator/Elevator.java

+                    motor.set(ControlMode.Position, unitModel.toTicks(pos));
+                }
+
+        }


Cntrl + alt + l

GaiaZano05 · 2022-09-09T10:38:40Z

src/main/java/frc/robot/subsystems/Elevator/Height.java

+
+    @Override
+    public void end(boolean interrupted) {
+        elevator.setPower(0);
+    }
+
+    @Override
+    public boolean isFinished() {
+        return Math.abs(elevator.getPosition() - height) <= Constants.Elevator.OFFSET;


Put isFinished() before end().

GaiaZano05 · 2022-09-09T10:39:09Z

src/main/java/frc/robot/subsystems/Elevator/JoystickPower.java

+        @Override
+        public void initialize () {
+        }
+


Delete lines

GaiaZano05 · 2022-09-09T10:39:33Z

src/main/java/frc/robot/subsystems/Elevator/JoystickPower.java

+            elevator.setPower(0);
+        }
+    }
+


Cntrl+alt+l

katzuv · 2022-09-10T17:37:33Z

src/main/java/frc/robot/subsystems/Elevator/Elevator.java

+        }
+
+        /**
+         <<<<<<< HEAD


This line should be deleted.

katzuv · 2022-09-10T17:55:23Z

src/main/java/frc/robot/subsystems/Elevator/Elevator.java

+
+public class Elevator extends SubsystemBase {
+    private final WPI_TalonFX motor = new WPI_TalonFX(Ports.Elevator.MOTOR);
+    private static Elevator INSTANCE = null;


The order, IMO, should be:

elevator instance

blank line

motor

unit model

katzuv

Great job! A few notes:

Command names should start with a verb.
There is some lack of documentation in the code, make sure to add it.
There is a bunch of code that was deleted in this branch, i.e. gripper code. Why is that?
There are some files here that are tracked which shouldn't be, i.e. files in the .idea directory. Read here to learn how to resolve this issue.

Good luck! I'm here for any questions 🙂

katzuv · 2022-09-10T17:56:09Z

src/main/java/frc/robot/subsystems/Elevator/Elevator.java

+        /**
+         * get power
+         * @return
+         */
+        public double getPower () {
+            return motor.get();
+        }


Is this method needed?

katzuv · 2022-09-10T17:57:25Z

src/main/java/frc/robot/subsystems/Elevator/Elevator.java

+                 * @param value
+                 * @return


Document value and and the return value.

katzuv · 2022-09-10T18:05:19Z

src/main/java/frc/robot/subsystems/Elevator/Elevator.java

+                 * @param value
+                 * @return
+                 */
+                public double getDeadband ( double value){


This method name is not clear. It sounds like it returns the deadband value, not the value after applying the deadbend on it.
But anyway, it seems to me you should use the Talon's deadbend functionality instead of implementing it by yourself.

katzuv · 2022-09-10T18:07:37Z

src/main/java/frc/robot/subsystems/Elevator/Height.java

+    private final double height;
+    private Elevator elevator;
+
+    public Height(double height, Elevator elevator) {


Command names should start with a verb.

katzuv · 2022-09-10T18:13:57Z

src/main/java/frc/robot/subsystems/Elevator/UnitModel.java

@@ -1,4 +1,4 @@
-package frc.robot.subsystems;
+package frc.robot.subsystems.Elevator;


UnitModel shouldn't live in the eleavotr's package, because it's relevant to other subsystems in the robot as well. Return it to where it was before 🙂.

katzuv · 2022-09-10T18:14:42Z

src/main/java/frc/robot/subsystems/Elevator/UnitModel.java

+    public double toVelocity(double ticks100ms) {
        return (ticks100ms / ticksPerUnit) * 10;
    }

-    public int toTicks100ms(double velocity){
+    public int toTicks100ms(double velocity) {


These changes should be made in this branch, as they are not relevant to the eleavtor. You should delete them and move them to a different branch.

katzuv · 2022-09-10T20:17:50Z

src/main/java/frc/robot/Constants.java

        public static final Boolean VOLT = true;
        public static final int SAT = 12;


These contants names aren't clear, please find more meaningful names.

lotan23 added 8 commits September 9, 2022 12:34

Revert "Merge pull request #5 from Galaxia5987/Jonathan"

e0674a7

This reverts commit 86ddd8e.

Delete files

81db2a8

Delete constants and ports

3df9815

.

b6ed23a

Rearrange files

10a6064

Fix merge conflicts

1b8164f

Add offset

b9e2ce8

Create Height command

ea7c5cf

lotan23 requested a review from GaiaZano05 September 9, 2022 10:05

GaiaZano05 reviewed Sep 9, 2022

View reviewed changes

lotan23 requested a review from katzuv September 10, 2022 14:23

katzuv reviewed Sep 10, 2022

View reviewed changes

src/main/java/frc/robot/subsystems/Elevator/Elevator.java

}

/**

<<<<<<< HEAD

Copy link

Member

katzuv Sep 10, 2022

Choose a reason for hiding this comment

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

This line should be deleted.

katzuv reviewed Sep 10, 2022

View reviewed changes

katzuv requested changes Sep 10, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jonathan #6

Jonathan #6

lotan23 commented Sep 9, 2022 •

edited

Loading

GaiaZano05 left a comment

GaiaZano05 Sep 9, 2022

GaiaZano05 Sep 9, 2022

GaiaZano05 Sep 9, 2022

GaiaZano05 Sep 9, 2022

GaiaZano05 Sep 9, 2022

GaiaZano05 Sep 9, 2022

GaiaZano05 Sep 9, 2022

GaiaZano05 Sep 9, 2022

katzuv Sep 10, 2022

katzuv Sep 10, 2022

katzuv left a comment •

edited

Loading

katzuv Sep 10, 2022

katzuv Sep 10, 2022

katzuv Sep 10, 2022

katzuv Sep 10, 2022

katzuv Sep 10, 2022

katzuv Sep 10, 2022

katzuv Sep 10, 2022

		@@ -1,4 +1,4 @@
		package frc.robot.subsystems;
		package frc.robot.subsystems.Elevator;

		public static final Boolean VOLT = true;
		public static final int SAT = 12;

Jonathan #6

Are you sure you want to change the base?

Jonathan #6

Conversation

lotan23 commented Sep 9, 2022 • edited Loading

GaiaZano05 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

katzuv left a comment • edited Loading

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

lotan23 commented Sep 9, 2022 •

edited

Loading

katzuv left a comment •

edited

Loading