Go Down

Topic: Blink Without Delay Revisited (Read 2434 times) previous topic - next topic

Morris Dovey

Feb 25, 2012, 05:48 pm Last Edit: Feb 26, 2012, 05:22 am by Morris Dovey Reason: 1
When it comes to programming, there's almost always more than one way to solve any problem. I have a personal preference for keeping the loop() function as uncluttered as possible, and so would like to share a different approach to asynchronous program tasks:

Code: [Select]
#define LED 13

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

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


(Code was tested on an Arduino Mega2560)

Edited to add #define LED
There's always a better way!

retrolefty

I too like using user defined functions in my larger sketches. I feel that there are not enough (any?) good examples of using user defined functions in the arduino IDE example sketches. I found when writing a sketch to support a TI 16 bit I2C A/D converter that the logic of the over all main sketch was much easier to read/understand/utilize after I wrote all the low level I2C A/D commands as small functions, such as configure ADC, select channel, read conversion value, etc. After testing each small function I placed them all into a IDE tabbed file.

So while not a great or experiance programmer, I can really see the advantages of breaking down a large sketch into logical related user functions and trying to keep the main loop() function as high level as possible.

Thanks for posting your example.

Lefty

jwatte

Proper separation (factoring) of functionality is very important in software design, especially as projects get bigger, and you have more than one person working on the code.

When it comes to the code in question, though, I dislike static variables inside functions, because they make the state impossible to control. There is no way I can write a unit test for that blink function, because I cannot control what state the "x" variable has at any particular time, for example. This is one reason objects are invented: keep state isolated from the rest of the program, and "grouped" with the code that works on the state, yet be able to control and re-use it.

Morris Dovey

In my current application I find myself monitoring three completely different sensors and controlling a heater, two different stepper motors, and three solenoid valves - as well as doing some fairly comprehensive data logging. It helped me a lot to write separate modules for each device/task and have a single function (similar to my watch() example) to make sure that everything runs exactly when needed (usually within 20-100 usec of the scheduled time).
There's always a better way!

robtillaart

Quote
(unit tests)... because I cannot control what state the "x" variable has at any particular time ...


agree, if you want that you should make a blink class that exposes the state.

That said, digitalWrite(13,x ^= 1);  could be rewritten as digitalWrite(13, !digitalRead(13));  and with digitalRead() the state can be read easily (OK takes some time)

Good educative example Morris, 5 points!
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Morris Dovey

@jwatte - Sometimes that's important and, fortunately, it's easy to move something like the x variable out of the function and make its scope global.

As a general practice I try to restrict the scope of static variables as much as I can to avoid the temptation to "fudge" a value from somewhere I shouldn't.   :smiley-mr-green:
There's always a better way!

bill2009

I like your code - thanks for sharing.

oric_dan

Quote

different approach to asynchronous program tasks


Good terse code Morris, but I don't quite see what is "different" about it. LOL.

[FWIW, I see it as a simpler example of the state-machine code I wrote in the prior thread
on this subject, and having exactly the same degree of modularization].

jwatte wrote:
Quote

I dislike static variables inside functions, because they make the state impossible to control.


That is the entire point of having state-machines, because they control their own state and timing, and
the rest of the program needn't worry about them. Afterall, that's what multitaskers do on their own
account, when you use their schedulers, etc.

OTOH, as Morris stated, you can make the static variables public, and have access to them, or do like
some of us did for decades before OOPs was even invented, ie, simply segregate them by limiting their
scope, and only allow access to them via use of specific read/write functions written for that purpose.
This way, you can access them during debugging/test, and ignore them afterwards.

IOW, you can use the OOP practice of data encapsulation without having a compiler that supports
OOP specifically. C always allowed that from day-one.







Coding Badly


Your code has a potential catastrophic failure when millis wraps.

Morris Dovey

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

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?
There's always a better way!

robtillaart

Quote
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"?
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Morris Dovey


I'm happy to share it...

Code: [Select]
/*----------------------------------------------------------------------------*/
/* 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();
}
/*----------------------------------------------------------------------------*/
There's always a better way!

oric_dan

codingbadly wrote:
Quote

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.


Morris Dovey

millis() wraps every 49d 17:02:47.295 and a 64-bit msec clock will wrap in only 584,542,046 years!   :smiley-mr-green:
There's always a better way!

oric_dan

@Morris:
Code: [Select]

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?

Go Up