Pages: [1]   Go Down
Author Topic: found bug in delayMicroSeconds when called with 0;  (Read 2067 times)
0 Members and 1 Guest are viewing this topic.
Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 211
Posts: 13488
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The delayMicroseconds function fails when called with a value of 0.
Although this is not done normally, it can happen when it is called with a variable which is the result of some expression/calculation.

The bug can be traced back to the if (--us ==0) statement, which causes an overflow when the param us is 0. The delay is then far longer (about 16384 micros) .

BUG seen in IDE 0.22 + 1.0.0 + 1.0.2

Find test program and patch below : NOTE the patch does not include the 1.0.2 20Mhz addition as I cannot test it .

The patched version of the delayMicroseconds() will return asap when called with 0 . This still takes about 1.2 uSec but that is way better than 16K uSec smiley

Q: can anybody confirm this bug in a 20Mhz duino?

Program to show bug
Code:
//
//    FILE: delayMicrosecondsBUG.pde
//  AUTHOR: Rob Tillaart
//    DATE: 2012-11-18
//
// PUPROSE: test delayMicroseconds()
//

void setup()
{
  Serial.begin(9600);
  Serial.println("start...");
  
  unsigned long m = micros();
  for (uint8_t i=0; i<100; i++)
  {
    delayMicroseconds(0);
  }
  Serial.println(micros()- m);

  m = micros();
  for (uint8_t i=0; i<100; i++)
  {
    delayMicroseconds(1);
  }
  Serial.println(micros()- m);

  m = micros();
  for (uint8_t i=0; i<100; i++)
  {
    delayMicroseconds(2);
  }
  Serial.println(micros()- m);
  
  m = micros();
  delayMicroseconds(20);
  Serial.println(micros()- m);
  m = micros();
  delayMicroseconds(200);
  Serial.println(micros()- m);
  m = micros();
  delayMicroseconds(2000);
  Serial.println(micros()- m);
}

void loop()
{}

====================
Find below a patch for the function (for 0.22 - 1.0.0 version):

Code:
void delayMicroseconds(unsigned int us)
{
// calling avrlib's delay_us() function with low values (e.g. 1 or
// 2 microseconds) gives delays longer than desired.
//delay_us(us);

#if F_CPU >= 16000000L
// for the 16 MHz clock on most Arduino boards

// for a one-microsecond delay, simply return.  the overhead
// of the function call yields a delay of approximately 1 1/8 us.
//  PATCH
// if (--us == 0)
// return;
if (us < 2) return;
us--;

// the following loop takes a quarter of a microsecond (4 cycles)
// per iteration, so execute it four times for each microsecond of
// delay requested.
us <<= 2;

// account for the time taken in the preceeding commands.
us -= 2;
#else
// for the 8 MHz internal clock on the ATmega168

// for a one- or two-microsecond delay, simply return.  the overhead of
// the function calls takes more than two microseconds.  can't just
// subtract two, since us is unsigned; we'd overflow.
// PATCHED LINES
// if (--us == 0)
// return;
// if (--us == 0)
// return;
if (us < 3) return;
us -= 2;

// the following loop takes half of a microsecond (4 cycles)
// per iteration, so execute it twice for each microsecond of
// delay requested.
us <<= 1;
    
// partially compensate for the time taken by the preceeding commands.
// we can't subtract any more than this or we'd overflow w/ small delays.
us--;
#endif

// busy wait
__asm__ __volatile__ (
"1: sbiw %0,1" "\n\t" // 2 cycles
"brne 1b" : "=w" (us) : "0" (us) // 2 cycles
);
}

TODO: post on the issue list yet ...
« Last Edit: November 18, 2012, 05:26:17 am by robtillaart » Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 211
Posts: 13488
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Reported as #1121 on  - https://github.com/arduino/Arduino/issues?state=open -
Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 211
Posts: 13488
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

a more generic patch might work for the 20Mhz too    (values smaller than 2 are exceeded by function call overhead)

Code:
void delayMicroseconds(unsigned int us)
{
  if (us <2) return;
...
Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Left Coast, CA (USA)
Online Online
Brattain Member
*****
Karma: 361
Posts: 17262
Measurement changes behavior
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


It will be interesting to see if the arduino developers will consider this a bug or a feature as they already note the minimum value on can use will accurate results. I the reference for delayMicroseconds:

Quote
Caveats and Known Issues
This function works very accurately in the range 3 microseconds and up. We cannot assure that delayMicroseconds will perform precisely for smaller delay-times.


Lefty
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 211
Posts: 13488
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi Lefty,

Rationale for that statement is the fact that a function call takes approx 1 usec. Checking the param takes some clock cycles too. So before one can start to count down, too much time may have passed already (for small values). The root cause is that one is near the cpu speed.

But mapping 0 usec on 16K usec is a serious Bug - with capital B smiley-wink

For the Due which is 84(?) Mhz I expect a re-implementation for this function. However no experience yet with the Due.
Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Left Coast, CA (USA)
Online Online
Brattain Member
*****
Karma: 361
Posts: 17262
Measurement changes behavior
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi Lefty,

Rationale for that statement is the fact that a function call takes approx 1 usec. Checking the param takes some clock cycles too. So before one can start to count down, too much time may have passed already (for small values). The root cause is that one is near the cpu speed.

But mapping 0 usec on 16K usec is a serious Bug - with capital B smiley-wink

For the Due which is 84(?) Mhz I expect a re-implementation for this function. However no experience yet with the Due.

I believe there is also a minimum 4 usec step size resolution for the delayMicroseconds function as implemented in the arduio AVR platform? So it's not capable of 1 usec resolution timing delays as one might assume otherwise even at values greater then 3?

Lefty
« Last Edit: November 18, 2012, 12:30:46 pm by retrolefty » Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 211
Posts: 13488
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The delayMicroseconds() implementation ends with
Code:
// busy wait
__asm__ __volatile__ (
"1: sbiw %0,1" "\n\t" // 2 cycles
"brne 1b" : "=w" (us) : "0" (us) // 2 cycles
which is a tight loop decrementing the var us. So I think the timing is quite precise for values > 2.  This loop takes 4 cycles so it is executed 4 times per usec (16Mhz assumed).

The stepsize of 4 usec is the precision of the micros() function so if you want to measure time smaller than 4 usec you need a HW timer solution. Can't find the link now.
Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Left Coast, CA (USA)
Online Online
Brattain Member
*****
Karma: 361
Posts: 17262
Measurement changes behavior
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The delayMicroseconds() implementation ends with
Code:
// busy wait
__asm__ __volatile__ (
"1: sbiw %0,1" "\n\t" // 2 cycles
"brne 1b" : "=w" (us) : "0" (us) // 2 cycles
which is a tight loop decrementing the var us. So I think the timing is quite precise for values > 2.  This loop takes 4 cycles so it is executed 4 times per usec (16Mhz assumed).

The stepsize of 4 usec is the precision of the micros() function so if you want to measure time smaller than 4 usec you need a HW timer solution. Can't find the link now.

Thanks for that.
 Yes, I was mixing up micros() with delayMicroseconds(). Put then again I'm old, so I'm allowed a certain amount of 'brain farts' per fortnight, as I continue to chase kids off my lawn.  smiley-wink

Lefty
Logged

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 197
Posts: 12744
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Find test program and patch below : NOTE the patch does not include the 1.0.2 20Mhz addition as I cannot test it .

The what?
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 211
Posts: 13488
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

@CB
Quote
The what?
The 1.0.2 version of wiring.c has additional code to implement this delayMicroseconds() function correctly(?) for a 20Mhz arduino.
As I do not have such a thing I cannot test it.

Clear now?
« Last Edit: November 20, 2012, 01:44:47 pm by robtillaart » Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 197
Posts: 12744
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset


Got it.  Thanks.
Logged

Global Moderator
Netherlands
Offline Offline
Shannon Member
*****
Karma: 211
Posts: 13488
In theory there is no difference between theory and practice, however in practice there are many...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

did some rework and fine tuning of the delayMicroseconds code, see  - https://github.com/arduino/Arduino/issues/1121 -

Resulted in more accurate timing for values up to 100uSeconds (above not tested)

Code:
/* Delay for the given number of microseconds.  Assumes a 8 or 16 MHz clock. */
void delayMicroseconds(unsigned int us)
{
    // calling avrlib's delay_us() function with low values (e.g. 1 or
    // 2 microseconds) gives delays longer than desired.
    // delay_us(us);

#if F_CPU == 20000000L
    // for the 20 MHz clock on rare Arduino boards

    // handle 0 and 1
    if (us == 0) return;
    __asm__ __volatile__ (
        "nop" "\n\t"
        "nop" "\n\t"
        "nop" "\n\t"
        "nop");
    if (us == 1) return;

    // calc # iterations
    us = (us<<2) + us; // x5 us

    // account for the time taken in the preceeding commands.
    us -= 7;

#elif F_CPU == 16000000L
    // for the 16 MHz clock on most Arduino boards

    // handle 0 and 1
    if (us == 0) return;
    if (us == 1) return;

    // calc # iterations
    us <<= 2;
    // account for the time taken in the preceeding commands.
    us -= 5;

#elif F_CPU == 8000000L
    // for the 8 MHz internal clock on the ATmega168

    // handle 0, 1;
    if (us < 2) return;
    // handle 2;
    if (us == 2) return;

    // calc # iterations
    us <<= 1;

    // account for the time taken in the preceeding commands.
    us -= 5;
    // quarter uSecond adjust
    __asm__ __volatile__ (
        "nop\n\t"
    );

#elif F_CPU == 1000000L

// TODO 1 MHz
#error "delayMicroseconds: 1 MHz not supported yet"

# else

#error "delayMicroseconds: not supported clockspeed"

#endif

    // busy wait
    __asm__ __volatile__ (
        "1: sbiw %0,1" "\n\t" // 2 cycles
        "brne 1b" : "=w" (us) : "0" (us) // 2 cycles
    );
}

16MHz now gives the following output:

delayMicroseconds(0) => 0.81920 ==> inf
delayMicroseconds(1) => 1.01880 ==> 0.01880
delayMicroseconds(2) => 2.02280 ==> 0.01140
delayMicroseconds(3) => 3.02880 ==> 0.00960
delayMicroseconds(4) => 4.03520 ==> 0.00880
delayMicroseconds(5) => 5.04080 ==> 0.00816
delayMicroseconds(10) => 10.07080 ==> 0.00708
delayMicroseconds(20) => 20.13200 ==> 0.00660
delayMicroseconds(50) => 50.31200 ==> 0.00624
delayMicroseconds(100) => 100.61360 ==> 0.00614

Last column is the relative error (inf due to divide by zero) and it is decreasing which is good. The value for DMS(0) is as good as it gets.
Logged

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Pages: [1]   Go Up
Jump to: