Bug in sleep.h


I'm new here, but i've just started my first project and ran into a few issues which i think i've now ironed out. According to the spec of the atmel mega and tiny processors after calling sli() one following instruction is guaranteed to run.

In the case of sleep_mode in sleep.h i noted the implementation is as follows:

sleep_enable(); \ sleep_cpu(); \ sleep_disable(); \

sleep_cpu is the "second" instruction and i am supposing that these map one for one at compile time down to a single asm instruction. Surely that's a timing bug right there, since now you can accidentally trigger an interrupt before the sleep occurs. I'm not sure what the process is for writing these libraries that come included so except for manually fixing it myself not sure what i can do.

I beleive the older examples that didn't use sleep_mode and directly use sli(), sleep_cpu(); are far more clear.

Unless anyone else knows otherwise why that sleep_enable needs to be before sleep_cpu when all examples i have read show the following:

... cli(); sleep_enable(); //maybeAttachSomeWakeInterruptsHere sli(); sleep_mode() //sleeping sleep_disable();

then i'll continue with my modified header.



Surely that's a timing bug right there...

Correct. From most applications an sei / sleep_mode sequence is a timing related bug (a "race condition").

The code snippets in the Libc documentation show the correct way to put the processor to sleep (sei immediately followed by sleep_cpu)... http://www.nongnu.org/avr-libc/user-manual/group__avr__sleep.html

I'm not sure what the process is for writing these libraries that come included...

Which libraries?

avr/sleep. h avr/power. h


sleep.h is part of AVR GCC, it's not a library provided by Arduino as such.

Sleep.h talks about how to use the macros in the comments of sleep.h, but the puzzling thing, after saying "if you do it like this there is a race condition", considering the set of the macros provided jointly or severally they have exactly that problem.

I think Arduino do maintain their own fork of AVR GCC, so maybe you can add a new issue to the long list of outstanding issues and maybe one day someone will look at it. It may well be already on the list... [or just not bother and leave it for the next guy to stumble over]

Edit; added important corrections to prevent severe misunderstanding; equipment damage; loss of life etc.

bobcousins: ...the macros it provides have exactly that problem.

"Macro" not "macros". Only one is a problem.

...maybe you can add a new issue

Or, just avoid the troublesome macro.

[quote author=Coding Badly link=msg=2066230 date=1422557711] "Macro" not "macros". Only one is a problem. [/quote]

Well done. How many points do you score for that? :)