Using 1 ms delay() in a for loop to check inputs.. Bad?

Question

So first off, I know millis() exists. Just asking out of curiosity. Say you have a button you wanna check for during a delay.. Maybe a 10 second delay. So you make a for loop that checks for input and delays 1 ms... 10,000 times. What's bad about this?

 int pin = 13;   
    bool button = false;
void setup()
{
  pinMode(pin, INPUT);    
}

void loop()
{

 while(button == false) {
 for(int i = 0; i&lt;10000; i++) {
 if(digitalRead(pin,HIGH){
 button = true;

}
     delay(1);
}

}
}

the busybee · Accepted Answer · 2022-03-18T09:02:47.760

You can do this, for quite simple sketches. It starts to become ugly if you add more features and conditions.

Therefore, the "correct^TM" (read: professional) way is to implement a state machine. See in the IDE the non-delay blink example.

const unsigned long DELAY_MS = 1000;
int pin = 13;
enum State {
    WAITING, WORKING,
};
State state;
unsigned long startTime;
void setup()
{
    pinMode(pin, INPUT);
    state = WAITING;
    startTime = millis();
}
void loop()
{
    switch (state) {
    case WAITING:
        // You can put both conditions in one "if" with an "||" operator.
        // But it will look less clear.
        if (millis() - startTime > DELAY_MS) {
            state = WORKING;
        } else if (digitalRead(pin)) {
            state = WORKING;
        }
        break;
    case WORKING:
        // anything to do after the delay or the button
        break;
    default:
        // any error handling you see fit
        break;
    }
}

The function loop() is designed to be called endlessly in repetition. Use this to your advantage.

score 1 · Answer 2 · answered Mar 18 '22 at 06:20

What's bad about this?

It looks a bit clunky but I suppose there is nothing particularly bad about it (apart from your typo of digitalWrite rather than digitalRead).

I would do it more like:

unsigned long now = millis ();
button = LOW;
while ((millis () - now < 10000) && (button == LOW))
  button = digitalRead (pin);

score 0 · Answer 3 · answered Mar 18 '22 at 08:48

I know millis() exists

Ok, millis is always the best way...

Say you have a button you wanna check for during a delay.. So you make a for loop that checks for input and delays 1 ms... 10,000 times.

Your code is not really doing what you want. Your while loop never ends until button is pressed so 10s delay has no sense. I suppose your code should exit after 10 seconds or if button is pressed.

Just asking out of curiosity

You're code, without millis(), should be

bool button = false;
...
...
void loop() {
  uint16_t ms2wait = 10000;
  while (button == false && --ms2wait > 0) {
    if(digitalRead(pin,HIGH)
      button = true;
    delay(1);
  }
}

Using 1 ms delay() in a for loop to check inputs.. Bad?

3 Answers3