Blink Without Delay Revisited

Your code has a potential catastrophic failure when millis wraps.

@Coding Badly - agreed, but I wanted to suggest an approach rather than a specific implementation. :slight_smile:

I'm doing my scheduling with usec rather than msec and using 64-bit time values which postpones the potential catastrophic failure for somewhat more than a half-million years (584,542 to be exact) - but requires that the time-keeping function be called at least once every 1:11:34.967295, which isn't a problem in my application.

It's a bit over the top for a blinky demonstration but would you care to see the code?

but would you care to see the code?

always, as long as it is educative !

I mean, Is your usec time keeper not a nice one to be "classified"?

I'm happy to share it...

/*----------------------------------------------------------------------------*/
/* Define a translation unit scope variable to hold latest RESET time      TU */
/*----------------------------------------------------------------------------*/
static time_t rst_time = 0-0;          /* Epochal time of latest RESET (usec) */
/*----------------------------------------------------------------------------*/
/* Extend micros() for 64-bit up-time                                      TU */
/*----------------------------------------------------------------------------*/
static time_t u64(void)
{  static union                        /* Definitions to allow accessing time */
   {  time_t hl;                       /* As a whole, or as...                */
      struct                           /* Anonymous struct container          */
      {  unsigned long l;              /* Lower half and...                   */
         unsigned long h;              /* Upper half                          */
      };                               /*  end: (anonymous) {}                */
   }  u = { 0 };                       /* Initialize to zero                  */
   static unsigned long t;             /* Variable to hold micros() value     */
   if (u.l > (t = micros())) ++u.h;    /* If micros() wrapped, adjust         */
   u.l = t;                            /* Save updated low half               */
   return u.hl;                        /* Return 64-bit usec time             */
}                                      /*  end: u64()                         */
/*----------------------------------------------------------------------------*/
/* Return current date/time as usec into the epoch                            */
/*----------------------------------------------------------------------------*/
time_t now(void)
{  return rst_time + u64();            /* Return reset time + uptime          */
}                                      /*  end: now()                         */
/*----------------------------------------------------------------------------*/
/* Set RESET date/time using externally-supplied current epochal date/time    */
/*----------------------------------------------------------------------------*/
void set_time(time_t t)
{  rst_time = t - u64();
}
/*----------------------------------------------------------------------------*/

codingbadly wrote:

Your code has a potential catastrophic failure when millis wraps.

I thought about this too. Do you know if delay() has this same problem? Do you know where the
source for delay() is located? Difficult for me to find.

By my calculation, millis() being unsigned long should take 47.7 days before wrapping.

An app that would be intended to run that long might have a 1-line fix in each state-machine to
deal with the problem of wrapping, in the form of if( (4294967296UL - milis()) < delay ) ....., or
somesuch [need to suss it all out]. Maybe reset the entire program, etc.

millis() wraps every 49d 17:02:47.295 and a 64-bit msec clock will wrap in only 584,542,046 years! :grin:

@Morris:

millis() wraps every 49d 17:02:47.295 and a 64-bit msec clock will wrap in only 584,542,046 years!

oops, typo on the 47.7 days. Do you know if delay() will crash?

delay() has been around long enough that I think we'd have heard about any problems - but I think I can guarantee that it'll never crash on my machine. :grin:

I welcome correction, but I believe rewriting two lines of code takes care of the wrapping problem with millis():

unsigned long blinkTime = 0;           /* When to execute blink()             */

void blink(void)
{  static int x = 0;                   /* Current LED state 0=>off, 1=>on     */
   digitalWrite(13,x ^= 1);            /* Set LED to opposite state           */
   blinkTime = millis();         // ***********************************************************
}
void watch(void)
{  if (millis() - blinkTime >= 250 ) blink();  // *************************************
}
void setup(void)
{  pinMode(13,OUTPUT);                 /* Use on-board LED at pin 13 as O/P   */
}
void loop(void)
{  watch();                            /* Change state whenever it's time     */
}

Jim

oric_dan(333):
oops, typo on the 47.7 days. Do you know if delay() will crash?

Delay does the subtraction:

void delay(unsigned long ms)
{
	uint16_t start = (uint16_t)micros();

	while (ms > 0) {
		if (((uint16_t)micros() - start) >= 1000) {
			ms--;
			start += 1000;
		}
	}
}

Morris, I know you are proposing an idea, but I suggest if this creeps into production that you use a constant for the LED. I curse every time I load up the "blink" sample program to test something, and have to change "13" to (say) "8" in three places.

Change to:

// configuration
const byte LED = 13;                      // pin for LED (13 is on-board LED)
const unsigned long blinkInterval = 250;  // milliseconds

// internal variables
unsigned long blinkTime = 0;           /* When blink() last executed        */

void blink ()
  {  
  static byte x = 0;                   /* Current LED state 0=>off, 1=>on     */
  digitalWrite (LED, x ^= 1);          /* Set LED to opposite state           */
  blinkTime = millis ();        
  }  // end of blink
  
void watch ()
  {  
  if ((millis () - blinkTime) >= blinkInterval) 
      blink ();  
  }  // end of watch
  
void setup ()
  {  
  pinMode (LED, OUTPUT);              
  }  // end of setup

void loop()
  {  
  watch ();                            /* Change state whenever it's time     */
  }  // end of loop

Also some pedantic code clean-ups. :wink:

I just wrote my own test and it works fine:

extern unsigned long timer0_millis;

/*************************************/
void setup() 
{
  pinMode(ledPin, OUTPUT);    
  Serial.begin(57600);
  timer0_millis = 4294967295UL - 10000UL;    // 2^32 = 4294967296.
}

/*************************************/
void loop()
{
  Serial.print("\n");
  Serial.print(timer0_millis);
  delay(1000);  
}

output:
4294957295
4294958295
4294959295
4294960297
4294961297
4294962298
4294963299
4294964300
4294965300
4294966302
6
1007
2007
3007
4008
5008

Nick:
thanks for posting the function code. Can you tell me where you found it, I am still having trouble
rooting through all the IDE directories. Thanks.

oric_dan(333):
Do you know if delay() has this same problem?

It does not.

Do you know where the source for delay() is located?

https://github.com/arduino/Arduino/blob/master/hardware/arduino/cores/arduino/wiring.c#L107

{ArduinoRoot}\hardware\arduino\cores\arduino
wiring.c

JimG:
I welcome correction...

Looks good to me.

It was in wiring.c.

I just use a brute-force "find in files" to root out things like this. Crimson Editor on Windows is good for that.

On the Mac or Linux, one approach could be to use find and grep, like this:

$ cd '/Applications/Arduino v0022.app/Contents/Resources/Java/hardware/arduino'

(or whatever directory it is).

Then:

$ find . -type f -name *.c* -exec grep -H -i -w "delay" {} ';'

This is searching all .c* (ie. c and cpp) files for "delay", not case-sensitive (-i) and full word (-w). The -H tells grep to show the file name, so you know which file to look into.

I get a fairly brief output:

./bootloaders/atmega/ATmegaBOOT_168.c:#include <util/delay.h>
./bootloaders/atmega8/ATmegaBOOT.c:#include <avr/delay.h>
./bootloaders/atmega8/ATmegaBOOT.c:		  //time_count=0; // exted the delay once entered prog.mode
./bootloaders/bt/ATmegaBOOT_168.c:#include <util/delay.h> 
./bootloaders/lilypad/src/ATmegaBOOT.c:	/* l needs to be volatile or the delay loops below might get
./bootloaders/stk500v2/stk500boot.c:#include	<util/delay.h>
./bootloaders/stk500v2/stk500boot.c:	delay_ms(100);							// delay after exit
./bootloaders/stk500v2/stk500boot.c:	// without a delay, we seem to read from the wrong channel
./bootloaders/stk500v2/stk500boot.c:	//delay(1);
./cores/arduino/wiring.c:void delay(unsigned long ms)
./cores/arduino/wiring.c:/* Delay for the given number of microseconds.  Assumes a 8 or 16 MHz clock. */
./cores/arduino/wiring.c:	// for a one-microsecond delay, simply return.  the overhead
./cores/arduino/wiring.c:	// of the function call yields a delay of approximately 1 1/8 us.
./cores/arduino/wiring.c:	// delay requested.
./cores/arduino/wiring.c:	// for a one- or two-microsecond delay, simply return.  the overhead of
./cores/arduino/wiring.c:	// delay requested.
./cores/arduino/wiring_analog.c:	// without a delay, we seem to read from the wrong channel
./cores/arduino/wiring_analog.c:	//delay(1);

If you happen to know it is void, you can narrow down the output:

$ find . -type f -name *.c* -exec grep -Hiw "void delay" {} ';' 

./cores/arduino/wiring.c:void delay(unsigned long ms)

No doubt there are more sophisticated ways of doing this. :slight_smile:

Another example, to find what the definition for HIGH is:

$ find . -type f -name *.h -exec grep -Hw "HIGH" {} ';' 

./cores/arduino/wiring.h:#define HIGH 0x1

This time I looked in .h files, and omitted the case-insensitive flag. Got a single match! Can't do much better than that. :slight_smile:

Nick and Badly, thanks for the pointers. Now I won't have to ask so many naiive questions [maybe].
But, speaking of which, am I missing something, or won't the following code have the same wrap
problems as the original? It uses the same construct.

I think it does, BUT since start is defined as a uint, the wrap will occur every 65.536 msec, and
the count will quickly catch up to the test point again, rather than possibly having to wait for
49.7 days, as in the original situation. Hmmm, ????

[maybe I need to look at micros() now - it never ends].

void delay(unsigned long ms)
{
    uint16_t start = (uint16_t)micros();

    while (ms > 0) {
        if (((uint16_t)micros() - start) >= 1000) {
            ms--;
            start += 1000;
        }
    }
}

Give these a perusal...

http://arduino.cc/playground/Code/TimingRollover

The key is to calculate a difference between now and some earlier time (or between now and some future time).

@JimG - Yes, that works too.

@Nick - Ok. I'd made the assumption that everyone who had an on-board LED would find it at pin 13. I'll gladly provide an easily modified top-of-page definition in future examples.

My local style police frown at // comments, and they shoot on sight when they see trailing opening braces or code undented to the left margin. I generally don't much worry about coding style - I have what appears to be a pretty good code "beautifier" that allows me to reformat to what works best for me with my 5081 roots, although I only take the trouble to use it if the problem/solution is sufficiently interesting. I like helping people, but I'm not much into pointless effort or self-abuse. :slight_smile:

It would seem that most folks here find their needs adequately met with a periodically overflowing uptime, while for my Arduino application "world" time offers advantages that far more than offset the costs.

The delay() function, to me, is a resource-waster. If I'd wanted fewer resources, I'd have bought a Uno. :grin:

The key is to calculate a difference between now and some earlier time (or between now and some future time).

DOH, some times you can't win for losing. or possibly the other way around, even.
Where's the beef? .... um, wrap?

Maybe I should take the rest of the weekend off, LOL.

/*************************************/
void setup() 
{
  Serial.begin(57600);
}

/*************************************/
void loop()
{
  unsigned int cnt, start;
  unsigned long lcnt, lstart;
  
  cnt = start = 65535U - 500U;
  lcnt = lstart = 4294967295UL - 500UL;
  
  Serial.print("\n    cnt  cnt-start          lcnt  lcnt-lstart ");
  while( 1 ) {
    Serial.print("\nUints: ");
    Serial.print( cnt );
    Serial.print(" ");
    Serial.print( cnt-start );
    if( (cnt - start) >= 1000) Serial.print(" yes");
      else Serial.print(" no");
      
    Serial.print("     Ulongs: ");
    Serial.print( lcnt );
    Serial.print(" ");
    Serial.print( lcnt-lstart );
    if( (lcnt - lstart) >= 1000L) Serial.print(" yes");
      else Serial.print(" no");
    
    cnt += 100;
    lcnt += 100L;
    delay(500);
  } 
}

output:
    cnt  cnt-start          lcnt  lcnt-lstart
Uints: 65035 0 no     Ulongs: 4294966795 0 no
Uints: 65135 100 no     Ulongs: 4294966895 100 no
Uints: 65235 200 no    Ulongs: 4294966995 200 no
Uints: 65335 300 no     Ulongs: 4294967095 300 no
Uints: 65435 400 no     Ulongs: 4294967195 400 no
Uints: 65535 500 no     Ulongs: 4294967295 500 no
Uints: 99 600 no     Ulongs: 99 600 no
Uints: 199 700 no     Ulongs: 199 700 no
Uints: 299 800 no     Ulongs: 299 800 no
Uints: 399 900 no     Ulongs: 399 900 no
Uints: 499 1000 yes   Ulongs: 499 1000 yes
Uints: 599 1100 yes   Ulongs: 599 1100 yes
Uints: 699 1200 yes     Ulongs: 699 1200 yes
Uints: 799 1300 yes     Ulongs: 799 1300 yes
Uints: 899 1400 yes     Ulongs: 899 1400 yes
Uints: 999 1500 yes     Ulongs: 999 1500 yes
Uints: 1099 1600 yes     Ulongs: 1099 1600 yes
Uints: 1199 1700 yes     Ulongs: 1199 1700 yes
Uints: 1299 1800 yes     Ulongs: 1299 1800 yes
Uints: 1399 1900 yes     Ulongs: 1399 1900 yes
Uints: 1499 2000 yes     Ulongs: 1499 2000 yes
Uints: 1599 2100 yes     Ulongs: 1599 2100 yes
Uints: 1699 2200 yes     Ulongs: 1699 2200 yes
Uints: 1799 2300 yes     Ulongs: 1799 2300 yes
Uints: 1899 2400 yes     Ulongs: 1899 2400 yes

They have the advantage that you can "comment out" a batch of lines, provided those lines use // comments. That is, because the /* ... */ comments don't nest.

The subtraction method of calculating elapsed time means you don't need to care if it overflows or not.