Anyone want to review my code?

Our robot is currently in in pieces so I can’t really test anything, so if any experienced pros wants to review my code and point out any stupid things I’m doing, I’d be really thankful haha.
For some context-- we have a traybot like every other team and I haven’t been able to test our autonomous so I’ve been just trying random simple things to eventually tweak when our robot is back together again.
Thanks!

 #include "vex.h"

 using namespace vex;

 vex::competition Competition;
 controller Controller1;
 
 private static double SMALLTOWER_HEIGHT = .25; //in rotations (1 rotation = 360 degrees)
 private static double MEDTOWER_HEIGHT = .5;
 private static double TALLTOWER_HEIGHT = .75;
 motor RightWheel = motor(PORT1, ratio18_1, true);
 motor LeftWheel = motor(PORT2, ratio18_1, false);
 motor RightArm = motor(PORT3, ratio36_1, false);
 motor LeftArm = motor(PORT5, ratio36_1, true);
 motor RightIntake = motor(PORT4, ratio6_1, false);
 motor LeftIntake = motor(PORT7, ratio6_1, true);
 motor Lift = motor(PORT6, ratio36_1, false);

 int startPosition = 0; //for autonomous
 bool preauton = false;
 bool autonomousMode = false;
 bool driverMode = false;
 bool controlsToggle = true;

 void pre_auton(void) { //choose robot position
     preauton = true;
     driverMode = false;
     autonomousMode = false;
     //0 = blueBig, 1 = blueSmall, 2 = redBig, 3 = redSmall
     while (preauton) {
         if (Brain.Screen.pressing()) {
             startPosition++;
             if (startPosition > 3) {
                 startPosition = 0;
             }
         }
         Brain.Screen.clearScreen();
         if (startPosition == 0) {
             Brain.Screen.printAt(115, 128, "Blue Big (tap to toggle)");
         } else if (startPosition == 1) {
             Brain.Screen.printAt(115, 128, "Blue Small (tap to toggle)");
         } else if (startPosition == 2) {
             Brain.Screen.printAt(115, 128, "Red Big (tap to toggle)");
         } else if (startPosition == 3) {
             Brain.Screen.printAt(115, 128, "Red Small (tap to toggle)");
         }

         task::sleep(300);
     }
 }

 void autonomous(void) { //runs during autonomous
     Brain.Screen.clearScreen();
     preauton = false;
     driverMode = false;
     autonomousMode = true;
     if (startPosition == 0) {
         /*------------------------------------------------------------------------------------------------
         BLUE BIG CODE
         ------------------------------------------------------------------------------------------------*/
         while (true) {
          //move backwards
          LeftWheel.startRotateFor(directionType::fwd, -5, rotationUnits::rev,50, velocityUnits::rpm);
          RightWheel.rotateFor(directionType::fwd, -5, rotationUnits::rev, 50, velocityUnits::rpm);
          //move forwards
          LeftWheel.startRotateFor(directionType::fwd, 5, rotationUnits::rev,50, velocityUnits::rpm);
          RightWheel.rotateFor(directionType::fwd, 5, rotationUnits::rev, 50, velocityUnits::rpm);
          //rotate left
          LeftWheel.startRotateFor(directionType::fwd, -5, rotationUnits::rev,50, velocityUnits::rpm);
          RightWheel.rotateFor(directionType::fwd, 5, rotationUnits::rev, 50, velocityUnits::rpm);
          //rotate right
          LeftWheel.startRotateFor(directionType::fwd, 5, rotationUnits::rev,50, velocityUnits::rpm);
          RightWheel.rotateFor(directionType::fwd, -5, rotationUnits::rev, 50, velocityUnits::rpm);
          task::sleep(20000); //20 seconds
          break;
         }
         //------------------------------------------------------------------------------------------------
     } else if (startPosition == 1) {
         /*
         BLUE SMALL CODE
         */
     } else if (startPosition == 2) {
         /*
         RED BIG CODE
         */
     } else if (startPosition == 3) {
         /*
         RED SMALL CODE
         */
     }

 }

 void usercontrol(void) { //runs during driver control (and also runs by default outside of competition)
     Brain.Screen.clearScreen();
     preauton = false;
     driverMode = true;
     autonomousMode = false;
     //initiate controller screen
     Controller1.Screen.setCursor(2, 2);
     Controller1.Screen.clearLine();
     while (driverMode) {
       
       if (Controller1.ButtonLeft.pressing()) 
       {
         //move to small tower height
         while(RightArm.rotation(rev) < SMALLTOWER_HEIGHT && LeftArm.rotation(rev) < SMALLTOWER_HEIGHT)
         {
            if (RightArm.rotation(rev) < SMALLTOWER_HEIGHT)
            {
              RightArm.spin(fwd, 50, velocityUnits::pct);
            }
            if (LeftArm.rotation(rev) < SMALLTOWER_HEIGHT)
            {
              LeftArm.spin(fwd, 50, velocityUnits::pct);
            }
         }
       }
       if (Controller1.ButtonUp.pressing()) 
       {
         //move to medium tower height
         while(RightArm.rotation(rev) < MEDTOWER_HEIGHT && LeftArm.rotation(rev) < MEDTOWER_HEIGHT)
         {
            if (RightArm.rotation(rev) < MEDTOWER_HEIGHT)
            {
              RightArm.spin(fwd, 50, velocityUnits::pct);
            }
            if (LeftArm.rotation(rev) < MEDTOWER_HEIGHT)
            {
              LeftArm.spin(fwd, 50, velocityUnits::pct);
            }
         }
       }
       if (Controller1.ButtonRight.pressing()) 
       {
         //move to tall tower height
         while(RightArm.rotation(rev) < TALLTOWER_HEIGHT && LeftArm.rotation(rev) < TALLTOWER_HEIGHT)
         {
            if (RightArm.rotation(rev) < TALLTOWER_HEIGHT)
            {
              RightArm.spin(fwd, 50, velocityUnits::pct);
            }
            if (LeftArm.rotation(rev) < TALLTOWER_HEIGHT)
            {
              LeftArm.spin(fwd, 50, velocityUnits::pct);
            }
         }
       }
       
     
         if (Controller1.ButtonDown.pressing()) {
             controlsToggle = !controlsToggle;
             task::sleep(100);
         }
         if (!controlsToggle) {
             Controller1.Screen.clearLine();
             Controller1.Screen.print("TANK CONTROL          ");
             //tank driving controls
             LeftWheel.spin(vex::directionType::fwd, Controller1.Axis3.value(), vex::velocityUnits::pct);
             RightWheel.spin(vex::directionType::fwd, Controller1.Axis2.value(), vex::velocityUnits::pct);

         } else if (controlsToggle) {
             Controller1.Screen.clearLine();
             Controller1.Screen.print("ARCADE CONTROL        ");

             //arcade driving controls
             LeftWheel.spin(directionType::fwd, (Controller1.Axis3.value() + Controller1.Axis4.value()) / 2, velocityUnits::pct);
             RightWheel.spin(directionType::fwd, (Controller1.Axis3.value() - Controller1.Axis4.value()) / 2, velocityUnits::pct);
             //right stick arm
             RightArm.spin(directionType::fwd, Controller1.Axis2.value(), velocityUnits::pct);
             LeftArm.spin(directionType::fwd, Controller1.Axis2.value(), velocityUnits::pct);
      
             
         }
         //Intake controls (right trigger)
         if (Controller1.ButtonR2.pressing()) {
             RightIntake.spin(directionType::fwd, 25, velocityUnits::pct);
             LeftIntake.spin(directionType::fwd, 25, velocityUnits::pct);
         } else if (Controller1.ButtonR1.pressing()) {
             RightIntake.spin(directionType::rev, 50, velocityUnits::pct);
             LeftIntake.spin(directionType::rev, 50, velocityUnits::pct);
         }
         //arm controls (left trigger)
         
         if (Controller1.ButtonL2.pressing()) {
             RightArm.spin(directionType::rev, 25, velocityUnits::rpm);
             LeftArm.spin(directionType::rev, 25, velocityUnits::rpm);
         } else if (Controller1.ButtonL1.pressing()) {
             RightArm.spin(directionType::fwd, 25, velocityUnits::rpm);
             LeftArm.spin(directionType::fwd, 25, velocityUnits::rpm);
         }

         //lift controls (X and B)
         if (Controller1.ButtonX.pressing()) {
             Lift.spin(directionType::fwd, 40, velocityUnits::rpm);
         } else if (Controller1.ButtonB.pressing()) {
             Lift.spin(directionType::rev, 40, velocityUnits::rpm);
         } else {
             Lift.stop(hold);
         }

         vex::task::sleep(20); //Sleep the task for a short amount of time to prevent wasted resources. 
     }
 }

 
 int main() {
     //autonomous(); //for testing outside of competiton
     //usercontrol(); //for testing outside of competition
     pre_auton(); //run pre autonomous code

     Competition.autonomous(autonomous);

     Competition.drivercontrol(usercontrol); 

     ///prevent main from exiting with an infinite loop (just in case)                  
     while (1) {
         vex::task::sleep(100); //sleep to prevent wasted resources
     }

 }

Code review: Search for the string Note:

 #include "vex.h"

 using namespace vex;

 vex::competition Competition;
 controller Controller1;
 //Note: Good use of global variables to declare your tower heights.
 private static double SMALLTOWER_HEIGHT = .25; //in rotations (1 rotation = 360 degrees)
 private static double MEDTOWER_HEIGHT = .5;
 private static double TALLTOWER_HEIGHT = .75;
 
 motor RightWheel = motor(PORT1, ratio18_1, true);
 motor LeftWheel = motor(PORT2, ratio18_1, false);
 motor RightArm = motor(PORT3, ratio36_1, false);
 motor LeftArm = motor(PORT5, ratio36_1, true);
 motor RightIntake = motor(PORT4, ratio6_1, false);
 motor LeftIntake = motor(PORT7, ratio6_1, true);
 motor Lift = motor(PORT6, ratio36_1, false);

 int startPosition = 0; //for autonomous
 //Note: What is the reason for the following variables?  They are only set to true in 
 //their own respective functions.  What purpose, outside of the function, do they serve?
 bool preauton = false;
 bool autonomousMode = false;
 bool driverMode = false;
 bool controlsToggle = true;

 void pre_auton(void) { //choose robot position
	 preauton = true;
	 driverMode = false;
	 autonomousMode = false;
	 //0 = blueBig, 1 = blueSmall, 2 = redBig, 3 = redSmall
	 while (preauton) {
		 if (Brain.Screen.pressing()) {
			 startPosition++;
			 if (startPosition > 3) {
				 startPosition = 0;
			 }
		 }
		 Brain.Screen.clearScreen();
		 if (startPosition == 0) {
			 Brain.Screen.printAt(115, 128, "Blue Big (tap to toggle)");
		 } else if (startPosition == 1) {
			 Brain.Screen.printAt(115, 128, "Blue Small (tap to toggle)");
		 } else if (startPosition == 2) {
			 Brain.Screen.printAt(115, 128, "Red Big (tap to toggle)");
		 } else if (startPosition == 3) {
			 Brain.Screen.printAt(115, 128, "Red Small (tap to toggle)");
		 }

		 task::sleep(300);
	 }
 }

 void autonomous(void) { //runs during autonomous
	 Brain.Screen.clearScreen();
	 preauton = false;
	 driverMode = false;
	 autonomousMode = true;
	 if (startPosition == 0) {
		 /*------------------------------------------------------------------------------------------------
		 BLUE BIG CODE
		 ------------------------------------------------------------------------------------------------*/
		 //Note: Why are you using a while(true) loop in autonomous? Not necessary unless you want your code to repeat.
		 while (true) {
		  //move backwards
		  LeftWheel.startRotateFor(directionType::fwd, -5, rotationUnits::rev,50, velocityUnits::rpm);
		  RightWheel.rotateFor(directionType::fwd, -5, rotationUnits::rev, 50, velocityUnits::rpm);
		  //move forwards
		  LeftWheel.startRotateFor(directionType::fwd, 5, rotationUnits::rev,50, velocityUnits::rpm);
		  RightWheel.rotateFor(directionType::fwd, 5, rotationUnits::rev, 50, velocityUnits::rpm);
		  //rotate left
		  LeftWheel.startRotateFor(directionType::fwd, -5, rotationUnits::rev,50, velocityUnits::rpm);
		  RightWheel.rotateFor(directionType::fwd, 5, rotationUnits::rev, 50, velocityUnits::rpm);
		  //rotate right
		  LeftWheel.startRotateFor(directionType::fwd, 5, rotationUnits::rev,50, velocityUnits::rpm);
		  RightWheel.rotateFor(directionType::fwd, -5, rotationUnits::rev, 50, velocityUnits::rpm);
		  //Note: You are using the following wait to keep your Automous from repeating.
		  //Not necessary if you remove the while loop.
		  task::sleep(20000); //20 seconds
		  break;
		 }
		 //------------------------------------------------------------------------------------------------
	 } else if (startPosition == 1) {
		 /*
		 BLUE SMALL CODE
		 */
	 } else if (startPosition == 2) {
		 /*
		 RED BIG CODE
		 */
	 } else if (startPosition == 3) {
		 /*
		 RED SMALL CODE
		 */
	 }

 }

 void usercontrol(void) { //runs during driver control (and also runs by default outside of competition)
	 Brain.Screen.clearScreen();
	 preauton = false;
	 driverMode = true;
	 autonomousMode = false;
	 //initiate controller screen
	 Controller1.Screen.setCursor(2, 2);
	 Controller1.Screen.clearLine();
	 while (driverMode) {
	   
	   if (Controller1.ButtonLeft.pressing()) 
	   {
		 //move to small tower height
		 //Note: Using a 'while' statement within your code may lock it up until the while is not true.
		 //Meaning all other controls will stop responding while the code is in this loop. 
		 //If the arm fails or cannot move to that position then your robot will stop all code because it is trapped in the loop.
         //Consider using 'if' statements instead of while.
		 while(RightArm.rotation(rev) < SMALLTOWER_HEIGHT && LeftArm.rotation(rev) < SMALLTOWER_HEIGHT)
		 {
			if (RightArm.rotation(rev) < SMALLTOWER_HEIGHT)
			{
			  RightArm.spin(fwd, 50, velocityUnits::pct);
			}
			if (LeftArm.rotation(rev) < SMALLTOWER_HEIGHT)
			{
			  LeftArm.spin(fwd, 50, velocityUnits::pct);
			}
		 }
	   }
	   if (Controller1.ButtonUp.pressing()) 
	   {
		 //move to medium tower height
		 while(RightArm.rotation(rev) < MEDTOWER_HEIGHT && LeftArm.rotation(rev) < MEDTOWER_HEIGHT)
		 {
			if (RightArm.rotation(rev) < MEDTOWER_HEIGHT)
			{
			  RightArm.spin(fwd, 50, velocityUnits::pct);
			}
			if (LeftArm.rotation(rev) < MEDTOWER_HEIGHT)
			{
			  LeftArm.spin(fwd, 50, velocityUnits::pct);
			}
		 }
	   }
	   if (Controller1.ButtonRight.pressing()) 
	   {
		 //move to tall tower height
		 while(RightArm.rotation(rev) < TALLTOWER_HEIGHT && LeftArm.rotation(rev) < TALLTOWER_HEIGHT)
		 {
			if (RightArm.rotation(rev) < TALLTOWER_HEIGHT)
			{
			  RightArm.spin(fwd, 50, velocityUnits::pct);
			}
			if (LeftArm.rotation(rev) < TALLTOWER_HEIGHT)
			{
			  LeftArm.spin(fwd, 50, velocityUnits::pct);
			}
		 }
	   }
	   
	 //Note: Without using an additional latch variable you may have issues with your modes flipping
	 // back and forth if you hold the button down too long.
		 if (Controller1.ButtonDown.pressing()) {
			 controlsToggle = !controlsToggle;
			 task::sleep(100);
		 }
		 //Note: The if / else if can be simplified.  
		 //if (controlsToggle) {//Arcade code } else {//Tank code}
		 if (!controlsToggle) {
			 Controller1.Screen.clearLine();
			 Controller1.Screen.print("TANK CONTROL          ");
			 //tank driving controls
			 LeftWheel.spin(vex::directionType::fwd, Controller1.Axis3.value(), vex::velocityUnits::pct);
			 RightWheel.spin(vex::directionType::fwd, Controller1.Axis2.value(), vex::velocityUnits::pct);

		 } else if (controlsToggle) {
			 Controller1.Screen.clearLine();
			 Controller1.Screen.print("ARCADE CONTROL        ");

			 //arcade driving controls
			 LeftWheel.spin(directionType::fwd, (Controller1.Axis3.value() + Controller1.Axis4.value()) / 2, velocityUnits::pct);
			 RightWheel.spin(directionType::fwd, (Controller1.Axis3.value() - Controller1.Axis4.value()) / 2, velocityUnits::pct);
			 //right stick arm
			 RightArm.spin(directionType::fwd, Controller1.Axis2.value(), velocityUnits::pct);
			 LeftArm.spin(directionType::fwd, Controller1.Axis2.value(), velocityUnits::pct);
	  
			 
		 }
		 //Intake controls (right trigger)
		 if (Controller1.ButtonR2.pressing()) {
			 RightIntake.spin(directionType::fwd, 25, velocityUnits::pct);
			 LeftIntake.spin(directionType::fwd, 25, velocityUnits::pct);
		 } else if (Controller1.ButtonR1.pressing()) {
			 RightIntake.spin(directionType::rev, 50, velocityUnits::pct);
			 LeftIntake.spin(directionType::rev, 50, velocityUnits::pct);
		 }
		 //Note: No code to stop the Intake. Is that planned or is .stop() code missing?
		 
		 //arm controls (left trigger)
		 if (Controller1.ButtonL2.pressing()) {
			 RightArm.spin(directionType::rev, 25, velocityUnits::rpm);
			 LeftArm.spin(directionType::rev, 25, velocityUnits::rpm);
		 } else if (Controller1.ButtonL1.pressing()) {
			 RightArm.spin(directionType::fwd, 25, velocityUnits::rpm);
			 LeftArm.spin(directionType::fwd, 25, velocityUnits::rpm);
		 }
		//Note: No code to stop the Arm. Is that planned or is .stop() code missing?

		 //lift controls (X and B)
		 if (Controller1.ButtonX.pressing()) {
			 Lift.spin(directionType::fwd, 40, velocityUnits::rpm);
		 } else if (Controller1.ButtonB.pressing()) {
			 Lift.spin(directionType::rev, 40, velocityUnits::rpm);
		 } else {
			 Lift.stop(hold);
		 }

		 vex::task::sleep(20); //Sleep the task for a short amount of time to prevent wasted resources. 
	 }
 }

 
 int main() {
	 //autonomous(); //for testing outside of competiton
	 //usercontrol(); //for testing outside of competition
	 pre_auton(); //run pre autonomous code

	 Competition.autonomous(autonomous);

	 Competition.drivercontrol(usercontrol); 

	 ///prevent main from exiting with an infinite loop (just in case)                  
	 while (1) {
		 vex::task::sleep(100); //sleep to prevent wasted resources
	 }

 }
3 Likes

This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.