Issue with millis() in a while loop

Hi all,

I'm trying to write a while loop that will break out of the loop after a certain amount of time using the millis() function. This is a simplified version of what I'm doing, which does't seem to work:

// include the library code:
#include <Wire.h>

unsigned long sec = 0;
unsigned long timeout = 4000;

void setup() {
  
  Serial.begin(9600);
  
}

void loop() {
  
    do
    {
    Serial.println( millis() );
    sec = millis();
    }
    while ( sec < timeout );
    
}

Looking at the serial monitor, 'sec' happily goes over 4000 without the loop stopping.

Does anyone know why this might be?

Thanks,

Mark

Yes,

  1. loop() is called over and over again

  2. you have used do - while so the while is checked AFTER the body of the do. Try a while where the while is checked first.

Mark

Hi Mark, thanks for the reply. I see what you mean and that is clearly an issue with this simple example. However in my actual program the loop is proceeded by more code which should be executed when the loop exits, which it doesn't.

Anyway I've changed the example as you say and now strangely I don't get anything at all printed in the monitor:

void loop() {
  
    while ( sec < timeout );
    {
    Serial.println( millis() );
    sec = millis();
    }
    
}

Nothing strange about that :wink:

    while ( sec < timeout );

Lose the semi-colon.

proceeded by

means before, so it can't be executed after the while loop.

Did you have the serial monitor on the right com port?

Post all the code each time not just part of it.

Lose the semi-colon.

Damm.

Mark

holmes4:
means before, so it can't be executed after the while loop.

Did you have the serial monitor on the right com port?

Post all the code each time not just part of it.
Damm.

Mark

PREcede means before. PROceed means to move forward, so doesn't really make sense in this context. However, his intent was clearly "followed by".

Regards,
Ray L.

Thanks all, clearly some school boy errors on my part.

Here is the actual loop from my program (as opposed to the test above):

  Start = micros();

  if ( lightlevel >= LightThresh )
  {
    Serial.println("Light sensed");
    do
    {
      miclevel = analogRead( MicrophonePin );
    }
    while ( (miclevel < SoundThresh) || ( micros()-Start > 2000000) );
    PicEarly = true;
  }

The idea is that if miclevel does not exceed SoundThresh within 2 seconds then the loop ends.

I'm happy to add the rest of the code if it helps.

markbeverley:
Thanks all, clearly some school boy errors on my part.

Here is the actual loop from my program (as opposed to the test above):

    while ( (miclevel < SoundThresh) || ( micros()-Start > 2000000) );

}

If miclevel is smaller than soundThresh, how it is ever supposed to get past this line?

Hi Ken, miclevel is essentially a microphone attached to an analogue input. This is being read in the loop - if/when the level of the input exceeds SoundThresh the loop will finish. If miclevel does not exceed SoundThresh within 2 seconds then the loop should also finish (but it doesn't).

Mark

markbeverley:
The idea is that if miclevel does not exceed SoundThresh within 2 seconds then the loop ends.

Don't use WHILE for that purpose as it blocks everything.

Look at how millis() is used to manage timing without blocking in several things at a time.

...R

if/when the level of the input exceeds SoundThresh the loop will finish. If miclevel does not exceed SoundThresh within 2 seconds then the loop should also finish (but it doesn't).

Look closely at this :wink:

while ( (miclevel < SoundThresh) || ( micros()-Start > 2000000) );

long constants should be marked long so the compiler knows their type:

  do { ... }
  while ( (miclevel < SoundThresh) || ( micros()-Start > 2000000ul) );

Also its not obvious but important that the time test is done exactly this way:

Subtract two timestamps from each other (they must either both be from micros()
or both from millis() originally. Compare the difference to a time interval.

Don't compare a timestamp directly to another timestamp, it doesn't work
at wrap-around (which is quite frequent when using micros()). Subtraction
works correctly at wrap-around so long as the result is in range of representation.

So your initial example was flawed anyway.

LarryD:
Look closely at this :wink:

while ( (miclevel < SoundThresh) || ( micros()-Start > 2000000) );

I believe I have used 'greater than' when I should have used 'less than' - good spot!

Will test when I next have the chance (Monday).