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:
#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 */
}
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.
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.
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).
(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)
@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.
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:
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 - 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?
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.
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.
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 */
}
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
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.
