Need little help with this code

Projects Programming
1

Hi
i wrote this code

the problem is that before i press the button the code loops as expected in ''loop function''

after i press it goes to the function i want and then after some time goes to an other function as was expected

the problem is that after i break from the last ''while loop'' goes again to ''loop funcrion'' as i wanted but doesnt loop there... goes again to the function as if i had press button again

i used and flags for button to solve that but something wrong with my code i suppose (maybe dont use them the right way)

here is the code :

byte lock_check = 13; // input pin to check lock-doors
byte contact_check = 14; // input pin to check contacts-doors
byte stop_check = 9; // input pin to check stop
byte end_limits = 2; // input pin to check end limits
byte relayLeft = 10; //the relay that drives left the car
byte relayRight = 11;//the relay that drives right the car
byte slowDownVVF = 12; // Pin to drive vvf voltage via transistor (slow speed)
byte electromagnet = 15; //the relay that drives up the electromagnet of doorslocks
byte call_button = 3; //pin to call to floor test for timelimits  test
byte rstMvButton = 4;  // reset time error in movement
unsigned long intervalMotor = 3000;  // maximum working time 
unsigned long previousMillisMotor;
bool callflag = false;

void setup() {
  Serial.begin(9600);

  // initialize the digital pins inputs.

  pinMode(contact_check, INPUT_PULLUP);
  pinMode(lock_check, INPUT_PULLUP);
  pinMode(stop_check, INPUT_PULLUP);
  pinMode(end_limits, INPUT_PULLUP);
  pinMode(call_button, INPUT_PULLUP);
  pinMode(rstMvButton, INPUT_PULLUP);

  // initialize the digital pins outputs.
  pinMode(relayRight, OUTPUT);
  pinMode(relayLeft, OUTPUT);
  pinMode(slowDownVVF, OUTPUT);
  pinMode(electromagnet, OUTPUT);

}

void loop() {


  boolean contacts;
  boolean doorLocks;
  boolean carStop;
  boolean limitsStop;
  boolean calling;


  contacts = digitalRead(contact_check);
  doorLocks = digitalRead(lock_check);
  carStop = digitalRead(stop_check);
  limitsStop = digitalRead(end_limits);
  calling = digitalRead(call_button);

  if (calling == false && callflag == false) {

    if (contacts == false && carStop == false && limitsStop == false) {
      callflag = true;
      Run_Test();
    }
  }
  if (contacts == false && carStop == false && limitsStop == false) {

    Serial.println(" Everything ok - Standby ");
  }

  if (carStop == true) {

    Serial.println(" Reads stop ");
  }

  if (contacts == true || carStop == true || limitsStop == true) {
    digitalWrite(relayRight, LOW); //keep the relay for right motion inactive
    digitalWrite(relayLeft, LOW); //keep the relay for left motion inactive
    digitalWrite(slowDownVVF, LOW); //keep the VVF running with high speed quite
    digitalWrite(electromagnet, LOW); //keep the relay for electromagnet inactive
  }

  if (carStop == true || limitsStop == true ) {
    stop_car();
  }

}

void Run_Test() {

  

  boolean doorLocks;
  boolean carStop;
  boolean limitsStop;
  boolean contacts;

  previousMillisMotor = millis();
  doorLocks = digitalRead(lock_check);
  carStop = digitalRead(stop_check);
  limitsStop = digitalRead(end_limits);
  contacts = digitalRead(contact_check);
  callflag = true;

  if (contacts == false  && carStop == false && limitsStop == false ) {
    do
    {

      callflag = false;
      doorLocks = digitalRead(lock_check);
      carStop = digitalRead(stop_check);
      limitsStop = digitalRead(end_limits);
      contacts = digitalRead(contact_check);
      digitalWrite(relayRight, HIGH);



      Serial.println(" Going to position ");



      if (millis() - previousMillisMotor >= intervalMotor)
      {

        timeError();

      }



    }

    while ( contacts == false && carStop == false && limitsStop == false);




  }


}




void stop_car() {

  boolean carStop;
  boolean limitsStop;
  carStop = digitalRead(stop_check);
  limitsStop = digitalRead(end_limits);


  while (carStop == true ) {

    boolean carStop;
    carStop = digitalRead(stop_check);
    digitalWrite(relayRight, LOW); //keep the relay for right motion inactive
    digitalWrite(relayLeft, LOW); //keep the relay for left motion inactive
    digitalWrite(slowDownVVF, LOW); //keep the VVF running with high speed quite
    digitalWrite(electromagnet, LOW); //keep the relay for electromagnet inactive
    Serial.println("Stop is Active");
    if (carStop == !true ) {
      break;
    }

  }

  while (limitsStop == true ) {

    boolean limitsStop;
    limitsStop = digitalRead(end_limits);
    digitalWrite(relayRight, LOW); //keep the relay for right motion inactive
    digitalWrite(relayLeft, LOW); //keep the relay for left motion inactive
    digitalWrite(slowDownVVF, LOW); //keep the VVF running with high speed quite
    digitalWrite(electromagnet, LOW); //keep the relay for electromagnet inactive
    Serial.println("end limits are on");
    if (limitsStop == !true ) {
      break;
    }

  }

}


void timeError()  {


  boolean resetMove;
  resetMove = digitalRead(rstMvButton);
  previousMillisMotor = millis();

  do {

    resetMove = digitalRead(rstMvButton);
    Serial.println("To Late - Time Limit");

    if (resetMove == false ) {
      Serial.println("Reads resetbutton");

      break;
    }

  } while (resetMove == true);

  loop();
}

thanks in advance

2

It's so much easier to read problem statements and provide assistance when they're written in English sentences with proper capitalization and punctuation.

3

Hi,
What is your code for?
What is it supposed to do?
What does it do?

Thanks.. Tom... :slight_smile:

4

The code is to control a motor

it is supposed to do :

void loop :

when it starts just stay in standby mode (void loop) and checks if ther is a press in call button and in the same time checks safety inputs

void stop_car :
while the program is in ''Void loop'' function if there is a safety off (stop and limit switch) then calls the ''void stop_car()'' function where it stays in loop until the safeties are on again . if the stop safeties are On again then returns to ''void loop'' function

also as it is in ''void loop'' checks some other safeties like locks and contacts and if they are off then sends comand all the relay of the motor be low

void Run_Test :
after the press of the button and if are all the safeties are ON goes to '' void Run_Test() '' function and stars to move the motor

if the motor is running and one of the above safeties goes off then the loop stops and goes to ''void loop'' where it checks safeties and do the actions i described in the ''void loop''

void timeError :
Also if the motor is running for some time

if that time is reached then stops all action and goes to ''void timeError()'' that stays in loop until a reset button is pressed

all the above are tested and worked... the only problem is that after the ''timeError'' when i reset it goes to ''void loop'' as it was supposed to go but doesnt stay in stand by mode... it goes again to ''
void Run_Test'' like if i had pressed the button again (but i dont)

it is like it keeps in memory the first call of the button.

that why i decide to use flags for button press but i didnt achieve to solve the problem even with them... something i have done wrong for sure...

that is why i need a more experience eye to notice the mistake

5

You have local variables in each of your functions with identical names - for example carstop. These will all be different from each other. That may be what you intend. However I think it would be less confusing if you gave them different names in the different functions.

However that is complicated by another factor. You are reading the same I/O pins in different places so you may or may not get different values. For example I see digitalRead(stop_check); in 5 different places.

if this was my program I would have one function to read and save the values from all the I/O pins once per iteration of loop() and I would use those values in all the code used in that iteration.

It would also be a good idea while debugging the program to print all the values when they have been collected.

When I am writing a program I prefer to cascade my IF tests rather than have several on one line as IMHO the cascade makes it easier to follow the logic and to insert print statements for debugging purposes.

...R

6

Robin2:
if this was my program I would have one function to read and save the values from all the I/O pins once per iteration of loop() and I would use those values in all the code used in that iteration.

Hi
thanks for pointing me this..

i move all variables like ''boolean'' at the first of sketch to be Global
Also i removed all the statements from the beging of each scope like the one you told me ''digitalRead(stop_check); ''

but i had to leave all the others inside the ''while'' and ''do while'' loops so can read if is any change in the conditions of the loop

of course the problem continues to ... when returns for ''void timeError()'' functions goes to ''void loop()'' and then continues in the '' void Run_Test() '' without press again the button to call this function

if the program returns from any other function it stays in ''void loop()'' normally...

Here is the code with the changes in variables :

byte lock_check = 13; // input pin to check lock-doors
byte contact_check = 14; // input pin to check contacts-doors
byte stop_check = 9; // input pin to check stop
byte end_limits = 2; // input pin to check end limits
byte relayLeft = 10; //the relay that drives left the car
byte relayRight = 11;//the relay that drives right the car
byte slowDownVVF = 12; // Pin to drive vvf voltage via transistor (slow speed)
byte electromagnet = 15; //the relay that drives up the electromagnet of doorslocks
byte call_button = 3; //pin to call to floor test for timelimits  test
byte rstMvButton = 4;  // reset time error in movement
unsigned long intervalMotor = 3000;  // maximum working time
unsigned long previousMillisMotor;

bool callflag = false;

boolean contacts;
boolean doorLocks;
boolean carStop;
boolean limitsStop;
boolean controlMaintain;
boolean carMaintain;
boolean calling;
boolean resetMove;

void setup() {


  Serial.begin(9600);

  // initialize the digital pins inputs.

  pinMode(contact_check, INPUT_PULLUP);
  pinMode(lock_check, INPUT_PULLUP);
  pinMode(stop_check, INPUT_PULLUP);
  pinMode(end_limits, INPUT_PULLUP);
  pinMode(call_button, INPUT_PULLUP);
  pinMode(rstMvButton, INPUT_PULLUP);

  // initialize the digital pins outputs.
  pinMode(relayRight, OUTPUT);
  pinMode(relayLeft, OUTPUT);
  pinMode(slowDownVVF, OUTPUT);
  pinMode(electromagnet, OUTPUT);

}

void loop() {


  contacts = digitalRead(contact_check);
  doorLocks = digitalRead(lock_check);
  carStop = digitalRead(stop_check);
  limitsStop = digitalRead(end_limits);
  calling = digitalRead(call_button);

  if (calling == false && callflag == false) {

    if (contacts == false && carStop == false && limitsStop == false) {
      callflag = true;
      Run_Test();
    }
  }
  if (contacts == false && carStop == false && limitsStop == false) {

    Serial.println(" Everything ok - Standby ");
  }

  if (carStop == true) {

    Serial.println(" Reads stop ");
  }

  if (contacts == true || carStop == true || limitsStop == true) {
    digitalWrite(relayRight, LOW); //keep the relay for right motion inactive
    digitalWrite(relayLeft, LOW); //keep the relay for left motion inactive
    digitalWrite(slowDownVVF, LOW); //keep the VVF running with high speed quite
    digitalWrite(electromagnet, LOW); //keep the relay for electromagnet inactive
  }

  if (carStop == true || limitsStop == true ) {
    stop_car();
  }

}

void Run_Test() {

  previousMillisMotor = millis();


  if (contacts == false  && carStop == false && limitsStop == false ) {
    do
    {

      callflag = false;
      doorLocks = digitalRead(lock_check);
      carStop = digitalRead(stop_check);
      limitsStop = digitalRead(end_limits);
      contacts = digitalRead(contact_check);
      digitalWrite(relayRight, HIGH);

      Serial.println(" Going to position ");

      if (millis() - previousMillisMotor >= intervalMotor)
      {

        timeError();

      }

    }

    while ( contacts == false && carStop == false && limitsStop == false);

  }

}


void stop_car() {

  while (carStop == true ) {

    boolean carStop;
    carStop = digitalRead(stop_check);
    digitalWrite(relayRight, LOW); //keep the relay for right motion inactive
    digitalWrite(relayLeft, LOW); //keep the relay for left motion inactive
    digitalWrite(slowDownVVF, LOW); //keep the VVF running with high speed quite
    digitalWrite(electromagnet, LOW); //keep the relay for electromagnet inactive
    Serial.println("Stop is Active");
    if (carStop == !true ) {
      break;
    }

  }

  while (limitsStop == true ) {

    boolean limitsStop;
    limitsStop = digitalRead(end_limits);
    digitalWrite(relayRight, LOW); //keep the relay for right motion inactive
    digitalWrite(relayLeft, LOW); //keep the relay for left motion inactive
    digitalWrite(slowDownVVF, LOW); //keep the VVF running with high speed quite
    digitalWrite(electromagnet, LOW); //keep the relay for electromagnet inactive
    Serial.println("end limits are on");
    if (limitsStop == !true ) {
      break;
    }

  }

}


void timeError()  {

  resetMove = digitalRead(rstMvButton);
  previousMillisMotor = millis();

  do {

    resetMove = digitalRead(rstMvButton);
    Serial.println("To Late - Time Limit");

    if (resetMove == false ) {
      Serial.println("Reads resetbutton");

      break;
    }

  } while (resetMove == true);

  loop();
}
7

You still have two extra copies of digitalRead(stop_check);

I strongly recommend that you don't use blocking WHILE loops in your code. Just use IF and allow loop() to do the iteration.

...R

8 
void timeError()  {
.
.
.
  loop();
}

This recursive call to loop() will cause a new set of local variables to be allocated on the stack. Eventually the stack will run out of space and your program will crash.

9

johnwasser:
This recursive call to loop() will cause a new set of local variables to be allocated on the stack. Eventually the stack will run out of space and your program will crash.

Well spotted. I completely missed that.

@caslor, I don't know if you put in that call to loop() in response to my comments. If so that was not what I meant. I just meant that you should let the functions terminate in the normal way - in which case loop() will be called again automatically.

...R

10

Robin2:
You still have two extra copies of digitalRead(stop_check);

Hi
i have the extra copies because every scope(function) have to check always the same thing..

Robin2:
I strongly recommend that you don't use blocking WHILE loops in your code. Just use IF and allow loop() to do the iteration.

...R

i dont know any other method beside 2 of them that can make code do what i want..

i want the motor start run until an other command be present

maybe if use the ''goto'' method i can do that... i will try it

11

Hi,
I would think this application is ideal as a state machine.

https://www.arduino.cc/en/Reference/SwitchCase

Tom... :slight_smile:

12

caslor:
i have the extra copies because every scope(function) have to check always the same thing..

I know you need to check but you don't need to do it the way you are doing it. The single check at the top of loop() is sufficient.

maybe if use the ''goto'' method i can do that... i will try it

Consider GOTO forbidden unless you are a sufficiently good programmer not to need help from anyone here. When you are that good you will realize that you don't need it.

Have look at the code in Several things at a time. Note how each function runs very briefly and returns to loop() so the next one can be called. And there may be dozens of calls to a function before it is actually time for it to do anything.

The tutorial Planning and Implementing a Program is an extended example of the same approach.

...R