Catapult Functions to Different Buttons

I’ve added a little bit of test code to our main code, to see if my programming skills are up to par.
My task is as follows:
Or robot is a catapult. We use a pronged intake. When it is pushed all the way down, a slipgear causes it to fling upward with the force of a thousand bulls charging at once. We have a limit switch on the front of the robot and a limit switch on the back of the robot. It works similar to the RI3D team’s. When the catapult is fresh after firing, and the arm is pointing upward, it’s pressing the back limit switch. When the arm is brought down and the intake is parallel to the field floor, the front limit switch is being pressed. If the catapult is moved about an inch even lower, the slipgear will slip and the catapult will launch, flinging the arm up and hitting to back limit switch.
I want it so that if the back limit switch is pressed, I can press button 8D and the catapult will pull down until it hits the front limit switch. When I press 6D, it should bring the catapult arm even lower and fire the catapult back to the original position. At the same time, if the front limit switch is pressed and instead of firing it I want to pull it back up to the back limit switch, I can press 8R.
I need advice on whether or not the following chunk of code will serve this purpose effectively, if there are any flaws, and how I can rearrange or revise the code to either be cleaner or less repetitive.

//TEST CODE FOR CATAPULT MOTORS:

		//Mini pre-note to self & teammates: When referring to buttons, use Xbox controller button names for easy reference

		//If the FRONT limit switch is pressed...
		if (SensorValue[FrontLimit] == 1) {

			// 1. If the Right Trigger is pressed, Shoot the catapult
			while (vexRT[Btn6D] == 1) {
				motor[BackLeftCatapult] = 127;
				motor[BackRightCatapult] = 127;
				motor[FrontLeftCatapult] = 127;
				motor[FrontRightCatapult] = 127;
			}

			// 2. If Button "B" is pressed, Pull arm back to back limit switch
			if (vexRT[Btn8R] == 1) {
				while (SensorValue[BackLimit] == 0) {
					motor[BackLeftCatapult] = -127;
					motor[BackRightCatapult] = -127;
					motor[FrontLeftCatapult] = -127;
					motor[FrontRightCatapult] = -127;
				}
			}

		}

		//If the BACK Limit Switch is pressed...
		if (SensorValue[BackLimit] == 1) {

			// 1. If Button "A" is pressed, Pull down Catapult
			if (vexRT[Btn8D] == 1) {
				while (SensorValue[FrontLimit] == 0) {
					motor[BackLeftCatapult] = 127;
					motor[BackRightCatapult] = 127;
					motor[FrontLeftCatapult] = 127;
					motor[FrontRightCatapult] = 127;
				}
			}
		}



		//END OF TEST CODE

I see you also asked the community about this. I will take a look at your code this weekend and see if I can give some suggestions.

The first thing I would do is get the catapult motor control into a function so you do not have to set four motors each time. Next I would separate the switch detection from the joystick button control and use what is called a state machine, this is probably overkill for your situation but it’s a useful concept to learn. The idea is that your catapult will move between two distinct states, either at the back waiting to be armed or at the front waiting to be either fired or dis-armed. Depending on which state you are in enables certain actions. Many state machines will have multiple states with sensors (or delays, or several sequential actions) causing the logic to move between them. Here is a version of your code written in this way.

typedef enum _tCatapultState{
    kCatapultStateArming = 1,
    kCatapultStateFiring
} tCatapultState;

void
setCatapultMotors( int speed ) {
    motor BackLeftCatapult   ] = speed;
    motor BackRightCatapult  ] = speed;
    motor FrontLeftCatapult  ] = speed;
    motor FrontRightCatapult ] = speed;
}

task main()
{
    tCatapultState state;
    
    while(1) {
      if( SensorValue BackLimit ] ) {
        // Catapult is at back
        state = kCatapultStateArming;
      }
      else
      if( SensorValue FrontLimit ] ) {
        // Catapult is at front 
        state = kCatapultStateFiring;
      }
      
      // Now check for buttons based on where we think we are
      switch(state) {
        case kCatapultStateArming:
          // Only Btn8D is valid
          if( vexRT Btn8D ] ) {
            setCatapultMotors( 127 );
          }
          else {
            setCatapultMotors( 0 );
          }
          break;
          
        case kCatapultStateFiring:
          // Btn6D and Btn8R are valid
          if( vexRT Btn6D ] ) {
            setCatapultMotors( 127 );
          }
          else
          if( vexRT Btn8R ] ) {
            setCatapultMotors( -127 );
          }
          else
            setCatapultMotors( 0 );
          break;
                
        default:
          // No switches were pressed so we need to start somewhere
          state = kCatapultStateArming;
          break;
      }
    
    // no need to run any faster
    wait1Msec(20);  
    }
}

I assume the switches are setup as touch sensors so pressing gives a value of 1). Any drive or other code is added before the final wait statement.

I’m also using what are known as enums in this code. The variable “state” is a special sort of integer that can only have certain values, in this case the values are kCatapultStateArming and kCatapultStateFiring (which in reality will be 1 and 2), these are defined in a similar way to a structure. Using enums in this way makes the code much more readable and helps avoid errors by setting the value of state to something unexpected.