Motor control with button and millis()

Hey,

I would like to control a motor with a button. If button0 is pressed for the first time, the motor should turn for 700 milliseconds and then turn off. If button0 is pressed for the second time, the motor should turn in the opposite direction (again for 700 milliseconds and then turn off and so on).

The counter, button and motor functions work like a charm, but I just can't find the problem with the millis() function...

#include <Bounce.h>

int motor1Pin = 26;   
int motor2Pin = 25;    
int speedPin = 22; 
int counter;

Bounce button0 = Bounce(13, 100);

long previousMillis = 0; 
long interval = 700;

void setup() {
  pinMode(motor1Pin, OUTPUT); 
  pinMode(motor2Pin, OUTPUT); 
  pinMode(speedPin, OUTPUT);
  digitalWrite(speedPin, HIGH);
  pinMode(13, INPUT_PULLUP);
}

void loop() {    
  button0.update();
  unsigned long currentMillis = millis();
 
  if(button0.fallingEdge()){ 
    counter ++;
  }  

  if(counter >= 2){
    counter = 0;
  }

  if(counter == 0){
    digitalWrite(motor1Pin, LOW);
    digitalWrite(motor2Pin, HIGH); 
    {
      if(currentMillis - previousMillis > interval) 
      {
        previousMillis = currentMillis;
        digitalWrite(motor1Pin, LOW); 
        digitalWrite(motor2Pin, LOW);
      }
    }
  }

  else if(counter == 1){
    digitalWrite(motor1Pin, HIGH);  
    digitalWrite(motor2Pin, LOW); 
    { 
      if(currentMillis - previousMillis > interval) 
      {
        previousMillis = currentMillis;
        digitalWrite(motor1Pin, LOW);  
        digitalWrite(motor2Pin, LOW);
      }
    }
  }
}

Thanks for your help 8)
Max

I don't understand the button stuff you are using - but you say that is OK.

What exactly does the present code do for the motor timing?

Perhaps this bit

if(currentMillis - previousMillis > interval) 
      {
        previousMillis = currentMillis;   //  <-----------not needed ?
        digitalWrite(motor1Pin, LOW); 
        digitalWrite(motor2Pin, LOW);
      }

should be outside any of the IF statements?

If you only want the timer to operate once you don't need the line I have marked.
But you do need to set previousMills when the button is pressed. And startMillis might be a better name.

...R

Hey Robin,

thank you so much for your answer! Yeah, the button stuff is basically just a counter :slight_smile:

I just tried your ideas and replaced the previousMillis with startMillis and moved 'the bit' outside of the if functions.
However the motor now just 'jumps' a little bit every time the button is pressed...

#include <Bounce.h>

int motor1Pin = 26;   
int motor2Pin = 25;    
int speedPin = 22; 
int counter;

Bounce button0 = Bounce(13, 100);

long startMillis = 0; 
long interval = 700;

void setup() {
  pinMode(motor1Pin, OUTPUT); 
  pinMode(motor2Pin, OUTPUT); 
  pinMode(speedPin, OUTPUT);
  digitalWrite(speedPin, HIGH);
  pinMode(13, INPUT_PULLUP);
}

void loop() {    
  button0.update();
  unsigned long currentMillis = millis();
  if(currentMillis - startMillis > interval) 
      {
        digitalWrite(motor1Pin, LOW); 
        digitalWrite(motor2Pin, LOW);
      }
      
  if(button0.fallingEdge()){ 
    counter ++;
  }  

  if(counter >= 2){
    counter = 0;
  }

  if(counter == 0){
    digitalWrite(motor1Pin, LOW);
    digitalWrite(motor2Pin, HIGH); 
  }

  else if(counter == 1){
    digitalWrite(motor1Pin, HIGH);  
    digitalWrite(motor2Pin, LOW); 
    }
  }

I think the currentMillis/startMillis - bit has to be inside of the if function, otherwise it stops the motor immediately...?
The code basically starts the motor once a button is pressed. The first time it should move in direction A for 700 milliseconds, the second time it should move in direction B for 700 milliseconds and so on.
Everything works but I need to stop the motor after 700 milliseconds (without delay), otherwise it just keeps turning until the button is again pressed...

Max

I'm not sure if the code you posted in the previous post is the version that cause the motor to make a short jump - or your attempt to get around that.

Think of the problem in simple terms

Check if the button is pressed and the motor is NOT running
if it is
start the motor
start the timer
set motorRunning = true

Check has the time elapsed
if it has
stop the motor
set motorRunning = false

See how the checking for the elapsed time must be independent of buttons being pressed

...R

Hey Robin,

thanks for your ideas, I just tried to rewrite the code, however I just can't get it to work! :~ The motor just turns once and then stops after 700 milliseconds, that's it...

#include <Bounce.h>

int motor1Pin = 26;   
int motor2Pin = 25;    
int speedPin = 22; 
int counter;

int motor1State;
int motor2State;

Bounce button0 = Bounce(13, 100);

long startMillis = 0;
long interval = 700;

void setup() {
  pinMode(motor1Pin, OUTPUT); 
  pinMode(motor2Pin, OUTPUT); 
  pinMode(speedPin, OUTPUT);
  digitalWrite(speedPin, HIGH);
  pinMode(13, INPUT_PULLUP);

}

void loop() {    
  button0.update();

  if(button0.fallingEdge()){ 
    counter ++;

  }  

  if(counter >= 2){
    counter = 0;
  }

  if(counter == 0){
    digitalWrite(motor1Pin, LOW);
    digitalWrite(motor2Pin, HIGH); 
    (motor1State == LOW);
    (motor2State == HIGH);
  }

  else if(counter == 1){
    digitalWrite(motor1Pin, HIGH);  
    digitalWrite(motor2Pin, LOW); 
    (motor1State == HIGH);
    (motor2State == LOW); 
  }

  unsigned long currentMillis = millis();
  if(currentMillis - startMillis > interval) {     

      motor1State = LOW;
      motor2State = LOW;
      digitalWrite(motor1Pin, motor1State);
      digitalWrite(motor2Pin, motor2State);

    }            
  }

Thanks!
Max

 if(counter == 0){
    digitalWrite(motor1Pin, LOW);
    digitalWrite(motor2Pin, HIGH); 
    (motor1State == LOW);
    (motor2State == HIGH);
  }

  else if(counter == 1){
    digitalWrite(motor1Pin, HIGH);  
    digitalWrite(motor2Pin, LOW); 
    (motor1State == HIGH);
    (motor2State == LOW); 
  }

In the absence of comments in your code I presume one or both of these sections turns the motor on.

You don't seem to have defined currentMillis at all - and all values connected with millis() MUST be defined as unsigned long. (Does the code you posted actually compile?)

You have not assigned any value to currentMillis - it should be updated at the start ot loop with currentMillis = millis();

You have not updated startMillis when the motors are started ( startMillis = currentMillis; )

I am asking this question to get you to think about the problem (and not as a criticism) - "Why did you think the code you posted would work?"

...R

Hey Robin,

THAT'S IT! Thank you so much, it works 8)
I moved the unsigned long of the currentMillis to the top and the startMillis = currentMillis; to the first if function.

The final code:

#include <Bounce.h>

int motor1Pin = 26;   
int motor2Pin = 25;    
int speedPin = 22; 
int counter;

int motor1State;
int motor2State;

Bounce button0 = Bounce(13, 100);

long startMillis = 0;
long interval = 700;
unsigned long currentMillis = millis();

void setup() {
  pinMode(motor1Pin, OUTPUT); 
  pinMode(motor2Pin, OUTPUT); 
  pinMode(speedPin, OUTPUT);
  digitalWrite(speedPin, HIGH);
  pinMode(13, INPUT_PULLUP);

}

void loop() {    
  button0.update();
  currentMillis = millis();
  
  if(button0.fallingEdge()){ 
    counter ++;
    startMillis = currentMillis;
  }  

  if(counter >= 2){
    counter = 0;
  }

  if(counter == 0){
    digitalWrite(motor1Pin, LOW);
    digitalWrite(motor2Pin, HIGH); 
    (motor1State == LOW);
    (motor2State == HIGH);

  }

  else if(counter == 1){
    digitalWrite(motor1Pin, HIGH);  
    digitalWrite(motor2Pin, LOW); 
    (motor1State == HIGH);
    (motor2State == LOW); 

  }

  if(currentMillis - startMillis > interval) {     

if(motor1State == HIGH);
      motor1State = LOW;
      motor2State = LOW;
      digitalWrite(motor1Pin, motor1State);
      digitalWrite(motor2Pin, motor2State);
   
    if(motor2State == HIGH);
      motor1State = LOW;
      motor2State = LOW;
      digitalWrite(motor1Pin, motor1State);
      digitalWrite(motor2Pin, motor2State);
    }            
  }

Max

long startMillis = 0;
long interval = 700;

You must read more carefully.

Both of these also need to be unsigned long because thay have to do with millis()

...R

OK, thanks for your help!

Max