4

I'm designing and programming an elevator-like robot for a high school project. Could I possibly do anything to make this any simpler? Or better? I have attached a picture of my design that I made in AutoCAD Inventor with labels.

enter image description here

For those not familiar with RobotC or VEX (it is VERY similar to C and C++): the limit switches (limit1, limit2, ...) and bump switches (floor1, floor2, ...) are analog buttons and return a value of 0 if not pressed and 1 if pressed. The motor (mainMotor) rotates the gear which causes the mechanism to travel upwards on the slide. When the shaft sticking out the motor mechanism moves up and down, it presses limit switches and causes it to return a value of 1.

int callup [3];
int calldown [3];
int floorat[3];

    int main ()
    {

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

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

        if (SensorValue[limit3] == 1)
        {
            floorat[2] = 1;
        }
        else
        {
            floorat[2] = 0;
        }

        if (SensorValue[floor1] == 1)
        {
            calldown[0] = 1;
            SensorValue[LED1] = 1;
        }

        if (SensorValue[floor2] == 1 && floorat[2] == 1)
        {
            calldown[1] = 1;
            SensorValue[LED2] = 1;
        }

        if (SensorValue[floor2] == 1 && floorat[0] == 1)
        {
            callup[1] = 1;
            SensorValue[LED2] = 1;
        }

        if (SensorValue[floor3])
        {
            callup[2] = 1;
            SensorValue[LED3] = 1;
        }

        motors ();

    }


    void motors ()
    {

        if (callup[2] == 1 && floorat[2] == 1)
        {
            int x = 1;
            while (x < 3)
            {
                SensorValue[LED3] = 1;
                wait(0.5);
                SensorValue[LED3] = 0;
                wait(0.5);
            }
            callup[2] = 0;
            main ();
        }
        else if (callup[1] == 1 && floorat[1] == 1)
        {
            int x = 1;
            while (x < 3)
            {
                SensorValue[LED2] = 1;
                wait(0.5);
                SensorValue[LED2] = 0;
                wait(0.5);
            }
            callup[1] = 0;
            main ();
        }
        else if (callup[0] == 1 && floorat[0] == 1)
        {
            int x = 1;
            while (x < 3)
            {
                SensorValue[LED1] = 1;
                wait(0.5);
                SensorValue[LED1] = 0;
                wait(0.5);
            }
            callup[0] = 0;
            main ();
        }

        if (callup[2] == 1 && floorat[1] == 1 && calldown[0] == 0 || callup[2] == 1 && floorat[0] == 1 && callup[1] == 0)
        {
            startMotor(mainMotor, 60);
            untilTouch(limit3);
            stopMotor(mainMotor);
            callup[2] = 0;
            wait(1);
            main ();
        }

        if (callup[1] == 1 && floorat[0] == 1)
        {
            startMotor(mainMotor, 60);
            untilTouch(limit2);
            stopMotor(mainMotor);
            callup[1] = 0;
            wait(1);
            main();
        }

        if (calldown[1] == 1 && floorat[2] == 1)
        {
            startMotor(mainMotor, -60);
            untilTouch(limit2);
            stopMotor(mainMotor);
            calldown[1] = 0;
            wait(1);
            main();
        }

        if (calldown[0] == 1 && floorat[2] == 1 && calldown[1] == 0 || calldown[0] == 1 && floorat[1] == 1)
        {
            startMotor(mainMotor, -60);
            untilTouch(limit1);
            stopMotor(mainMotor);
            calldown[0] = 0;
            wait(1);
            main();
        }
    }

Although it shouldn't be a concern for this question, the 60 in the startMotor command is the speed of the motor, just to make it clearer.

Feel free to ask any more questions.

Sam Javed
  • 115
  • 2
  • 2
  • 9
  • You might consider how you'd represent your elevator system in terms of tables, if the language allows. Consider what the code might look like for 100 floors instead of just 3. How would you write that so that only a few lines of code can consult a larger set of tables / data structures to know what to do? – Joe Z Nov 09 '13 at 21:37
  • You may also want to consider a `while` loop. Really, anything other than the bottomless recursion and calling of `main()` would be better. Never mind the fact that calling `main()` is a no-no, the recursion will blow your stack eventually. – Macattack Nov 09 '13 at 21:42
  • I could use while loops and/or for loops. But I didn't use them in here because the only floor that would actually need a loop would be floor2 and it was easier to just type the code out manually. – Sam Javed Nov 09 '13 at 21:44
  • It may have been easier to do that way, but unless VEX is very different from normal C your implementation will eventually crash. I prefer my elevators reliable. :) – Retired Ninja Nov 09 '13 at 21:48
  • What could I do to not have to refer to main() over and over? It's basically C with special variables, so suggestions from C users would be helpful too. – Sam Javed Nov 09 '13 at 21:51
  • Shame you didn't post this over on [Robotics Stack Exchange](http://robotics.stackexchange.com/). *8') – Mark Booth Nov 09 '13 at 23:08
  • you should only ever call main in obfuscated code challenges. – Grady Player Nov 10 '13 at 01:49

3 Answers3

3

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.

tigrou
  • 4,236
  • 5
  • 33
  • 59
2

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;
}
River
  • 53
  • 3
  • 8
  • when you declare an array (floorat[2]), it creates three variables: floorat[0], floorat[1], and floorat[2]. That's what I was accessing later in the code. And as I just said, those are not just single variables, they're arrays so I couldn't rename them. And I also need to check each floor later on in the code, so simplifying it down to just one variable would not be helpful at all. – Sam Javed Nov 09 '13 at 21:49
  • In C declaring `int f[2];` gives you space for two integers indexed at 0 and 1. It's possible your language is different, but you should check and be sure. – Retired Ninja Nov 09 '13 at 21:51
  • @user2974881 When you declare an array in C, the value in the brackets is the amount of locations in the array. To reference them use 0 to value, because the first location is 0, not 1. What I meant is that it appears you only use the floorat array to determine what floor the elevator is at (based on limit), so instead of doing a complex if else system to check if it is true that it is on floor 1, then floor 2, then floor 3, and assign a 0 or 1 to each part, you could just use an int and assign the floor value to it, then check for a 1, 2, or 3, not a 0 or 1 in three different array locations. – River Nov 09 '13 at 21:57
  • Yes, I noticed that. I mixed it up with Visual Basic. I cannot imagine how confusing it might be after I learn more languages. – Sam Javed Nov 09 '13 at 21:58
2

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.

Simon Bosley
  • 1,114
  • 3
  • 18
  • 41