Question about loops on button press

Projects Programming
1

Hello, I'm new to Arduino and trying to figure out how to make multiple LED's blink at specified times for a specific amount of time after a button is pressed. Once the LED's go through their designated routine the program should end, until the button is pressed again and everything repeats.

I'm having difficulty grasping how to use a 4 pin push button to run the program when pressed in conjunction with millis. I will post the code I have so far, and I understand why it's not working (as soon as the button is released the loop ends and "myFunction" stops running. I'm just not sure how to fix it so that the function loops properly. Should I be using for or WHILE loops in the function or something? Thanks for any help.

int ledPin = 8; // choose the pin for the LED
int buttonPin = 7; // choose the input pin (for a pushbutton)
int val = 0; // variable for reading the pin status

// On and Off Times (as int, max=32secs)
const unsigned int onTime = 1000;
const unsigned int offTime = 200;
// Interval is how long we wait
int interval = onTime;
// Used to track if LED should be on or off
boolean LEDstate = true;

long previousMillis = 0; // will store last time LED was updated

void setup() {
pinMode(ledPin, OUTPUT); // declare LED as output
pinMode(buttonPin, INPUT); // declare pushbutton as input
Serial.begin(9600);
}

void loop(){

val = digitalRead(buttonPin); // READ INPUT VALUE FOR BUTTON

if (val == HIGH) { // IF BUTTON IS DOWN
myFunction();
}
}

void myFunction(){

digitalWrite(8, LEDstate);

unsigned long currentMillis = millis();

if (currentMillis - previousMillis >= interval) {
previousMillis = currentMillis;

if (LEDstate) {
interval = offTime;
} else {
interval = onTime;
}
LEDstate = !(LEDstate);

previousMillis = currentMillis;

}
}

2 
const unsigned int onTime = 1000;
const unsigned int offTime = 200;

Time is always stored in unsigned long variables, not int variables.

The code you posted does something. You did not say what it does.
The switch is wired to the Arduino somehow. You did not say how.
You want the code to do something. You did not say what.

More information == more help.

3

from what you are saying in the text (not the code) you will need to learn how to use a flag

your code says

if (val == HIGH) { // IF BUTTON IS DOWN
myFunction();
}

so what do you expect when you release the button the "if" fails and the call to myFunction(); stops

what you should be saying is

if (val == HIGH) { // IF BUTTON IS DOWN
flag=1;//call flag anything you want
}

the flag will always stay 1 until you tell it to change

then use

if (flag==1){
myFunction();
}

now the function will run forever as flag is stuck at 1

now you just need a timer to turn flag to 0 after a set amount of time

so make a basic millis timer (do not use the same previousMillis or interval names)

if (currentMillis - timer >= timeToRun) {
flag=0;
}

ok last part is to work out where to get the timestamp for the above timer. It will have to be updated every loop until it is required so

if (flag==0){timer=currentMillis;}

by updating the timestamp every loop the if (currentMillis - timer >= timeToRun) will never be true until you stop updating it every loop (you press the button and the flag is set to 1)

4

PaulS:

const unsigned int onTime = 1000;

const unsigned int offTime = 200;



Time is always stored in unsigned long variables, not int variables.

That is, should be stored in unsigned long variables. For the reason that if you later change an interval control constant like onTime to a value that overflows an int, you will experience a failure.

However, it is a safe and acceptable practice to use an unsigned int, or even a byte for that purpose, if you have a justification to do it (as long as the assigned value doesn't overflow the data type). Thus, it is a matter of best practices and not program correctness.

On the other hand, time stamp variables must always be unsigned long type.

You defined previousMillis as a long, which will result in a failure.

5

gpop1:
from what you are saying in the text (not the code) you will need to learn how to use a flag

Yes, I that was what I was missing, thank you sir.

6

Ok, so it seems to be working now, but it isn't consistent. When I press the button it blinks a certain amount of times based on the "timeToRun" variable which is 10000, and then stops. If I press the button again it goes through the process again, but sometimes the LED ends up on, sometimes it's off.
(Sometimes it stays ON in the 6th blink, sometimes it does the 6th blink and turns off).

There's obviously some timing issue, so I'm not sure if I should be trying to reset the values once it finishes running so they're consistent each time, or is something off in the code?

I thought it was something to do with how long my finger was on the button before I removed it that was messing it up so I added a delay, but that doesn't seem to help. Any suggestions? Here's the current code:

/************************* VARIABLES ****************************/

int ledPin = 8; // LED pin
int buttonPin = 7; // Input pin (for a pushbutton)
int buttonVal = 0; // variable for reading the pin status
const unsigned long onTime = 1000; // On Times (as int, max=32secs)
const unsigned long offTime = 500; // Off Times (as int, max=32secs)
int waitInterval = onTime; // Interval how long we wait
boolean LED8state = false; // Used to track if LED on pin 8 should be on or off
unsigned long previousMillisLED = 0; // Will store last time LED was updated
unsigned long previousMillisTimer = 0; // Will store last time Timer was updated
const unsigned long timeToRun = 10000; // Amount of time to run entire program
int flag = 0; // Remember if function should loop or not
unsigned long currentMillis = millis(); // Tracks the time program has been running

/************************* SETUP ********************************/

void setup() {
pinMode(ledPin, OUTPUT); // Declare LED as output
pinMode(buttonPin, INPUT); // Declare pushbutton as input
Serial.begin(9600); // Communicate with PC
}

/************************* LOOP *********************************/

void loop(){

buttonVal = digitalRead(buttonPin); // Read Input value for button

if (buttonVal == HIGH) { // If the button is down...
delay(1000); // Delay to give user time to remove finger from button
flag = 1; // Set flag to 1 (loop function)
// Application Started
}

currentMillis = millis(); // Counts up from 0

if (flag == 1)
{
myFunction(); // Run function
}
if (flag == 0)
{
previousMillisTimer = currentMillis; // Remembers last Millis time flag was 1
}
}

/************************* FUNCTIONS ****************************/

// LED Blink function
void myFunction(){

digitalWrite(8, LED8state); // Set Pin 8 each timethrough myFunction() (true or false, 1 or 0)

if (currentMillis - previousMillisLED >= waitInterval) { // If amount (which keeps growing) >= 1000

if (LED8state) { // Change wait interval, based on current LED state
waitInterval = offTime; // LED is currently on, set time to stay off
} else {
waitInterval = onTime; // LED is currently off, set time to stay on
}

LED8state = !(LED8state); // Toggle the LED's on/off state
previousMillisLED = currentMillis; // Remember current Millis for LED for later
}

if (currentMillis - previousMillisTimer >= timeToRun) { // If amount >= 10000
flag = 0; // Set flag to 0 to end loop
}
}

7

@Nooffswitch: Please read the two posts by Nick Gammon at the top of the Forum for guidelines on posting here, especially the use of code tags ("</>") when posting source code. It helps us help you.

8

Before you post your code again, pay some attention to the variable names. The word millis does NOT need to appear in the name, necessarily. previosLEDstateChangeTime makes more sense than previousMillisLED.

   delay(1000);                        // Delay to give user time to remove finger from button

A three toed sloth can move faster than that.

There is no sense in having Serial.begin() in setup() with no Serial.print()s anywhere. Debug by guesswork is moronic.

9

econjack:
@Nooffswitch: Please read the two posts by Nick Gammon at the top of the Forum for guidelines on posting here, especially the use of code tags ("</>") when posting source code. It helps us help you.

Ok, I'll check it out, thanks.

10

PaulS:
Before you post your code again, pay some attention to the variable names. The word millis does NOT need to appear in the name, necessarily. previosLEDstateChangeTime makes more sense than previousMillisLED.

   delay(1000);                        // Delay to give user time to remove finger from button

A three toed sloth can move faster than that.

There is no sense in having Serial.begin() in setup() with no Serial.print()s anywhere. Debug by guesswork is moronic.

The variable names make sense to me or I'd change them, plus there's comments for more clarity. I do have Serial.println all over my code, I just removed it before posting here to keep things cleaner.

Is the delay too slow? Would it fix the issues I'm having by changing it?

11

I do have Serial.println all over my code, I just removed it before posting here to keep things cleaner.

You can post the code with the Serial.print()s in it, or you can expect us to add them, to learn what your code is doing. Which seems more likely to get people to actually try running your code?

Is the delay too slow?

Delays are always the same speed - one microsecond per microsecond. The delay is way too long. Whether that matters, or not, is hard to say without actually running the code and observing its behavior. And, that would be necessary as, frankly, I can't understand what you want the code to do or what it is actually doing.

12

you tell the arduino I want to hit and button and for a set amount of time toggle a led. Now you could be smart and try to calculate when the time is up if the led is going to be high or low but whats the point.

why not just tell the arduino that when the timer is done to go ahead and set the pin low so it will always end up being off

maybe where you set the flag to 0 you can add a extra line to shut the led off

13

gpop1:
maybe where you set the flag to 0 you can add a extra line to shut the led off

I can give that a try, however my concern is once I add more LED's and extend the time to say 3 minutes, how off will the result be then? If I can't get one LED blinking when I want it for the amount of time I want, it might bite me down the road.

14

Nooffswitch:
I can give that a try, however my concern is once I add more LED's and extend the time to say 3 minutes, how off will the result be then? If I can't get one LED blinking when I want it for the amount of time I want, it might bite me down the road.

look at the main loop code and read from top to bottom. Now do that hundreds of times a second.

what do you think its going to do every time it sees that delay(1000)

its going to come to a grinding halt and stay on that line for 1 second. So the led wont toggle or do nothing for 1 second.....now it loops and were back to the delay again.

loss the delay and add a millis timer to handle the debounce it should be about 25millis for the length of this program.

Then again the problem is most probably the fact you didn't add a 10k pull down resistor so why not just use INPUT_PULLUP to solve the problem instead. (research how to use a pullup and how to change the code to read LOW)

15

gpop1:
look at the main loop code and read from top to bottom. Now do that hundreds of times a second.

what do you think its going to do every time it sees that delay(1000)

its going to come to a grinding halt and stay on that line for 1 second. So the led wont toggle or do nothing for 1 second.....now it loops and were back to the delay again.

loss the delay and add a millis timer to handle the debounce it should be about 25millis for the length of this program.

Then again the problem is most probably the fact you didn't add a 10k pull down resistor so why not just use INPUT_PULLUP to solve the problem instead. (research how to use a pullup and how to change the code to read LOW)

Ok I'll look into that, I appreciate the help.

16

PaulS:
The word millis does NOT need to appear in the name,

However, I think it's a good idea that any name that relates to a physical quantity should always be suffixed with the unit of measurement. getWeightKg() or getWeightLbs(), INTERVAL_LENGTH_MIN or INTERVAL_LENGHT_MS.