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<10000; i++) {
     if(digitalRead(pin,HIGH){
     button = true;
}
     delay(1);

    }
  }
}

Answer 1

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.

Answer 2

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);

Answer 3

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);
  }
}