Go Down

Topic: Issue with millis() in a while loop (Read 198 times) previous topic - next topic

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:

Code: [Select]
// 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

holmes4

#1
Jan 16, 2015, 04:38 pm Last Edit: Jan 16, 2015, 04:41 pm by holmes4
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:

Code: [Select]
void loop() {
  
    while ( sec < timeout );
    {
    Serial.println( millis() );
    sec = millis();
    }
    
}

wildbill

Nothing strange about that  ;)

Code: [Select]
    while ( sec < timeout );

Lose the semi-colon.

holmes4

#4
Jan 16, 2015, 05:22 pm Last Edit: Jan 16, 2015, 05:23 pm by holmes4
Quote
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.

Quote
Lose the semi-colon.
Damm.

Mark

RayLivingston

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

Code: [Select]

  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.

KenF

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

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

Code: [Select]

    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

Robin2

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

LarryD

#10
Jan 16, 2015, 09:43 pm Last Edit: Jan 16, 2015, 09:44 pm by LarryD
Quote
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  ;)

while ( (miclevel < SoundThresh) || ( micros()-Start > 2000000) );
The way you have it in your schematic isn't the same as how you have it wired up!

MarkT

#11
Jan 16, 2015, 10:11 pm Last Edit: Jan 16, 2015, 10:12 pm by MarkT
long constants should be marked long so the compiler knows their type:

Code: [Select]

  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.
[ I won't respond to messages, use the forum please ]

Look closely at this  ;)

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

Go Up
 


Please enter a valid email to subscribe

Confirm your email address

We need to confirm your email address.
To complete the subscription, please click the link in the email we just sent you.

Thank you for subscribing!

Arduino
via Egeo 16
Torino, 10131
Italy