stopping motor by time (mills) or switch

Hi
i wrote this small code to control a motor to work by pressing once the a button and stopping it also by an other button or stop after some time

the problem is that after i push the button.. motor (led by the time) works without being able to stop from time or by pushing stop button

something i havent understand about ''while'' command i guess

////////// Motor activation and stopping it my switch or time ///////////



unsigned long previousMillisMotor=0;



int intervalMotor = 10000;  // maximum working time 



const byte  buttonPin = 2; 
const byte  motorPin = 5; 
const byte  stopingPin = 12;
int buttonState = 0;
int buttonStopState = 0;

void setup() {
 

  pinMode(buttonPin, INPUT_PULLUP );
  pinMode(stopingPin, INPUT_PULLUP );
  pinMode(motorPin, OUTPUT );
  Serial.begin(9600);

}

void loop() {


 buttonState = digitalRead(buttonPin);


if (buttonState == LOW) {
  
  
  motorrun();

}

else { digitalWrite(motorPin, LOW);
 Serial.println(buttonState); 
  }




}




void motorrun() {


   unsigned long currentMillis = millis();
   buttonStopState = digitalRead(stopingPin);
  



  digitalWrite(motorPin, HIGH);
  Serial.println(buttonState);
  if (buttonStopState == LOW) {
      digitalWrite(motorPin, LOW);
  }
     while (buttonStopState == HIGH) {
  
     if ((unsigned long)(currentMillis - previousMillisMotor) >= intervalMotor) {
     digitalWrite(motorPin, LOW);
     Serial.println(" time limit ");
    
      previousMillisMotor = currentMillis;    // save current time to motor previousMillis
      
     
      }
      
 
   }


}

thanks in advance

You program will only exit the WHILE loop when buttonStopState is LOW, but nowhere in the WHILE loop does buttonStopState change because you don't read it after that first time.

Try this version:

unsigned long previousMillisMotor;
unsigned long switchMillis;
unsigned long intervalMotor = 10000;  // maximum working time

bool motorFlag = false;

//const byte  onSwitch    = 8;
//const byte  offSwitch   = 9;
//const byte  motorPin    = 13;

const byte  onSwitch   = 2;
const byte  offSwitch  = 12;
const byte  motorPin   = 5;

byte onSwitchState;
byte offSwitchState;

byte lastOnSwitchState   = HIGH;
byte lastOffSwitchState  = HIGH;

//*************************************************************************
void setup()
{
  Serial.begin(9600);
  
  pinMode(onSwitch,  INPUT_PULLUP);
  pinMode(offSwitch, INPUT_PULLUP);
  
  pinMode(motorPin,  OUTPUT);
  
} //END of                  s e t u p ( )

//*************************************************************************
void loop()
{
  //**********************************
  if(motorFlag == true && millis() - previousMillisMotor >= intervalMotor)
  {
    //motor off
    digitalWrite(motorPin, LOW);    
    motorFlag = false;
  }

  //**********************************
  //check switches every 50ms
  if (millis() - switchMillis >= 50)
  {
    switchMillis = millis();
    checkSwitches();
  }


} //END of                      l o o p ( )


//*************************************************************************
void checkSwitches()
{
  //**********************************
  onSwitchState = digitalRead(onSwitch);

  if (motorFlag == false && lastOnSwitchState != onSwitchState)
  {
    lastOnSwitchState = onSwitchState;
    
    //is the switch pushed?
    if (onSwitchState == LOW)
    {
      //motor on
      digitalWrite(motorPin, HIGH);
      
      motorFlag = true;
      //initalize the timer
      previousMillisMotor = millis();
    }
  }

  //**********************************
  offSwitchState = digitalRead(offSwitch);
  
  if (motorFlag == true && lastOffSwitchState != offSwitchState)
  {
    lastOffSwitchState = offSwitchState;
    
    //is the switch pushed?
    if (offSwitchState == LOW)
    {
      //motor off
      digitalWrite(motorPin, LOW);
      motorFlag = false;
    }

  }

} //END of          c h e c k S w i t c h e s ( )

Edit:
Updated code

Thank you Larryd for the code… i can confirm that is working fine

But still want to know what i have written wrong (have understand wrong) in my code…

u see i want to implement it later to a bigger project i am making that has more buttons and functions with the ‘‘while’’ command

i change a little the code but still cant go out of while loop even the conditions are not the same (time reached or the stop button pressed )

////////// Motor activation and stopping it my switch or time ///////////



unsigned long previousMillisMotor=0;
int intervalMotor = 10000;  // maximum working time 


const byte  buttonPin = 2; 
const byte  motorPin = 5; 
const byte  stopingPin = 12;
int buttonState = 0;
int buttonStopState = 0;

void setup() {
 

  pinMode(buttonPin, INPUT_PULLUP );
  pinMode(stopingPin, INPUT_PULLUP );
  pinMode(motorPin, OUTPUT );
  Serial.begin(9600);

}

void loop() {


 buttonState = digitalRead(buttonPin);
 buttonStopState = digitalRead(stopingPin);

if (buttonState == LOW) {
  
  
  motorrun();

}

else { digitalWrite(motorPin, LOW);

 // Just to monitor if push buttons works ///////
  // /* 
     
    Serial.print(buttonState); 
    Serial.println(" On time ");
   
    Serial.println(" stop button ");
    Serial.print(buttonStopState); 
    
   // */
  }




}




void motorrun() {


   unsigned long currentMillis = millis();
   buttonStopState = digitalRead(stopingPin);
  
   


while (buttonStopState = HIGH && (currentMillis - previousMillisMotor <= intervalMotor))
  { 
    buttonStopState = digitalRead(stopingPin);
    
    digitalWrite(motorPin, HIGH);

    previousMillisMotor = currentMillis;  // Remember the time
    
//// just to monitor if push buttons works //////////////
   // /*
    Serial.println(" On time ");
    Serial.print(buttonState); 
    
    Serial.println(" stop button ");
    Serial.print(buttonStopState); 

   //*/
    
    
  
   if (buttonStopState = LOW || (currentMillis - previousMillisMotor > intervalMotor)) {
      
      digitalWrite(motorPin, LOW);
      previousMillisMotor = currentMillis; // Remember the time
   
    //// just to monitor if push buttons works //////////////
   // /*
    Serial.println(" On time ");
    Serial.print(buttonState); 
    
    Serial.println(" stop button ");
    Serial.print(buttonStopState); 

   // */

   break;
 
      } 
 
 
   }

}

from the time that

(currentMillis - previousMillisMotor <= intervalMotor) after passing 10sec is not true anymore, shouldnt stopped while loop ?

the same for the

buttonStopState = HIGH as when i press the button goes low , shouldnt exit ‘‘while’’ loop ?

I seldom use ‘while/do’ as they can be blocking.
Look at the code posted in reply#2 to see one way of avoiding using ‘while’.

You should get into the habit of writing sketches that run at full out speed.
This creates code that is responsive and, IMO, is less prone to weird hiccups.

‘while’ can be effectively used in situations where it’s code segment can run at full speed, i.e. NOT waiting for user intervention, and where it executes quickly, such as scanning an array.

In the future, format your code using and clear up white space weirdness.

.

(currentMillis - previousMillisMotor <= intervalMotor)
after passing 10sec is not true anymore, shouldnt stopped while loop ?

Yes but what happens 72 days later when millis() overflows? :wink:

Always use a enable flag in a combination like this.
if( flag == true && currentMillis - previousMillisMotor <= intervalMotor)
{
flag = false;
. . .
}

.

Thanks one more time.. i will follow your suggestion :slight_smile:

One more question... when we set the '' bool motorFlag = false;'' at the beginning.. when we run one function (example void runmotor() { ) and we change the motorflag state, after the end of the function that the program will return to '' void loop '' the flag will have the state that we declare in the last function ??

does the program stores the changes of the the flag state and we can use it in loop or other void functions ?

maybe it is a silly question but as a newbie in code will help me clear some things

thanks in advance

caslor:
Thanks one more time… i will follow your suggestion :slight_smile:

One more question… when we set the ‘’ bool motorFlag = false;’’ at the beginning… when we run one function (example void runmotor() { ) and we change the motorflag state, after the end of the function that the program will return to ‘’ void loop ‘’ the flag will have the state that we declare in the last function ??
Putting: bool motorFlag = false; above the setup function part of the sketch, makes motorFlag a global variable i.e. available to the whole sketch.

does the program stores the changes of the the flag state and we can use it in loop or other void functions ?
Global variables are available to other functions.

maybe it is a silly question but as a newbie in code will help me clear some things
A silly question is one that is not asked

thanks in advance

Putting: bool motorFlag = false; above the setup function part of the sketch, makes motorFlag a global variable i.e. available to the whole sketch.

Global variables are available to other functions.

ok one more last question based to the above..

so every Variable that is setup to a function is available only to this function .. right?

what is the case of the void setup ? the variables there ? , are only availble for this function only ?

You should fully understand what ‘scope’ is all about.
Read :
https://www.arduino.cc/en/Reference/Scope

int gPWMval;  // any function will see this variable

void setup()
{
  // ...
}

void loop()
{
  int i;    // "i" is only "visible" inside of "loop"
  float f;  // "f" is only "visible" inside of "loop"
  // ...

  for (int j = 0; j <100; j++){
  // variable j can only be accessed inside the for-loop brackets
  }

}

.

I tried to add some elements in the code (such a contact switch that has NC contacts )

But i am Afraid i missing something behind the logic of C

unsigned long previousMillisMotor;
unsigned long switchMillis;
unsigned long intervalMotor = 10000;  // maximum working time 10 sec


bool motorFlag = false;


const byte  onSwitch   = 2;
const byte  offSwitch  = 12;
const byte  motorPin   = 5;
const byte  contacts   = 13;

byte onSwitchState;
byte offSwitchState;
byte contactsState;   // Normaly closed 

byte lastOnSwitchState   = HIGH;
byte lastOffSwitchState  = HIGH;


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

  pinMode(onSwitch,  INPUT_PULLUP);
  pinMode(offSwitch, INPUT_PULLUP);
  pinMode(contacts, INPUT_PULLUP);

  pinMode(motorPin,  OUTPUT);

} //END of                  s e t u p ( )

//*************************************************************************
void loop()
{

  //**********************************
  
  if(motorFlag == true  && millis() - previousMillisMotor >= intervalMotor)
  {
    //motor off
    digitalWrite(motorPin, LOW); 
    Serial.println(" Error Time limit " );  
    motorFlag = false;
  }




  //**********************************
  //check switches every 50ms
  if (millis() - switchMillis >= 50)
  {
    switchMillis = millis();
    checkSwitches();
  }


} //END of                      l o o p ( )


//*************************************************************************
void checkSwitches()
{

  //**********************************
  onSwitchState = digitalRead(onSwitch);
  contactsState = digitalRead(contacts);

  if (motorFlag == false && contactsState == LOW  &&  lastOnSwitchState != onSwitchState)
  {
    lastOnSwitchState = onSwitchState;

    //is the switch pushed?
    if (onSwitchState == LOW)
    {
      //motor on
      digitalWrite(motorPin, HIGH);
     Serial.println(" Motor Running " );
      motorFlag = true;
 
      //is the contact closed?
      if (contactsState == HIGH && motorFlag == true) {  
      digitalWrite(motorPin, LOW);
      Serial.println(" Motor is off kleidaria " );
      motorFlag = false;
      }
     previousMillisMotor = millis();

  /*  //is the contact closed?
      if (contactsState == HIGH && motorFlag == true) {  
      digitalWrite(motorPin, LOW);
      Serial.println(" Motor is off kleidaria " );
      motorFlag = false;
      }

     */

      }
   }

  //**********************************
 offSwitchState = digitalRead(offSwitch);
 
  if (motorFlag == true  && lastOffSwitchState != offSwitchState)
  {
    lastOffSwitchState = offSwitchState;
   
    //is the switch pushed?
    if (offSwitchState == LOW)
    {
      //motor off
      digitalWrite(motorPin, LOW);
      Serial.println(" Motor is off " );
      motorFlag = false;
    }

   }

 //****************************************************


} //END of          c h e c k S w i t c h e s ( )

i thought that when the code reads this

 if (motorFlag == false && contactsState == LOW  &&  lastOnSwitchState != onSwitchState)
  {
    lastOnSwitchState = onSwitchState;

    //is the switch pushed?
    if (onSwitchState == LOW)
    {
      //motor on
      digitalWrite(motorPin, HIGH);
     Serial.println(" Motor Running " );
      motorFlag = true;
 
      //is the contact closed?
      if (contactsState == HIGH && motorFlag == true) {  
      digitalWrite(motorPin, LOW);
      Serial.println(" Motor is off kleidaria " );
      motorFlag = false;
      }
     previousMillisMotor = millis();

and have opend the switch(contacts) the motor will stopped

but mine is still working (can stop it from stop switch or stopping from time limit )

My question is :

there is something wrong with the code i added? or in order to work have i to use interrupted pin ?

How can contactsState == LOW and contactsState == HIGH at the same time?

2017-09-23_18-53-19.jpg

or in order to work have i to use interrupted pin ?

Never use interrupts for slow events.

BTW:

  • Suggest you place { and } on lines by themselves.
  • Use to format your code

Thanks again (Now i understand the tip about + :slight_smile: )

Here is the code i changed and work fine :

unsigned long previousMillisMotor;
unsigned long switchMillis;
unsigned long intervalMotor = 10000;  // maximum working time 10 sec


bool motorFlag = false;


const byte  onSwitch   = 2;
const byte  offSwitch  = 12;
const byte  motorPin   = 5;
const byte  contacts   = 13;

byte onSwitchState;
byte offSwitchState;
byte contactsState;   // Normaly closed

byte lastOnSwitchState   = HIGH;
byte lastOffSwitchState  = HIGH;


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

  pinMode(onSwitch,  INPUT_PULLUP);
  pinMode(offSwitch, INPUT_PULLUP);
  pinMode(contacts, INPUT_PULLUP);

  pinMode(motorPin,  OUTPUT);

} //END of                  s e t u p ( )

//*************************************************************************
void loop()
{

  //**********************************

  if (motorFlag == true  && millis() - previousMillisMotor >= intervalMotor)
  {
    //motor off
    digitalWrite(motorPin, LOW);
    Serial.println(" Error Time limit " );
    motorFlag = false;
  }




  //**********************************
  //check switches every 50ms
  if (millis() - switchMillis >= 50)
  {
    switchMillis = millis();
    checkSwitches();
  }


} //END of                      l o o p ( )


//*************************************************************************
void checkSwitches()
{

  //**********************************
  onSwitchState = digitalRead(onSwitch);
  contactsState = digitalRead(contacts);

  if (motorFlag == false && contactsState == LOW  &&  lastOnSwitchState != onSwitchState)
  {
    lastOnSwitchState = onSwitchState;

    //is the switch pushed?
    if (onSwitchState == LOW)
    {
      //motor on
      digitalWrite(motorPin, HIGH);
      Serial.println(" Motor Running " );
      motorFlag = true;


      previousMillisMotor = millis();

      /

    }
  }

  //**********************************
  offSwitchState = digitalRead(offSwitch);

  if (motorFlag == true  && lastOffSwitchState != offSwitchState)
  {
    lastOffSwitchState = offSwitchState;

    //is the switch pushed?
    if (offSwitchState == LOW)
    {
      //motor off
      digitalWrite(motorPin, LOW);
      Serial.println(" Motor is off " );
      motorFlag = false;
    }

  }



  //is the contact closed?
  if (contactsState == HIGH && motorFlag == true) {
    digitalWrite(motorPin, LOW);
    Serial.println(" Motor is off kleidaria " );
    motorFlag = false;
  }


  //****************************************************


} //END of          c h e c k S w i t c h e s ( )

The problem was from the beginning a wrong connection so i had tried lot of combination in code
(i had one micro-switch that has on/grnd/off positions and i was connecting it to be always NC and by press going NO - tested it with my multimeter , but seem not working well with this setup… when i connected it be at NO in stand by and going to NC by pressing it , the above code worked fine )

previousMillisMotor = millis();

/

Why the stray / ?

unsigned long previousMillisMotor;
unsigned long switchMillis;
unsigned long intervalMotor = 10000UL;  // maximum working time 10 sec

bool motorFlag = false;  //false means Motor is OFF

//const byte  onSwitch    = 8;  //N.O.
//const byte  offSwitch   = 9;  //N.O.
//const byte  contacts    = 10; //N.C.
//const byte  motorPin    = 13; //LOW = ON

const byte  onSwitch  = 2;  //N.O.
const byte  offSwitch = 12; //N.O.
const byte  motorPin  = 5;  //N.C.
const byte  contacts  = 13; //LOW = ON

byte onSwitchState;
byte offSwitchState;
byte contactsState;   // Normaly closed

byte lastOnSwitchState   = HIGH;
byte lastOffSwitchState  = HIGH;

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

  pinMode(onSwitch,  INPUT_PULLUP);
  pinMode(offSwitch, INPUT_PULLUP);
  pinMode(contacts,  INPUT_PULLUP);

  pinMode(motorPin,  OUTPUT);

} //END of                     s e t u p ( )

//*************************************************************************
void loop()
{
  //**********************************
  if (motorFlag == true  && millis() - previousMillisMotor >= intervalMotor)
  {
    //motor off
    digitalWrite(motorPin, LOW);
    Serial.println(" Error Time limit " );
    motorFlag = false;
  }

  //**********************************
  //check switches every 50ms
  if (millis() - switchMillis >= 50)
  {
    switchMillis = millis();
    checkSwitches();
  }

} //END of                      l o o p ( )

//*************************************************************************
void checkSwitches()
{
  //**********************************
  onSwitchState = digitalRead(onSwitch);
  contactsState = digitalRead(contacts);

  if (motorFlag == false && contactsState == LOW  &&  lastOnSwitchState != onSwitchState)
  {
    lastOnSwitchState = onSwitchState;

    //is the switch pushed?
    if (onSwitchState == LOW)
    {
      //motor on
      digitalWrite(motorPin, HIGH);
      Serial.println(" Motor Running " );
      motorFlag = true;
      previousMillisMotor = millis();
    }
  }

  //**********************************
  offSwitchState = digitalRead(offSwitch);

  if (motorFlag == true  && lastOffSwitchState != offSwitchState)
  {
    lastOffSwitchState = offSwitchState;

    //is the switch pushed?
    if (offSwitchState == LOW)
    {
      //motor off
      digitalWrite(motorPin, LOW);
      Serial.println(" Motor is off " );
      motorFlag = false;
    }
  }

  //**********************************
  //is the contact closed?
  if (contactsState == HIGH && motorFlag == true)
  {
    digitalWrite(motorPin, LOW);
    Serial.println(" Motor is off kleidaria " );
    motorFlag = false;
  }

} //END of           c h e c k S w i t c h e s ( )

//*************************************************************************

.

larryd:
previousMillisMotor = millis();

/

Why the stray / ?

.

I had one comment there and delete it... hadnt notice it when i copied the code to paste it here..

later i compiled the code it pointed me the mistake and remove it :slight_smile: