A variable seems to be fooling me...

Hi,

Yesterday I started a thread with a different problem for this topic. As I changed the code significantly, I am starting a new thread and giving some more accurate info (thanks Tom).

I am trying to build a roll-up curtain automated system. It consists of 2 switched (up and down), 2 hall effect switches for the end of stroke (will be embedded into the fabric of the curtain), Arduino One, THIS motor shield and THIS motor.

The configuration of the switches is with a pull-up resistor (real, not the one in Arduino, learning!) and the hall effect sensors are normally closed, thus they always give a HIGH result except when they are activated with a magnet.

My logic is: (IF) you press up or down button AND the end of stroke is not active, move the motor (WHILE) none of the following events happens: the end of stroke is reached or any of the buttons is switched, in which case, stop the motor.

This means, I want it to stop at maximum strokes automatically or press any button to keep it where I want manually.

The code is below (there are some Serial.println to monitor where the code is running) and this is what happens:

Push any switch - motor starts correctly
Placing a magnet on the hall sensor - stops the motor correctly

However: pushing the switch when the motor is moving - stops the motor momentarily (actually, the WHILE loop stops) but it restarts to move. If the button I push is the opposite as the one that started the movement, the motor turns the opposite way.

This tells me that, even though at the beginning of the loop there is a reading of the switch status (and it should be HIGH as the switch is not pressed), the value of the button remains as LOW (as if the switch was pressed).

I hope I made myself clear :confused:

If any of you could show me the way to the light I will be more than grateful.

Here is the CODE:

/* ROLLER CURTAIN AUTOMATIC STOP SYSTEM WITH MANUAL CONTROL
    THIS PROGRAM INTENDS TO OPERATE A MOTOR TO ROLL UP AND DOWN A FABRIC CURTAIN
    THE SET UP IS:
    ARDUINO UNO
    MOTOR SHIELD HiLetgo L298P DC Motor Drive Module
    12VDC GEARED MOTOR
    2X HALL EFFECT SENSORS AS END OF STROKE SWITCHES - NORMALLY CLOSED (NORMAL STATE HIGH), ACTIVATED WITH A MAGNET EMBEDDED INTO THE CURTAIN
    2X MECHANICAL SWITCHES IN PULL-UP RESISTOR CONFIGURATION (NORMAL STATE HIGH)

    OPERATION:
    PUSH UP OR DOWN BUTTON TO START ROLLING UP OR DOWN THE CURTAIN, UNLESS THE END OF STROKE OF EACH SIDE IS ACTIVE (MEANING THE CURTAIN IS COMPLETELY OPEN OR CLOSED). IN SUCH CASE NOTHING SHOULD HAPPEN
    THE CURTAIN SHOULD STOP IF EITHER THE USER PUSHES ANY BUTTON (UP OR DOWN) OR THE END OF STROKE IS REACHED.
*/


//PIN SELECTION - (SOME PINS NOT AVAILABLE DUE TO THE SHIELD)
int pinUp = 3;
int pinDown = 5;
int eosUp = 6;
int eosDown = 7;

//VARIABLES ASSIGNATION
int buttonUp = 0;
int buttonDown = 0;
int endOfStrokeUp = 0;
int endOfStrokeDown = 0;

//MOTOR PINS SELECTION (SHIELD)
int pinMotor = 10;                //ANALOG ---- SPEED OF THE MOTOR
int pinDirection = 12;            //DIGITAL: HIGH AND LOW DETERMINE DIRECTION


void setup() {

  pinMode(pinUp, INPUT);
  pinMode(pinDown, INPUT);
  pinMode(eosUp, INPUT);
  pinMode(eosDown, INPUT);
  pinMode(pinMotor, OUTPUT);
  pinMode(pinDirection, OUTPUT);

  Serial.begin(9600);

}

void loop() {

Serial.println("beginning of the loop");

  buttonUp = digitalRead(pinUp);  //READ THE INPUT VARIABLES
  buttonDown = digitalRead(pinDown);
  endOfStrokeUp = digitalRead(eosUp);
  endOfStrokeDown = digitalRead(eosDown);


  if (buttonUp == LOW && endOfStrokeUp == HIGH)       // IF UP BUTTON IS PRESSED AND UPPER END OF STROKE IS NOT ACTIVATED
  {
    Serial.println("Entered IF up button activated");
    
    delay(500);                                       //DELAY TO LET TIME FOR THE BUTTON BE RELEASED
    buttonUp = HIGH;                                  //GIVE BACK THE VALUE "HIGH" TO THE VARIABLE TO ALLOW THE WHILE LOOP TO START


    while (endOfStrokeUp == HIGH && buttonUp == HIGH && buttonDown == HIGH) //WHILE THERE ARE NO BUTTONS PRESSED, THE END OF STROKE IS NOT ACTIVE
    {
      analogWrite(pinMotor, 255);                     // MOVE MOTOR MAX SPEED
      digitalWrite(pinDirection, HIGH);               // MOVE MOTOR UP DIRECTION
      
      Serial.println("Entered in While UP, motor up moving");


      buttonUp = digitalRead(pinUp);                  //READ VARIABLES INSIDE THE LOOP
      buttonDown = digitalRead(pinDown);
      endOfStrokeUp = digitalRead(eosUp);             //READ UPPER END OF LINE (NO NEED TO READ THE LOW BOTTOM END OF STROKE BECAUSE THE MOTOR IS MOVING THE CURTAIN UP)

    }
    
Serial.println("Exit while up, proceeding to stop motor from up movement");
    
    analogWrite(pinMotor, 255);                       //MAKE A REVERSE MOVEMENT TO BREAK AND STOP MOTOR
    digitalWrite(pinDirection, LOW);
    delay(100);
    analogWrite(pinMotor, 0);
    digitalWrite(pinDirection, LOW);                  //THIS LINE CAN POSSIBLY BE OMITTED: TRY BOTH WAYS

    Serial.println("Motor stopped");
  }

 
  if (buttonDown == LOW && endOfStrokeDown == HIGH)   //IF DOWN BUTTON IS PRESSED (PULLUP RESISTOR) AND UPPER END OF STROKE IS NOT ACTIVATED (HALL EFFECT SENSOR, N.C.)
  {
    Serial.println("Entered IF down button pushed");

    delay(500);                                       //DELAY TO LET TIME FOR THE BUTTON BE RELEASED

    buttonDown = HIGH;                                //GIVE BACK THE VALUE HIGH TO THE VARIABLE TO ALLOW THE WHILE LOOP TO START

    while (endOfStrokeDown == HIGH && buttonUp == HIGH && buttonDown == HIGH) //WHILE THERE ARE NO BUTTONS PRESSED, THE END OF STROKE IS NOT ACTIVE
    {

      analogWrite(pinMotor, 255);                     // MOVE MOTOR MAX. SPEED
      digitalWrite(pinDirection, LOW);                // MOVE MOTOR DOWN DIRECTION

Serial.println("Entered in While DOWN, motor up moving");

      buttonUp = digitalRead(pinUp);                  //READ VARIABLES INSIDE THE LOOP
      buttonDown = digitalRead(pinDown);
      endOfStrokeDown = digitalRead(eosDown);         //NO NEED TO READ THE LOW BOTTOM END OF STROKE BECAUSE THE MOTOR IS MOVING UP

Serial.println("Exit while down, proceeding to stop motor from down movement");

    }


    analogWrite(pinMotor, 255);                       //MAKE A REVERSE MOVEMENT TO BREAK AND STOP MOTOR
    digitalWrite(pinDirection, HIGH);
    delay(100);
    analogWrite(pinMotor, 0);
    digitalWrite(pinDirection, HIGH);
    
    Serial.println("Motor stopped from down movement");
  }


Serial.print("Loop finished");
}

What happens (analysis for the up button)

you press the button and release it again within 500ms
2)
you enter the while-loop and the motor starts moving
3)
you press the button again
4)
the while-loop will end and the motor will stop

next iteration of loop() will see that the button is still pressed and starts the process again

Little note: there is no need to call analogWrite or digitalWrite with the same value time after time in a while-loop.

The best way to solve this is to use a finite statemachine.

You have a few states that the system can be in; below the basics (for moving up) that need to be expanded
1)
waiting for button press
2)
moving up
3)
wait for button to be released
4)
waiting for up sensor or other button press
5)
stop motor
6)
wait for button to be released

Your loop basics will look like below; note that I started counting from 0, not 1

void loop()
{
  // read buttons and sensors
  ...
  ...

  switch (currentState)
  {
    case 0:
      if (buttonUp == LOW && endOfStrokeUp == HIGH)
      {
        currentState = 1;
      }
      break;
    case 1:
      // start the motor
      ...
      ...
      currentState  = 2;
      break;
    case 2:
      // wait for button to be released
      if (buttonUp == HIGH)
      {
        currentState = 3;
      }
    case 3:
      // wait for button to be pressed or endpoint to be reached
      if (buttonUp == LOW || endOfStrokeUp == LOW)
      {
        currentState = 4;
      }
      break;
    case 4:
      // stop motor
      ...
      ...
      currentState = 5;
      break;
    case 5:
      // wait for button to be released (if it was pressed)
      if (buttonUP == HIGH)
      {
        currentState = 0;
      }
  }
}

In the above, currentState will only change when certain conditions are met; which condition depends on the state that your in.

The first thing to do is to give the steps names; you can place the below near the top of your code

enum STATES
{
  WAIT_START,
  START_MOVE,
  WAIT_RELEASE_AFTER_START,
  WAIT_BUTTON_OR_ENDPOINT,
  STOP_MOTOR,
  WAIT_RELEASE,
};

STATES currentState = WAIT_START;

And next replace all the numbers in above loop() by the names (0 equals WAIT_START and so on).

If the button is pressed while in WAIT_START, you can set the motor values for direction and speed.

  switch (currentState)
  {
    case WAIT_START:
      if (buttonUP == LOW && endOfStrokeUp == HIGH)
      {
        speed = 255;
        direction = HIGH;
        currentState = START_MOVE;
      }
      if (buttonDOWN == LOW && endOfStrokeDown == HIGH)
      {
        speed = 255;
        direction = LOW;
        currentState = START_MOVE;
      }
      break;

And in START_MOVE you can make use of those variables.

It's just a framework. Hope that this gets you on the way.

OP's previous Thread.

...R

JoanPorquer:
If any of you could show me the way to the light I will be more than grateful.

I reckon life would be much simpler if you organize your program as a series of short single purpose functions. Then your code in loop() might be like this

void loop() {
   readButtons();
   readHallSensor();
   updateMotorState();
   operateMotor();
}

By having all your code in one function (loop() ) it is very hard to figure out what is happening. Have a look at Planning and Implementing a Program

The functions to read the buttons and the hall-sensor should just save the values to variables. The function to update the motor state should look at those variables and update another variable that lets the motor code know what to do. The operate motor function should just check the variable that identifies what it should do. This way each part can be tested separately and, for example, the operate motor function has no direct link with the button code.

If you want a responsive program you should not use WHILE as it blocks until it completes. Use IF and allow loop() to do the repetition. That will also allow you to read the buttons in one place rather than in several. Also have a look at how the code is organized in Several Things at a Time

Note how each function runs very briefly and returns to loop() so the next one can be called. None of the functions tries to complete a task in one call. And there may be dozens of calls to a function before it is actually time for it to do anything.

...R

Robin2, sterredje,

Thank you so much both, I have seen the light. I understand the purpose of your comments, making small codes or functions apart so the main loop calls them, right?

I will try these approaches and get back to you with the result (although I know you already know that it will work) :wink:

Thank you again!

Joan.

sterretje:
You have a few states that the system can be in; below the basics (for moving up) that need to be expanded
1)
waiting for button press
2)
moving up
3)
wait for button to be released

That is one way - I'd use state change detection on the button (see example in the IDE), so reacting to the button becoming pressed, not being pressed or waiting for it to be released again (that sounds like a recipe for trouble, if the user decides to not release the button for whatever reason).

Debouncing the button is important as well: ignore button presses for 10-50 ms after the first has been detected, as that's simply the button bouncing.