Problem with buttons on simple 3 story elevator

I started coding for about a month and I did this project 3 story elevator, I have a problem with my code, the problem is the elevator only moves while the button is being pressed down, if I release it motor will stop also. However, it has no problem with stopping on the right floor even though I am still pressing down the button. Can someone help me point out the issue with my code and fix it?


```cpp
#include <NewPing.h>
#include <LiquidCrystal_I2C.h>
#include <Wire.h>

const int enA = 10;
const int in1 = 2;
const int in2 = 3;
const int trigPin = A0;
const int echoPin = A1;
NewPing sonar(trigPin, echoPin, 40);
const int floor1Pin = 7;
const int floor2Pin = 8;
const int floor3Pin = 12;

LiquidCrystal_I2C lcd(0x27, 16, 2);

void setup() {
  lcd.init();
  lcd.backlight();

  Serial.begin(9600);

  pinMode(enA, OUTPUT);
  pinMode(in1, OUTPUT);
  pinMode(in2, OUTPUT);
  pinMode(trigPin, OUTPUT);
  pinMode(echoPin, INPUT);
  pinMode(floor1Pin, INPUT_PULLUP);
  pinMode(floor2Pin, INPUT_PULLUP);
  pinMode(floor3Pin, INPUT_PULLUP);

  digitalWrite(in1, LOW);
  digitalWrite(in2, LOW);
  analogWrite(enA, 0);

  lcd.setCursor(0, 0);
  lcd.print("Current Floor: 1");
}

void loop() {
  int motorCommand = 0;

  if (digitalRead(floor1Pin) == LOW) {
    motorCommand = 1;
  } else if (digitalRead(floor2Pin) == LOW) {
    motorCommand = 2;
  } else if (digitalRead(floor3Pin) == LOW) {
    motorCommand = 3;
  }

  int distance = getDistance();

  switch (motorCommand) {
    case 1:
      controlMotor(distance, 2, 3);
      displayFloor(1);
      break;
    case 2:
      controlMotor(distance, 20, 21);
      displayFloor(2);
      break;
    case 3:
      controlMotor(distance, 38, 39);
      displayFloor(3);
      break;
    default:

      motorStop();
      break;
  }
}

void controlMotor(int distance, int lowerThreshold, int upperThreshold) {

  if (distance >= lowerThreshold && distance <= upperThreshold) {
    motorStop();
  } else if (distance > upperThreshold) {
    moveDown();
  } else if (distance < lowerThreshold) {
    moveUp();
  }

  Serial.println("Ultrasonic reading: " + String(distance) + " cm");
}

void moveUp() {
  digitalWrite(in1, HIGH);
  digitalWrite(in2, LOW);
  analogWrite(enA, 255);
}

void moveDown() {
  digitalWrite(in1, LOW);
  digitalWrite(in2, HIGH);
  analogWrite(enA, 255);
}

void motorStop() {
  digitalWrite(in1, LOW);
  digitalWrite(in2, LOW);
  analogWrite(enA, 0);
}

void displayFloor(int targetFloor) {
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print("Floor: ");
  lcd.setCursor(0, 1);
  lcd.print(targetFloor);
}

int getDistance() {
  unsigned int duration = sonar.ping_median(5);
  return NewPing::convert_cm(duration);
}

You made this a local variable, so every time loop() runs, it gets created and set to zero, and at the end of loop(), it's value is lost. You need it to be persistent from one call of loop() to the next, so that it remembers which button was pressed. Either make it a global variable or make it static so is value is not lost.

Hello jordanangelo

Welcome to the worldbest Arduino forum ever.

Check and play with this mods from you sketch.
I moved the variable "motorCommand" to the logical position to reset it there. Check the comments. The mod is not tested.

#include <NewPing.h>
#include <LiquidCrystal_I2C.h>
#include <Wire.h>
const int enA = 10;
const int in1 = 2;
const int in2 = 3;
const int trigPin = A0;
const int echoPin = A1;
NewPing sonar(trigPin, echoPin, 40);
const int floor1Pin = 7;
const int floor2Pin = 8;
const int floor3Pin = 12;
// make global
int motorCommand;
LiquidCrystal_I2C lcd(0x27, 16, 2);
void setup()
{
  lcd.init();
  lcd.backlight();
  Serial.begin(9600);
  pinMode(enA, OUTPUT);
  pinMode(in1, OUTPUT);
  pinMode(in2, OUTPUT);
  pinMode(trigPin, OUTPUT);
  pinMode(echoPin, INPUT);
  pinMode(floor1Pin, INPUT_PULLUP);
  pinMode(floor2Pin, INPUT_PULLUP);
  pinMode(floor3Pin, INPUT_PULLUP);
  digitalWrite(in1, LOW);
  digitalWrite(in2, LOW);
  analogWrite(enA, 0);
  lcd.setCursor(0, 0);
  lcd.print("Current Floor: 1");
}
void loop()
{
  // don´t reset the command here
  //  int motorCommand = 0;
  if (digitalRead(floor1Pin) == LOW)
  {
    motorCommand = 1;
  }
  else if (digitalRead(floor2Pin) == LOW)
  {
    motorCommand = 2;
  }
  else if (digitalRead(floor3Pin) == LOW)
  {
    motorCommand = 3;
  }
  int distance = getDistance();
  switch (motorCommand)
  {
    case 1:
      controlMotor(distance, 2, 3);
      displayFloor(1);
      break;
    case 2:
      controlMotor(distance, 20, 21);
      displayFloor(2);
      break;
    case 3:
      controlMotor(distance, 38, 39);
      displayFloor(3);
      break;
    default:
      motorStop();
      break;
  }
}
void controlMotor(int distance, int lowerThreshold, int upperThreshold)
{
  if (distance >= lowerThreshold && distance <= upperThreshold)
  {
    motorStop();
    // reset motor command here
    motorCommand = 0;
  } else if (distance > upperThreshold)
  {
    moveDown();
  } else if (distance < lowerThreshold)
  {
    moveUp();
  }
  Serial.println("Ultrasonic reading: " + String(distance) + " cm");
}
void moveUp()
{
  digitalWrite(in1, HIGH);
  digitalWrite(in2, LOW);
  analogWrite(enA, 255);
}
void moveDown()
{
  digitalWrite(in1, LOW);
  digitalWrite(in2, HIGH);
  analogWrite(enA, 255);
}
void motorStop()
{
  digitalWrite(in1, LOW);
  digitalWrite(in2, LOW);
  analogWrite(enA, 0);
}
void displayFloor(int targetFloor)
{
  lcd.clear();
  lcd.setCursor(0, 0);
  lcd.print("Floor: ");
  lcd.setCursor(0, 1);
  lcd.print(targetFloor);
}
int getDistance()
{
  unsigned int duration = sonar.ping_median(5);
  return NewPing::convert_cm(duration);
}

Have a nice day and enjoy coding in C++.

Missing ;

1 Like

Thank you for pointing this out.
I have made a correction.

1 Like

While your code might ultimately work, there are more robust and flexible ways you might like to explore…

The first, is to forget it’s an elevator, and think of a motor moving the load between multiple different known positions.
Using that, having a ‘current’ position, and a ‘target’ position makes things easier,.. it either has to move forward or backwards to reach the target - it can do that without you pressing buttons or doing anything.

The next step is profiling acceleration and deceleration between the end points. Depending on the motor and drivers you’re using that can be quite straightforward.

Finally a ‘state machine’ to sequence or interrupt the individual operations…. door closed, movement, door open etc.
These can be controlled by passenger request buttons, limit switches and so-on to reproduce a realistic elevator experience.

Work on one piece at a time in a modular/functional way - then you can stitch the modules together for the final project.

Hello @jordanangelo - I put @paulpaulson edit of your sketch into a simulation that you can freely experiment with. The BUTTONs are the floors calling. The LEDs are the indication of IN1 and IN2 (motors) being called.

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