Page 1 of 1
BUG: chThdSleepMilliseconds() sleeps 1 clock cycle too short
Posted: Mon Nov 25, 2013 8:05 pm
by positron
Hi,
as I understand, both chThdSleepMilliseconds(x) and ..Microseconds(x) are supposed to sleep AT LEAST the time x.
The actual time should be between x and x+"1 clock cycle" (which might typically be 1ms).
However, what we find when measuring with an oscilloscope is that chThdSleepMilliseconds(1) sleeps between 0 and 1 msec.
I guess, this is a "< vs. <=" or "+1 missing" type of bug in ChibiOS.
(Platform: STM32F407, kernel 2.5.2unstable)
Re: BUG: chThdSleepMilliseconds() sleeps 1 clock cycle too s
Posted: Mon Nov 25, 2013 8:46 pm
by Giovanni
Hi,
The behavior is the expected one, the system is tick based so the next tick counts as 1. If you need to work with a guaranteed minimum the you have to add a +1 externally.
chSysSleep(1) is commonly used to align to the next tick boundary, zero is a reserved constant (TIME_IMMEDIATE) and is used as timeout specification by several other primitives.
Giovanni
Re: BUG: chThdSleepMilliseconds() sleeps 1 clock cycle too s
Posted: Mon Nov 25, 2013 9:37 pm
by positron
Thanks for your quick reply.
The documentation says: "The specified time is rounded up to a value allowed by the real system tick clock."
To me, this means what I just described above: That the delay can be longer but not shorter.
This is also what just about everybody will expect. An API like that which does delays shorter than specified is prone to introduce bugs on the application side.
However, if this is expected behavior, please clearly state in the documentation that a delay X is at least X - 1 tick long.
Programmers will assume from lots of other systems, that delays are a guaranteed minimum (see usleep(), etc.)
Re: BUG: chThdSleepMilliseconds() sleeps 1 clock cycle too s
Posted: Mon Nov 25, 2013 9:45 pm
by Giovanni
The rounding up is done while converting from time to system ticks. So I have a 100Hz system tick and I specify 1mS, 1mS is rounded up to 10mS = 1 system tick.
The correct way to add that +1 would be:
chThdSleep(S2ST(secs) + 1)
This way the rounding is added after the conversion in system ticks, one tick is added instead of a whole second.
Giovanni
Re: BUG: chThdSleepMilliseconds() sleeps 1 clock cycle too s
Posted: Mon Nov 25, 2013 10:06 pm
by positron
Thanks, I found out the correct way to do things immediately after my post so I edited the post to remove the second question.
But please clarify the docu!
I introduced:
// Do not use chThdSleepMilliseconds()/Microseconds() because these functions
// will in fact sleep at least 1 clock cycle LESS then specified. To get
// reliable minimum delays, use these:
#define chThdSleepMsec(msec) chThdSleep(MS2ST(msec)+1)
#define chThdSleepUsec(usec) chThdSleep(US2ST(usec)+1)