problem using millis() turning motor on for specified period

Hi

I am trying to turn on a DC brushed motor for a set time when a button is pressed.

I am using an arduino uno with the arduino motor shield and this DC motor : https://uk.rs-online.com/web/p/dc-geared-motors/4540861/

here is my code

const int buttonPin1 = 2;
const long motorRunMillis = 5000;
unsigned long currentMillis = 0;
int buttonState1 = 0; 

void setup() {
  
  //Setup Channel A
  pinMode(13, OUTPUT); //Initiates Motor Channel A pin
  pinMode(8, OUTPUT); //Initiates Brake Channel A pin
  pinMode(buttonPin1, INPUT);
}

void loop(){
 
  readButton(); 
}

void readButton() {
   buttonState1 = digitalRead(buttonPin1);
   if (buttonState1 == HIGH) {                  //if button is pressed, start timer and turn motor on
    currentMillis = millis(); 
    digitalWrite(13, HIGH);
    digitalWrite(8, LOW);
    analogWrite(11, 255); 
    if (currentMillis > motorRunMillis){      //if current time is greater than 5000ms, turn motor off
      digitalWrite(8, HIGH);
      
    }
   
   }

}

I have done this using a delay but I would like to do it the 'proper way' using the millis() function.

This program is going horribly wrong somewhere (probably something i have obviously missed out) and the motor turns on when the program is uploaded and when the button is pressed the motor turns off, when it is pressed and held further the motor turns on but a lot slower and quieter.

Any help/advice much appreciated!

and if any further info required let me know !

thanks in advance.

the motor turns on when the program is uploaded

Turn it off in the setup() section of the sketch.

It looks like a state machine to me:

Motor has two sates: IS_ON, IS_OFF

switch (state)
  case IS_ON: change to IS_OFF if chronometer is up to motorRunMillis;
  case IS_OFF: change to IS_ON and Reset chronometer if buttonPin1 is HIGH;

Jacques

This is not the correct way to use millis()

    if (currentMillis > motorRunMillis){      //if current time is greater than 5000ms, turn motor off
      digitalWrite(8, HIGH);
     
    }

Have a look at how it is done in Several Things at a Time

...R

thank you both for your replies.

Robin2:
This is not the correct way to use millis()

    if (currentMillis > motorRunMillis){      //if current time is greater than 5000ms, turn motor off

digitalWrite(8, HIGH);
   
    }




Have a look at how it is done in [Several Things at a Time](http://forum.arduino.cc/index.php?topic=223286.0)


...R

Yes robin I was afraid thats where my problem was, is it correct to use the line

currentMillis = millis();

in the if statement for when the button is pressed? ie am I correct in saying the time is basically started when the button is pressed?

I was actually reading your several things at a time post earlier today! However I struggled in understanding how to implement the method you use the millis() for flashing an led to using it to just switch something on and off once due to a particular event (button pressed) ?

jbellavance:
Turn it off in the setup() section of the sketch.

Yes I will add that!

To start the countdown:currentMillis = millis();
To find out if the time is up:if(millis() - currentMillis >= motorRunMillis) {...}
Jacques

Try not to think of using millis as being some esoteric coding practice to learn. It's not. It is something you already do every single day.

If it was 4:00 when I said to meet me in ten minutes. Now it is 4:09, should you be here yet? What if it was 4:11? How did you figure that out? That's all we're doing with millis.

Delta_G:
Try not to think of using millis as being some esoteric coding practice to learn. It's not. It is something you already do every single day.

If it was 4:00 when I said to meet me in ten minutes. Now it is 4:09, should you be here yet? What if it was 4:11? How did you figure that out? That's all we're doing with millis.

thanks for the anology delta, I think that has improved my understanding more, to be honest I was probably overthinking/complicating it to begin with!.

jbellavance:
To start the countdown:currentMillis = millis();
To find out if the time is up:if(millis() - currentMillis >= motorRunMillis) {...}
Jacques

thank you again! I have modified my code from your suggestions, the motor only runs when the button is pressed now, however it still does not turn off after the time period I have set, here is my latest coding:

const int buttonPin1 = 2;
const long motorRunMillis = 5000;

int buttonState1 = 0; 

void setup() {
  
  //Setup Channel A
  pinMode(13, OUTPUT); //Initiates Motor Channel A pin
  pinMode(8, OUTPUT); //Initiates Brake Channel A pin
  pinMode(buttonPin1, INPUT);
  digitalWrite(8,HIGH);
}

void loop(){
 
  readButton(); 
}

void readButton() {
   buttonState1 = digitalRead(buttonPin1);
   if (buttonState1 == HIGH) {                  //if button is pressed, start timer and turn motor on
    unsigned long currentMillis = millis(); 
    digitalWrite(13, HIGH);
    digitalWrite(8, LOW);
    analogWrite(11, 255); 
    if (millis() - currentMillis >= motorRunMillis){      //if current time is greater than 5000ms, turn motor off
      digitalWrite(8, HIGH);
      
    }
   
   }

}

:sweat_smile:

Not a pro, but "buttonState1 == 1" when the button is pressed, correct?

So

if (millis() - currentMillis >= motorRunMillis){      //if current time is greater than 5000ms, turn motor off
      digitalWrite(8, HIGH);
      
    }

does not get checked unless you're holding the button down because it's inside the "if(buttonState1 == 1)". Or am I looking at it all wrong? Haven't had coffee yet. So, very much a possibility!

DangerToMyself:
Not a pro, but "buttonState1 == 1" when the button is pressed, correct?

So

if (millis() - currentMillis >= motorRunMillis){      //if current time is greater than 5000ms, turn motor off

digitalWrite(8, HIGH);
     
    }


does not get checked unless you're holding the button down because it's inside the "if(buttonState1 == 1)". Or am I looking at it all wrong? Haven't had coffee yet. So, very much a possibility!

right you are! But if I hold the button down, the motor still stays on after the time period.

I think I need to write this program so that button pressing triggers event, ie if flag is true check timer
and if flag is false check to see if button is pressed (button pressed -> flag is true -> motor turns on -> timer starts etc)

anybody any advice on how to go about writing a program like this?

I am trying to turn on a DC brushed motor for a set time when a button is pressed.

Here is an example that uses an LED rather than a motor but it should be easy to adapt. Note that you may need to change the logic that detects button presses depending on how yours is wired.

unsigned long pressedTime;
unsigned long currentTime;
unsigned long period = 5000;
const byte buttonPin = A1;
byte buttonState = HIGH;
const byte ledPin = 13;
boolean timing = false;

void setup()
{
  Serial.begin(115200);
  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, HIGH);
}

void loop()
{
  currentTime = millis();
  buttonState = digitalRead(buttonPin);
  if (buttonState == LOW && !timing)
  {
    pressedTime = currentTime;
    digitalWrite(ledPin, LOW);
    timing = true;
  }
  if (timing && (currentTime - pressedTime >= period))
  {
    digitalWrite(ledPin, HIGH);
    timing = false;
  }
}

UKHeliBob I used your code and modified it to suit my circuit and it works perfectly! Thank you so much!

Here is the working code for anyone interested:

const int buttonPin = 2;
const long motorRunMillis = 5000;
unsigned long pressedTime;
unsigned long currentTime;
boolean timing = false;
int buttonState = 0; 

int buttonPushCounter = 0;

void setup() {
  
  //Setup Channel A
  pinMode(13, OUTPUT); //Initiates Motor Channel A pin
  pinMode(8, OUTPUT); //Initiates Brake Channel A pin
  pinMode(buttonPin, INPUT);
  digitalWrite(8,HIGH);
}

void loop(){
 
  readButton(); 
}

void readButton() {
   
   currentTime = millis();
   buttonState = digitalRead(buttonPin);
   if (buttonState == HIGH && !timing) {
    pressedTime = currentTime;
    digitalWrite(13, HIGH);
    digitalWrite(8, LOW);
    analogWrite(11, 255);
    
    timing = true;
   }

   if (timing && (currentTime - pressedTime >= motorRunMillis))
   {
    digitalWrite(8, HIGH);
    timing = false;
   }  
}

I was just wondering, whether you'd be able to help me understand the use of the boolean 'timing' in this program as, although I have it working, I don't quite understand this part of the program!

Thanks again :smiley:

I was just wondering, whether you'd be able to help me understand the use of the boolean 'timing' in this program as, although I have it working, I don't quite understand this part of the program!

The state of the timing variable controls whether the state of the button is checked during the time the motor/LED is running. Without it, pressing the button or holding it down at any time would cause a new timing period to start each time the state of the button is checked and found to be pressed.

Of course, that might be what you want to happen.

The code could easily be modified to detect when the button becomes pressed rather than when it is pressed so holding it down would not be a problem but even using that technique a new timing period would start is the button were pressed, released and pressed again during the timing period.

With that in mind I hope that you can see that your original requirement

I am trying to turn on a DC brushed motor for a set time when a button is pressed.

is actually very incomplete !

new_to_this:
right you are! But if I hold the button down, the motor still stays on after the time period.

That's because "currentMillis" was constantly being set to millis() while your button was pressed. So millis() - currentMillis would never be greater or equal to motorRunMillis

You are on track with the other code. So the point is mute.

I still see it as a state machine:

void handleMotor() {
  switch (motorState) {
    case IS_OFF: {
      if (digitalRead(buttonPin1) == HIGH) {
        currentMillis = millis();
        motorState = IS_ON;
        digitalWrite(13, HIGH);
        digitalWrite(8, HIGH);
      }
    break;
    }
    case IS_ON: {
      if (millis() - currentMillis >= motorRunMillis) {
        motorState = IS_OFF;
        digitalWrite(13, LOW);
        digitalWrite(8, LOW);
      }
    break;
    }
  }
}

Jacques