RobotC - Programming an Elevator

Question 1

I'm not familiar with RobotC or VEX, however I've noticed a certain amount of replicated operations that could be made into their own functions.

The following code snippets I would make into separate functions. So in the large function called motors you have the following set of operations:

    int x = 1;
    while (x < 3)
    {
        SensorValue[LED3] = 1;
        wait(0.5);
        SensorValue[LED3] = 0;
        wait(0.5);
    }
    callup[2] = 0;
    main ();

This is repeated with slightly different values.

Here I'd write a function like the following:

void adjust_sensors( size_t led, size_t level )
{
    int x = 1;
    while (x < 3)
    {
        SensorValue[led] = 1;
        wait(0.5);
        SensorValue[led] = 0;
        wait(0.5);
    }
    callup[level] = 0;
    main ();
}

You can do the same for the following code as well:

startMotor(mainMotor, 60);
untilTouch(limit3);
stopMotor(mainMotor);
callup[2] = 0;
wait(1);
main ();

Also it seems like the while loop will never end because the value of x never changes.

You also have a typo at the top when you declare:

int callown [2];

I presume you meant:

int calldown [2];

Would be good to add some comments to your code as well for clarity.

Hope this helps.

Question 2

Let's define what are the states of an elevator at a given moment:

An elevator can go up, down, or be idle.

enter image description here

The elevator is at a given floor and go from one floor to the other when it trigger a switch:

enter image description here

Now, if we translate this into some pseudo code (which should be easily translated to RobotC) :

enum elevator_status = { idle, down, up };
int currentfloor; //1, 2, 3 


switch(elevator_status)
{
    case idle:    
        //we check if a button is pressed and possibly go up or down           
        if(SensorValue(floor1))
        {         
             if(currentfloor > 1)
                elevator_status = down;               
        }
        else if(SensorValue(floor2))
        {
              if(currentfloor > 2)
                  elevator_status = down;
              else if(currentfloor < 2)
                  elevator_status = up;               
        }
        else if(SensorValue(floor3))
        {
              if(currentfloor < 3)
                  elevator_status = up;   
        }
        break;

    case up:
    case down:    
        //we check if we trigger a floor switch and stop the elevator
        if(SensorValue(limit1))
        {      
           currentfloor = 1;
           elevator_status = idle;
        }
        else if(SensorValue(limit2))
        {
           currentfloor = 2;
           elevator_status = idle;
        }
        else if(SensorValue(limit3))
        {
           currentfloor = 3;
           elevator_status = idle;
        }
        break;
}


//we set the speed of the motor
if(elevator_status == up)
{
   set_motorstate(cw);
)
else if(elevator_status == down)
{
   set_motorstate(ccw);   
}
else if(elevator_status == idle)
{
   set_motorstate(idle);   
}

Note : in this code the elevator will only take care of new up and down floor calls when the elevator is idle. It does not store up and down call while it is moving and go there later. I do not know if it was a requirement for you.

Question 3

I could be way off, because I'm just a student with questions of my own but it looks like you may have made a mistake in your array sizes. For instance, when you declared:

int floorat[2];

This made the array size 2. Then you refer to 3 element locations in this array [0, 1, 2]. Also, can't you just use a regular integer, and assign it values 1, 2, or 3?

I would recommend redefining these varaibles as:

int callup;
int calldown;
int floorat;

Then you can avoid extra lines of code and simplify the if/else clauses to:

if (SensorValue[limit1] == 1)
{
    floorat = 1;
}

if (SensorValue[limit2] == 1)
{
    floorat = 2;
}


if (SensorValue[limit3] == 1)
{
    floorat = 3;
}