Elevator code is not working perfectly

Projects Programming
1

I'm having a small issue with the code that works my elevator for my engineering project. The code works in the sense that I can go to each floor from the next corresponding floor or from floor 1 to 4 and vice versa. When I want to go from floor 1 to floor 3 or from floor 4 to floor 2 is where the problems happens. The if I am at floor 1 and press floor 3's call button the elevator begins to rise then stops at floor two. I can click floor 3's button again and it will go to 3, but it won't skip over 2. The same thing happens when going from floor 4 to 2 just in the opposite order. I'm am a noob when it comes to programming so please try to keep it simple in your responses.

Elevator_Relay_Programming.ino (3.65 KB)

2

Here is the code so you don't need to download it.

int pushbutton1 = 2;   //pushbutton1 is attached to pin 2
int pushbutton2 = 3;   //pushbutton2 is attached to pin 3
int pushbutton3 = 4;   //pushbutton3 is attached to pin 4
int pushbutton4 = 5;   //pushbutton4 is attached to pin 5
int spst1 = 6;         //Close switch to SPST1 switch
int spst2 = 7;         //Close switch to SPST2 switch
int halleffect1 = 8;   //halleffect 1 is attached to pin 9
int halleffect2 = 9;   //halleffect 2 is attached to pin 10
int halleffect3 = 10;  //halleffect 3 is attached to pin 11
int halleffect4 = 11;  //halleffect 4 is attached to pin 12


void setup() {

//set inputs
pinMode(pushbutton1, INPUT);
pinMode(pushbutton2, INPUT);
pinMode(pushbutton3, INPUT);
pinMode(pushbutton4, INPUT);
pinMode(halleffect1, INPUT);
pinMode(halleffect2, INPUT);
pinMode(halleffect3, INPUT);
pinMode(halleffect4, INPUT);

//set outputs
pinMode(spst1, OUTPUT);
pinMode(spst2, OUTPUT);

//begin serial comm.
Serial.begin(9600);
}

void loop() {
//If lift is called to first floor
if (digitalRead(pushbutton1) == HIGH) {
  //while first floor hall effect is not sensing magnetic field
  while (digitalRead(halleffect1) == HIGH) {
    //run motor down
    motordown();
    Serial.println("Going Down");
  }
}
  //when first floor hall effect senses magnetic field
  else if (digitalRead(halleffect1) == LOW) {
    //stop motor
    motorstop();
    Serial.println("Arriving at Ground Floor");
  }



//If lift is called to second floor
if (digitalRead(pushbutton2) == HIGH) {
  //while first floor hall effect is sensing magnetic field
  while (digitalRead(halleffect1) == LOW) {
    //run motor up
    motorup();
    Serial.println("Going Up");
  }
  //while third or fourth floors hall effects are sensing magnetic field
  while (digitalRead(halleffect3) == LOW || digitalRead(halleffect4) == LOW) {
    //run motor down
    motordown();
    Serial.println("Going Down");
  }
}
  //when second floor hall effect senses magnetic field
  else if (digitalRead(halleffect2) == LOW) {
    //stop motor
    motorstop();
    Serial.println("Arriving at Second Floor");
  }



//If lift is called to third floor  
if (digitalRead(pushbutton3) == HIGH) {
  //while first or second floors hall effects are sensing magnetic field
  while (digitalRead(halleffect1) == LOW || digitalRead(halleffect2) == LOW) {
    //run motor up
    motorup();
    Serial.println("Going Up");
  }
  //while fourth floor hall effect is sensing magnetic field
  while (digitalRead(halleffect4) == LOW) {
    //run motor down
    motordown();
    Serial.println("Going Down");
  }
}
  //when third floor hall effect senses magnetic field
  else if (digitalRead(halleffect3) == LOW) {
    //stop motor
    motorstop();
    Serial.println("Arriving at Third Floor");
  }
  
  
//If lift is called to fourth floor
if (digitalRead(pushbutton4) == HIGH) {
  //while fourth floor hall effect is not sensing magnetic field
  while (digitalRead(halleffect4) == HIGH) {
    //run motor up
    motorup();
    Serial.println("Going Up");
  }
}
  //when fourth floor hall effect senses magnetic field
  else if (digitalRead(halleffect4) == LOW) {
    //stop motor
    motorstop();
    Serial.println("Arriving at Fourth Floor");
  }
}
//run motor in one direction
void motorup() {  
digitalWrite(spst1, HIGH);
digitalWrite(spst2, LOW);
}
//run motor in opposite direction
void motordown() {  
digitalWrite(spst1, HIGH);
digitalWrite(spst2, HIGH);
}
//stop motor
void motorstop() {
digitalWrite(spst1, LOW);
digitalWrite(spst2, LOW);
}
3

You have an awful lot of your code in loop()

It would be much easier to manage the logic if you break the code into functions. Also, I think you need to read all the buttons at the same time and save the values. Then work from those saved values.

Something like this

void loop() {
   readButtons();
   decideDestination();
   operateMotors();
}

Have a look at planning and implementing a program.

At a wild guess, your code for detecting arrival at a floor is not disabled for the floors you want to bypass.

...R

4

This may be a good project to use state machine techniques.

5

Thank you Robin2! I went though the tutorial and also found some better examples of using if, else if, and while statements through google. After a long night of no success I tried it again tonight and finally got it just hours before the project is due tomorrow. I'll post the updated code incase anyone wants to see it.

const int pushbutton1 = 2;   //pushbutton1 is attached to pin 2
const int pushbutton2 = 3;   //pushbutton2 is attached to pin 3
const int pushbutton3 = 4;   //pushbutton3 is attached to pin 4
const int pushbutton4 = 5;   //pushbutton4 is attached to pin 5
const int spst1 = 6;         //activate SPST1 coil 
const int spst2 = 7;         //activate SPST2 coil
const int halleffect1 = 8;   //halleffect 1 is attached to pin 9
const int halleffect2 = 9;   //halleffect 2 is attached to pin 10
const int halleffect3 = 10;  //halleffect 3 is attached to pin 11
const int halleffect4 = 11;  //halleffect 4 is attached to pin 12

void setup() 
{
  //set inputs
  pinMode(pushbutton1, INPUT);
  pinMode(pushbutton2, INPUT);
  pinMode(pushbutton3, INPUT);
  pinMode(pushbutton4, INPUT);
  pinMode(halleffect1, INPUT);
  pinMode(halleffect2, INPUT);
  pinMode(halleffect3, INPUT);
  pinMode(halleffect4, INPUT);

  //set outputs
  pinMode(spst1, OUTPUT);
  pinMode(spst2, OUTPUT);

  //begin serial comm.
  Serial.begin(9600);

}

void loop() 
{
  //Read floor sensors
  digitalRead(halleffect1);
  digitalRead(halleffect2);
  digitalRead(halleffect3);
  digitalRead(halleffect4);

  //run first floor function if button 1 is pressed  
  if (digitalRead(pushbutton1) == HIGH) 
  {
    floor1function();
  }
  //run second floor function if button 2 is pressed
  else if (digitalRead(pushbutton2) == HIGH) 
  {
    floor2function();
  }
  //run third floor function if button 3 is pressed
  else if (digitalRead(pushbutton3) == HIGH) 
  {
    floor3function();
  }
  //run forth floor function if button 4 is pressed
  else if (digitalRead(pushbutton4) == HIGH) 
  {
    floor4function();
  }
  //turn motor off
  digitalWrite(spst1, LOW);
  digitalWrite(spst2, LOW);

}



void floor1function() 
{
  //if first floor sensor is not triggered run motor down while it is not
  if (digitalRead(halleffect1) != LOW)
    while (digitalRead(halleffect1) == HIGH) 
    {
      motordown();
    }
}



void floor2function()
{
  //if 3rd or 4th floor sensors are triggered run motor down while the 2nd floor is not
  if (digitalRead(halleffect3) == LOW || digitalRead(halleffect4) == LOW)
  {
    while (digitalRead(halleffect2) == HIGH)
    {
      motordown();
    }
  }
  //else if the 1st floor sensor is triggered run the motor up while the 2nd floor is not
  else if (digitalRead(halleffect1) == LOW)
  {
    while (digitalRead(halleffect2) == HIGH)
    {
      motorup();
    }
  }
}



void floor3function()
{
  //if 1st or 2nd floor sensors are triggerd run the motor up while the 3rd floor is not
  if (digitalRead(halleffect1) == LOW || digitalRead(halleffect2) == LOW)
  {
    while (digitalRead(halleffect3) == HIGH)
    {
      motorup();
    }
  }
  //else if the 4th floor sensor is triggered run the motor down while the 3rd floor is not
  else if (digitalRead(halleffect4) == LOW)
  {
    while (digitalRead(halleffect3) == HIGH)
    {
      motordown();
    }
  }
}




void floor4function() 
{
  //if the 4th floor sensor is not triggered run the motor up while it is not
  if (digitalRead(halleffect4) != LOW) 
  {
    while (digitalRead(halleffect4) == HIGH) 
    {
      motorup();
    }
  }
}

//run motor in up direction
void motorup() 
{
  digitalWrite(spst1, HIGH);
  digitalWrite(spst2, LOW);
  Serial.println("Going Up");
}

//run motor in down direction
void motordown() 
{
  digitalWrite(spst1, HIGH);
  digitalWrite(spst2, HIGH);
  Serial.println("Going Down");
}
6

That looks good for a first program.

...R

7 
  digitalRead(halleffect1);
  digitalRead(halleffect2);
  digitalRead(halleffect3);
  digitalRead(halleffect4);

And throw the results away. Why bother?

Arrays! Your code screams for arrays and for loops.

floor1function();

Like the pinTypeOneFunction()? Surely you have some idea what this function is supposed to do. And, really, does function need to be part of the function name?

  digitalWrite(spst1, LOW);
  digitalWrite(spst2, LOW);

What the heck? The name implies that the pins have sinlge-pole, single-throw switches attached. How can you write to a switch?

    while (digitalRead(halleffect1) == HIGH) 
    {
      motordown();
    }

Is your Arduino broken? Don't the pins STAY in the state you tell them to be in? There is no need to keep telling the pin to be in some state.

8

Paul S

Like I said I literally just started using Arduino and have never programmed anything in my life, but the program works exactly like I need it to so thats all that matters. I was handed the Arduino in my engineering class barely given any instructions on how to write code so I am sure it could use some fine tuning.

I don't know what an array is and the hall effects need to be read at the beginning of the loop otherwise it wouldn't stop at the right floor(at least with the way I wrote it).

The name for SPST1 and 2 imply their single pole, single throw switches because they are SPST relays. As I'm sure you know a SPST relay or any relay for that matter has a coil that throws the switch, the arduino controls the relays and allows the voltage for my motor to pass through the switch. I had used relays because I have a motor that pulls around 6amps of current.

As for the floor1function it sounds good to me I don't really see what the complaint is.

Finally the hall effects are not told to be in any position. I have a magnet attached to my platform and it is low when the platform is there and high when it is not. So in my head it made sense to say while the hall effect is not seeing a magnetic field run the motor in the direction it needs to go. The only place in the entire code that I tell any pins to be in any state is for the relay pins(SPST1 & 2).

Apparently I wrong to get excited to see it run and post the code without commenting what was going on with. I will edit the original post so maybe its more clear.

9

PaulS:
And throw the results away. Why bother?

@PaulS has sharper eyes than I have, but useless lines do not prevent the code from working.

I think his other comments are about choosing more meaningful names for variables - for example spst1Pin rather than spst1

The point about choosing variable names carefully is so that if humans can more easily understand the code they are less likely to make mistakes in the logic.

Arrays allow you to make code a lot tidier by avoiding the need for several lines that are nearly identical. The more lines you have, the more room there is for a mistake.

However, regardless of these opportunities for improvement, the overall code is easy to follow.

...R

10

the hall effects need to be read at the beginning of the loop otherwise it wouldn't stop at the right floor(at least with the way I wrote it).

You are reading the hall effect sensors blindfolded. Since you don't care what state they are in, nothing is accomplished by reading them.

If you think that something IS, there is something wrong with your hardware or program logic.

As for the floor1function it sounds good to me I don't really see what the complaint is.

The Arduino team could have supplied functions like readFunction1(), readFunction2(), writeFunction1(), writeFunction2(), etc. Then, you'd need a cheat sheet to know which function to call to read a digital pin, or to write to a PWM pin. Instead, they use names that (mostly) make sense.

Does floor1function() cause the elevator to go up or down? The names does not mean a thing, except to you. If anyone else were to use it, you'd need to supply a cheat sheet.

If the function were called upToFloorOne() or downToFloorOne(), anyone could figure out which function to call to go from where the were to where they wanted to be.

The other problem with the names is, of course, that you don't need so many functions. Using my naming convention, you'd need functions upToFloor(n) and downToFloor(n). Then, you'd probably want to create a function, changeLevel(currFloor, destFloor); that would take you from one floor to another. It would figure out whether you needed to go up or down.

Finally, I'm relatively certain that that is what your instructor expects you to figure out.

Finally the hall effects are not told to be in any position. I have a magnet attached to my platform and it is low when the platform is there and high when it is not. So in my head it made sense to say while the hall effect is not seeing a magnetic field run the motor in the direction it needs to go.

In MY mind, it makes more sense to tell the motor to go some direction, and then watch for the appropriate state change to happen. Once you've spurred the horse, and it's started running, there is nothing gained by kicking it again and again. While the pins don't mind, it is not necessary.

    motordown();
    while (digitalRead(halleffect1) == HIGH) 
    {
       // Don't need to do anything until we get to the right floor.
    }

Apparently I wrong to get excited to see it run and post the code without commenting what was going on with.

No, you weren't. I get excited when my code works, too, and like to show it off.

I simply saw some useless code, and some opportunities for improvement. When someone points out stuff in my code that could be improved, I make a copy, and try out there suggestions.

11

Does floor1function() cause the elevator to go up or down? The names does not mean a thing, except to you. If anyone else were to use it, you'd need to supply a cheat sheet.

As far as I can see it makes the elevator go to floor 1 - that seems fairly clear to me.

Lighten up, perhaps ?

...R

12

As far as I can see it makes the elevator go to floor 1 - that seems fairly clear to me.

Which direction? That might seem obvious for floor 1, but not for the other floors.

Lighten up, perhaps ?

I'm as light as I can be.

13

Thank you, I will use both of your criticism for future projects. As for the last 2 comments about the naming of the function yes Paul you are correct it is obvious for the floor 1 and 4 which direction it needs to go. I left it the same as for floors 2 and 3 because within the function it decides which direction to move the motor. So I still don't think I should change that, but its a free world we can each have our own opinion. Thanks again for the help the project was a success(one of the very few that did work). I enjoyed it so much I'll probably refine the build design and code over the summer.

14

PaulS:
Which direction? That might seem obvious for floor 1, but not for the other floors.

That is properly resolved within each function. :slight_smile: :slight_smile:

...R

15

A state machine solution is often far more better suited for these kind of problems than a functional one.

Here is some (untested) code that implemets the state machine described in the diagram using my state machine library

#include <SM.h>
SM elevator(checkBtn);

const int halleffect1 = 8;   //halleffect 1 is attached to pin 9?????
const int spst1 = 6;         //activate SPST1 coil 
const int spst2 = 7;         //activate SPST2 coil

byte regBtns;//register button presses
byte pos;//indicate actual floor postion
byte mask;
enum dir{down, up} currDir;//hold curretn direction for checking and moving

void setup() {
  //set outputs
  pinMode(spst1, OUTPUT);
  pinMode(spst2, OUTPUT);

  //begin serial comm.
  Serial.begin(115200);
  
  //send elevatot to first floor
  motorDown();
  while(!digitalRead(halleffect1));//wait for first floor
  motorStop();
}//setup()

void loop(){
  regBtns |= (PORTD>>2)&0xf;//register button press
  pos = (PORTB>>(halleffect1-8))&0xf;//read postion sensors
  EXEC(elevator);//run statemachine
}//loop()

State checkBtn(){
  if(currDir==down){//check downward from current pos
    mask = (pos>>1)|(pos>>2)|(pos>>3);//set mask to all lower positions
    if(regBtns&mask){//there are registered buttons below current position
      motorDown();//start moving downward
      elevator.Set(moveDir);
    }//if(regBtns&mask)
    else currDir = up;//no registerd buttons, change direction
  }else{//check upward from current position
    mask = ((pos<<1)|(pos<<2)|(pos<<3))&0xf;//set mask to all higher positions
    if(regBtns&mask){//there are registered buttons above current position
      motorUp();//start moving downward
      elevator.Set(moveDir);
    }//if(regBtns&mask)
    else currDir = down;//no registerd buttons, change direction    
  }//if(currDir==down)else
}//checkBtn()

State moveDir(){
  if(regBtns&pos){//elevator has reached a desired position
     motorStop();
     regBtns &= ~pos;//unregister position
     elevator.Set(stopMove);
  }//if(position reached)
}//moveDir()

State stopMove(){
  if(elevator.Timeout(1000)) elevator.Set(checkBtn);
}//stopMove()

//run motor in one direction
void motorUp() {  
  digitalWrite(spst1, HIGH);
  digitalWrite(spst2, LOW);
}//motorUp()

//run motor in opposite direction
void motorDown() {  
  digitalWrite(spst1, HIGH);
  digitalWrite(spst2, HIGH);
}//motordown()

//stop motor
void motorStop() {
  digitalWrite(spst1, LOW);
  digitalWrite(spst2, LOW);
}//motorstop

elevator state digram.png