Safe mode by ProfessorAtomicManiac · Pull Request #32 · DeepBlueRobotics/RobotCode2023

ProfessorAtomicManiac · 2023-04-28T02:43:10Z

Don't merge into master until after everything is tested out

ProfessorAtomicManiac

Technically this code has already been tested and it works ok, but I still see a lot of problems with it that I would like to see resolved ideally before Save the Music. I can't "Request Changes" on my own branch but I do not approve this PR. @FriedLongJohns (work on arm related issues) or @BrownBear85 (work on drivetrain related issues) please fix this when you have time.

src/main/java/org/carlmontrobotics/robotcode2023/Constants.java

src/main/java/org/carlmontrobotics/robotcode2023/RobotContainer.java

src/main/java/org/carlmontrobotics/robotcode2023/subsystems/Arm.java

src/main/java/org/carlmontrobotics/robotcode2023/Robot.java

Fix everything charles whined about except using multipliers instead of limits

src/main/java/org/carlmontrobotics/robotcode2023/Constants.java

src/main/java/org/carlmontrobotics/robotcode2023/commands/TeleopDrive.java

src/main/java/org/carlmontrobotics/robotcode2023/subsystems/Arm.java

was an unused controller rumble variable

wasn't fully implemented before +allow ArmTeleop.getRequestedSpeeds() to use baby max vel +fix Arm.arm/wristConstraints not changing the max ff accel even though we had a baby max accel

FriedLongJohns · 2023-09-30T16:49:03Z

Tasks

(in chronological order, not importance order!)

Copy arm encoder shutoff code to wrist
Reorganize wrist enum RollerMode
Make sure Roller is unaffected by slow/baby mode
Review drivetrain baby/slowmode code

^ Above tasks can be done in any order

- Review that all baby/slow/normal speeds are applied correctly
- Merge baby/slow/normal mode into an enum
- Charles moment
- Test code AT WORKSESSION
- Remove debug statements, merge into master

Also, a review of how baby/safemode affects things (Correct me if I'm wrong, @ProfessorAtomicManiac ):

  Drivetrain
    slows turning max accel + max vel
    slows movement max accel + max vel
  Arm/Wrist
     slows all feedForward motor accel + max vel
     turns off manual joystick control
   Roller
     no effect

ProfessorAtomicManiac · 2023-09-30T17:00:26Z

Yep that's correct!
Also there is a "slow mode" which is completely different from "baby mode". "Slow Mode" does:

Drivetrain
  slows turning max accel + max vel (not as much as baby mode i think)
  slows movement max accel + max vel (not as much as baby mode)
Arm/Wrist
   no effect
Roller
   no effect

This reverts commit cf37551.

was changeable but in constants, now moved to Roller subsystem =Also remove get/set of RollerMode.speed in favor of just making it public (if you both get and set a variable publicly without doing anything special, don't make it private)

…Code2023 into safe-mode

now only check one enum instead of checking both slow and baby (baby takes priority, of course)

ProfessorAtomicManiac

From the looks of it, it mostly works. The code is still rough in some places so please try to clean those up before Sunday's worksession.

ProfessorAtomicManiac · 2023-10-01T04:04:31Z

src/main/java/org/carlmontrobotics/robotcode2023/Constants.java

-        // TODO: Determine actual speeds/timings for roller
-        public static class RollerMode {
-            public static RollerMode INTAKE_CONE = new RollerMode(-0.5, .5, GameObject.CONE, conePickupColor);
-            public static RollerMode INTAKE_CUBE = new RollerMode(0.4, .25, GameObject.CUBE, cubePickupColor);
-            // The obj indicates which game object the roller is trying to intake
-            // if obj == NONE, that means it is trying to outtake rather than intake
-            public static RollerMode OUTTAKE_CONE = new RollerMode(0.5, .5, GameObject.NONE, defaultColor);
-            public static RollerMode OUTTAKE_CUBE = new RollerMode(-0.5, .5, GameObject.NONE, defaultColor);
-            public static RollerMode STOP = new RollerMode(0, .1, GameObject.NONE, defaultColor);
-            public double speed, time;
-            public GameObject obj;
-            public Color ledColor;
-
-            /**
-             * @param speed  A number between -1 and 1
-             * @param time   Amount of time in seconds to keep the motor running after
-             *               distance sensor has detected an object
-             * @param intake Whether the roller is outtaking or intaking
-             */
-            public RollerMode(double speed, double time, GameObject obj, Color ledColor) {
-                this.speed = speed;
-                this.time = time;
-                this.obj = obj;
-                this.ledColor = ledColor;
-            }
-        }


Originally Ryan wanted all constants put into Constants.java. Personally, I think constants that are strictly only used in subsystems should stay in their subsystem classes and the only appropriate constants that should be put in Constants.java are constants that are used between subsystems or ports. That way, there is less clutter in Constants.java and it is easier to find certain constants. Seems like you agree with me on this, but do we wanna make it convention that all subsystem specific constants (excluding ports) should stay in subsystems.

Other solutions include creating completely new files just to store constants to organize them better. I realize that storing goal positions in Arm.java would be extremely annoying. We should probably discuss and agree on how exactly we want to store constants.

ProfessorAtomicManiac · 2023-10-01T04:06:00Z

src/main/java/org/carlmontrobotics/robotcode2023/Robot.java

+    //set mode to baby if baby was true
+		//else but don't set mode to norm if slow was true
+		if (SmartDashboard.getBoolean("safeMode", false)) {
+			RobotContainer.driverMode = RobotContainer.DriverMode.BABY;
+		} else if (RobotContainer.driverMode.isSlow()) {
+			RobotContainer.driverMode = RobotContainer.DriverMode.NORM;
+		}


Once you enable safeMode or slow the robot, how is it supposed to return back to normal mode when switched off?

Also see my comment in RobotContainer.java

ProfessorAtomicManiac · 2023-10-01T04:24:57Z

src/main/java/org/carlmontrobotics/robotcode2023/RobotContainer.java

+
+  public static enum DriverMode {
+    NORM, SLOW, BABY;
+
+    public boolean isNorm() {
+      return this == DriverMode.NORM;
+    }
+
+    public boolean isSlow() {
+      return this == DriverMode.SLOW;
+    }
+
+    public boolean isBaby() {
+      return this == DriverMode.BABY;
+    }
+  }


I'm really against having the logic for DriverMode snucked away in a class file meant to initialize joysticks and commands. DriverMode logic is also mixed into Robot.java. In the future if someone wants to change up DriverMode, they would have to edit code in RobotContainer.java and Robot.java, both files that I would argue would not be the first place a programmer would think of to look in. Please refactor the driver mode code (maybe make an entire subsystem class just to keep track of DriverMode, as this is used across multiple subsystems. That's just an idea though, maybe there is a less "boilerplaty" solution).

I also know that Ryan implemented a lib199 implmentation of all the safe mode stuff if you want to use that instead. I forgot how it works though.

src/main/java/org/carlmontrobotics/robotcode2023/Robot.java

ProfessorAtomicManiac · 2023-10-01T04:45:34Z

src/main/java/org/carlmontrobotics/robotcode2023/commands/TeleopDrive.java

+    double driveMultiplier = ((DriverMode)mode.get()).isBaby() ? kBabyDriveSpeed : (
+														 ((DriverMode)mode.get()).isSlow() ? kSlowDriveSpeed : kNormalDriveSpeed
+		);
+    double rotationMultiplier = ((DriverMode)mode.get()).isBaby() ? kBabyTurnSpeed : (
+														    ((DriverMode)mode.get()).isSlow() ? kSlowDriveRotation : kNormalDriveRotation
+		);


Reading the code before, I realized that kBabyDriveSpeed was never actually used and baby mode was just "slow mode". This will fix that error but kinda interesting that baby mode was never used.

ProfessorAtomicManiac · 2023-10-01T05:02:33Z

src/main/java/org/carlmontrobotics/robotcode2023/subsystems/Roller.java

+    public static class RollerMode {
+        public static RollerMode INTAKE_CONE = new RollerMode(-0.5, .5, GameObject.CONE, conePickupColor);
+        public static RollerMode INTAKE_CUBE = new RollerMode(0.4, .25, GameObject.CUBE, cubePickupColor);
+        // The obj indicates which game object the roller is trying to intake
+        // if obj == NONE, that means it is trying to outtake rather than intake
+        public static RollerMode OUTTAKE_CONE = new RollerMode(0.5, .5, GameObject.NONE, defaultColor);
+        public static RollerMode OUTTAKE_CUBE = new RollerMode(-0.5, .5, GameObject.NONE, defaultColor);
+        public static RollerMode STOP = new RollerMode(0, .1, GameObject.NONE, defaultColor);
+        public double speed;
+        public double time;
+        public GameObject obj;
+        public Color ledColor;
+
+        /**
+         * @param speed  A number between -1 and 1
+         * @param time   Amount of time in seconds to keep the motor running after
+         *               distance sensor has detected an object
+         * @param intake Whether the roller is outtaking or intaking
+         */
+        public RollerMode(double speed, double time, GameObject obj, Color ledColor) {
+                this.speed = speed;
+                this.time = time;
+                this.obj = obj;
+                this.ledColor = ledColor;
+        }
+    }


See my comment in Constants.java

make robot be able to get back to NORM drivermode shift lambda work from RobotContainer to ArmTeleop itself

BrownBear85 and others added 3 commits April 21, 2023 17:45

safe mode (child mode)

efc4728

child mode refuses arm control

1eba207

Slowmode (dt, arm+wrist, disable X and joysticks)

a92d442

ProfessorAtomicManiac commented Apr 28, 2023

View reviewed changes

FriedLongJohns and others added 4 commits April 28, 2023 17:30

fix

917c156

Fix everything charles whined about except using multipliers instead of limits

Use limits instead of multipliers

6fa4ab0

Add Arm kD value

ddf341b

addding encoder connected code and recalibrated

20da511

ProfessorAtomicManiac commented Sep 29, 2023

View reviewed changes

src/main/java/org/carlmontrobotics/robotcode2023/subsystems/Arm.java Outdated Show resolved Hide resolved

FriedLongJohns added 2 commits September 29, 2023 16:50

Remove unused forbFlag

c3afc26

was an unused controller rumble variable

fix baby vel & accel limits

4039891

wasn't fully implemented before +allow ArmTeleop.getRequestedSpeeds() to use baby max vel +fix Arm.arm/wristConstraints not changing the max ff accel even though we had a baby max accel

merged baby and slow booloean suppliers into slow

cf37551

FriedLongJohns requested a review from TuskAct2 October 1, 2023 00:22

This comment was marked as resolved.

Sign in to view

FriedLongJohns added 5 commits September 30, 2023 17:29

Revert "merged baby and slow booloean suppliers into slow"

f510639

This reverts commit cf37551.

Shift RollerMode to Roller

eec4b4a

was changeable but in constants, now moved to Roller subsystem =Also remove get/set of RollerMode.speed in favor of just making it public (if you both get and set a variable publicly without doing anything special, don't make it private)

Merge branch 'master' into safe-mode

d6b8515

Reorganize lambdas

88cace6

Merge branch 'safe-mode' of https://github.com/DeepBlueRobotics/Robot…

87c5866

…Code2023 into safe-mode

FriedLongJohns approved these changes Oct 1, 2023

View reviewed changes

TuskAct2 and others added 5 commits October 1, 2023 00:54

copied arm encoder dc logic into wrist

46866e3

if encoders are dc then motors will stop

6a5b4a7

shorten functions

6d929d1

fix stupid ifs

e5907cd

merge driver modes

9c4f55b

now only check one enum instead of checking both slow and baby (baby takes priority, of course)

FriedLongJohns assigned ProfessorAtomicManiac and FriedLongJohns Oct 1, 2023

FriedLongJohns assigned TuskAct2 Oct 1, 2023

ProfessorAtomicManiac added 7 commits October 1, 2023 03:55

alt + shift + f, autoformatting everything

fe366fe

added constants together

15f0207

documentation of DISCONNECTED_ENCODER_TIMEOUT_SEC

1812ca5

format RunRoller and TeleopDrive

531783d

remove useless line

becd19a

made wrist encoder disconnection detection work

7ff0122

documentation of disconnected encoder code

8955c82

ProfessorAtomicManiac commented Oct 1, 2023

View reviewed changes

Fix logic

feea3e3

make robot be able to get back to NORM drivermode shift lambda work from RobotContainer to ArmTeleop itself

Conversation

ProfessorAtomicManiac commented Apr 28, 2023

Uh oh!

ProfessorAtomicManiac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FriedLongJohns commented Sep 30, 2023 • edited by ProfessorAtomicManiac Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

ProfessorAtomicManiac commented Sep 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

ProfessorAtomicManiac left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ProfessorAtomicManiac Oct 1, 2023

Choose a reason for hiding this comment

Uh oh!

ProfessorAtomicManiac Oct 1, 2023

Choose a reason for hiding this comment

Uh oh!

ProfessorAtomicManiac Oct 1, 2023

Choose a reason for hiding this comment

Uh oh!

ProfessorAtomicManiac Oct 1, 2023

Choose a reason for hiding this comment

Uh oh!

ProfessorAtomicManiac Oct 1, 2023

Choose a reason for hiding this comment

Uh oh!

ProfessorAtomicManiac Oct 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ProfessorAtomicManiac Oct 1, 2023

Choose a reason for hiding this comment

Uh oh!

ProfessorAtomicManiac Oct 1, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

FriedLongJohns commented Sep 30, 2023 •

edited by ProfessorAtomicManiac

Loading

ProfessorAtomicManiac commented Sep 30, 2023 •

edited

Loading

ProfessorAtomicManiac left a comment •

edited

Loading

ProfessorAtomicManiac Oct 1, 2023 •

edited

Loading