Go Down

Topic: Bug in sleep.h (Read 1 time) previous topic - next topic

terraslate

Hello

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.

Regards

Terra

Coding Badly

#1
Jan 28, 2015, 09:54 am Last Edit: Jan 28, 2015, 09:54 am by Coding Badly
Quote
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

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


terraslate

avr/sleep. h
avr/power. h

etc

bobcousins

#3
Jan 29, 2015, 07:19 pm Last Edit: Jan 30, 2015, 09:25 am by bobcousins
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.
Please ask questions in the forum so everyone can benefit. PM me for paid work.

Coding Badly

...the macros it provides have exactly that problem.
"Macro" not "macros".  Only one is a problem.

Quote
...maybe you can add a new issue
Or, just avoid the troublesome macro.


bobcousins

"Macro" not "macros".  Only one is a problem.
Well done. How many points do you score for that? :)
Please ask questions in the forum so everyone can benefit. PM me for paid work.

Go Up