[sgtnoodle]

The article's Delay function unfortunately has a rollover bug. If the counter rolls over in the middle of a delay, the delay will return early. It's a nefarious bug because it only occurs every 50 days, and only if the program is delaying. Also, while it's probably ok in this usage since it's a 32-bit cpu, it would be better to use atomic access intrinsics or a critical section and memory barriers rather than volatile to make the counter variable "ISR safe".

Embedded code is a lot of fun, but if you want to safely work on the "really fun" stuff it's important to maintain a high level of quality and correctness for fundamental but lame utility functions like this. Otherwise you could literally build a ticking time bomb!

[myself248]

Not saying this isn't a real bug (it definitely is!), but most quadcopters aren't powered on for 50 minutes at a time, let alone 50 days. Unlikely to make anything actually fall out of the sky.

[roland35]

This is a good point, although the author notes in the article that the battery will only last a few minutes you never know if this code might get copied into a different project!

What I would do in this particular case is make a note of the limitations in the function block comment. There are always trade-offs in embedded code so I think it is fair to say that yes this code does not handle rollover, FYI.

[emilfihlman]

>Also, while it's probably ok in this usage since it's a 32-bit cpu, it would be better to use atomic access intrinsics or a critical section and memory barriers rather than volatile to make the counter variable "ISR safe".

The dislike for volatile coming from people in higher systems is pretty tiring and unwarranted.

Volatile is a perfectly fine tool on low level mcus.

[fhars]

No, it is not, it will just work most of the time in low level MCUs because compilers support this abuse for lack of clearly better options. C and C++ have no support whatsoever for dealing with interrupts: volatile does not introduce any memory fences, atomics are for sharing data between threads which you do not have in many embedded systems, and sig_atomic_t is only defined for situations where you have a posix-like runtime environment. So sharing data between an ISR and the main program is undefined in C, and the compilers are free to compile „volatile“ the way most people expect, but the language does not guarantee that it works.

It can be argued that the only kind of variable that can be shared safely between an ISR and the rest of the code is a volatile sig_atomic_t, but that is an argument from analogy and not from first principles, as the C abstract machine has no concept of ISR. The same goes for the argument that you should use atomic intrinsics, plausible, but still just an analogy.

(Just in case anyone was wondering if C as a language is close to the metal: no, it is not.)