Running code every x seconds, no matter how long execution within loop takes

Question

I'm trying to make an LED blink to the beat of a certain song. The song has exactly 125 bpm.
The code that I wrote seems to work at first, but the longer it runs the bigger the difference in time between the LED flashes and the next beat starts. The LED seems to blink a tiny bit too slow.

I think that happens because lastBlink is kind of depending on the blink which happened right before that to stay in sync, instead of using one static initial value to sync to...

unsigned int bpm = 125;
int flashDuration = 10;
unsigned int lastBlink = 0;
for(;;) {
    if (getTickCount() >= lastBlink+1000/(bpm/60)) {
        lastBlink = getTickCount();
        printf("Blink!\r\n");
        RS232_SendByte(cport_nr, 4); //LED ON
        delay(flashDuration);
        RS232_SendByte(cport_nr, 0); //LED OFF
    }
}

Answer 1

Add value to lastBlink, not reread it as the getTickCount might have skipped more than the exact beats want to wait.

lastblink+=1000/(bpm/60);

Answer 2

Busy-waiting is bad, it spins the CPU for no good reason, and under most OS's it will lead to your process being punished -- the OS will notice that it is using up lots of CPU time and dynamically lower its priority so that other, less-greedy programs get first dibs on CPU time. It's much better to sleep until the appointed time(s) instead.

The trick is to dynamically calculate the amount of time to sleep until the next time to blink, based on the current system-clock time. (Simply delaying by a fixed amount of time means you will inevitably drift, since each iteration of your loop takes a non-zero and somewhat indeterminate time to execute).

Example code (tested under MacOS/X, probably also compiles under Linux, but can be adapted for just about any OS with some changes) follows:

#include <stdio.h>
#include <unistd.h>
#include <sys/times.h>

// unit conversion code, just to make the conversion more obvious and self-documenting
static unsigned long long SecondsToMillis(unsigned long secs) {return secs*1000;}
static unsigned long long MillisToMicros(unsigned long ms)    {return ms*1000;}
static unsigned long long NanosToMillis(unsigned long nanos)  {return nanos/1000000;}

// Returns the current absolute time, in milliseconds, based on the appropriate high-resolution clock
static unsigned long long getCurrentTimeMillis()
{
#if defined(USE_POSIX_MONOTONIC_CLOCK)
   // Nicer New-style version using clock_gettime() and the monotonic clock
   struct timespec ts;
   return (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) ? (SecondsToMillis(ts.tv_sec)+NanosToMillis(ts.tv_nsec)) : 0;
#  else
   // old-school POSIX version using times()
   static clock_t _ticksPerSecond = 0;
   if (_ticksPerSecond <= 0) _ticksPerSecond = sysconf(_SC_CLK_TCK);

   struct tms junk; clock_t newTicks = (clock_t) times(&junk);
   return (_ticksPerSecond > 0) ? (SecondsToMillis((unsigned long long)newTicks)/_ticksPerSecond) : 0;
#endif
}

int main(int, char **)
{
   const unsigned int bpm = 125;
   const unsigned int flashDurationMillis = 10;
   const unsigned int millisBetweenBlinks = SecondsToMillis(60)/bpm;
   printf("Milliseconds between blinks:  %u\n", millisBetweenBlinks);

   unsigned long long nextBlinkTimeMillis = getCurrentTimeMillis();
   for(;;) {
       long long millisToSleepFor = nextBlinkTimeMillis - getCurrentTimeMillis();
       if (millisToSleepFor > 0) usleep(MillisToMicros(millisToSleepFor));

       printf("Blink!\r\n");
       //RS232_SendByte(cport_nr, 4); //LED ON
       usleep(MillisToMicros(flashDurationMillis));
       //RS232_SendByte(cport_nr, 0); //LED OFF
       nextBlinkTimeMillis += millisBetweenBlinks;
   }
}