go_away

Author Topic: Bug in delay_us function (timer640.c)?  (Read 2012 times)

0 Members and 1 Guest are viewing this topic.

Offline JadeKnightTopic starter

  • Jr. Member
  • **
  • Posts: 8
  • Helpful? 0
Bug in delay_us function (timer640.c)?
« on: September 24, 2009, 01:33:32 AM »
In timer640.c:
Current version:
Code: [Select]
void delay_us(unsigned short time_us)
{
unsigned short delay_loops;
register unsigned short i;

delay_loops = (time_us+3)/5*CYCLES_PER_US; // +3 for rounding up (dirty)

// one loop takes 5 cpu cycles
for (i=0; i < delay_loops; i++) {};
}
The delay_loops calculation appears to be incorrect. If you pass in 1us, you get a delay_loops of 0 (1+3=4, 4/5=0, 0*16=0). For the Axon you really want something close to 16 cycles, or 3 loops (provided the 5 cycle per loop comment holds true).

Proposed new version:
Code: [Select]
void delay_us(unsigned short time_us)
{
unsigned short delay_loops;
register unsigned short i;

delay_loops = ((time_us * CYCLES_PER_US)+3) / 5; // +3 for rounding up (dirty)

// one loop takes 5 cpu cycles
for (i=0; i < delay_loops; i++) {};
}
With this version, if you pass in 1us, you get a delay_loops of 3 (1*16=16, 16+3=19, 19/5=3), which equates to 15 cycles (closest possible to the 16 cycles per us on the Axon).

Is this correct, or am I missing something?

Offline Admin

  • Administrator
  • Supreme Robot
  • *****
  • Posts: 11,666
  • Helpful? 169
    • Society of Robots
Re: Bug in delay_us function (timer640.c)?
« Reply #1 on: September 24, 2009, 05:18:17 PM »
I took the delay_us code directly from AVRlib.

I never measured the accuracy . . . do you have an oscope to try both?

Offline Webbot

  • Expert Roboticist
  • Supreme Robot
  • *****
  • Posts: 2,166
  • Helpful? 111
Re: Bug in delay_us function (timer640.c)?
« Reply #2 on: September 24, 2009, 05:28:42 PM »
In timer640.c:
Current version:
Code: [Select]
void delay_us(unsigned short time_us)
{
unsigned short delay_loops;
register unsigned short i;

delay_loops = (time_us+3)/5*CYCLES_PER_US; // +3 for rounding up (dirty)

// one loop takes 5 cpu cycles
for (i=0; i < delay_loops; i++) {};
}
The delay_loops calculation appears to be incorrect. If you pass in 1us, you get a delay_loops of 0 (1+3=4, 4/5=0, 0*16=0). For the Axon you really want something close to 16 cycles, or 3 loops (provided the 5 cycle per loop comment holds true).

Proposed new version:
Code: [Select]
void delay_us(unsigned short time_us)
{
unsigned short delay_loops;
register unsigned short i;

delay_loops = ((time_us * CYCLES_PER_US)+3) / 5; // +3 for rounding up (dirty)

// one loop takes 5 cpu cycles
for (i=0; i < delay_loops; i++) {};
}
With this version, if you pass in 1us, you get a delay_loops of 3 (1*16=16, 16+3=19, 19/5=3), which equates to 15 cycles (closest possible to the 16 cycles per us on the Axon).

Is this correct, or am I missing something?

Your original formula for the existing code fails to take into account how long it takes to perform:
Code: [Select]
delay_loops = (time_us+3)/5*CYCLES_PER_US;
Plus the overhead of it being in a subroutine ie pushing/popping various registers at the start/end of the code

Edit:- Oh, and if you have other interrupts going on for Uarts, Timers etc then the time taken to service those interrupts will also get added on.

Just a thought
« Last Edit: September 24, 2009, 05:38:01 PM by Webbot »
Webbot Home: http://webbot.org.uk/
WebbotLib online docs: http://webbot.org.uk/WebbotLibDocs
If your in the neighbourhood: http://www.hovinghamspa.co.uk

Offline JadeKnightTopic starter

  • Jr. Member
  • **
  • Posts: 8
  • Helpful? 0
Re: Bug in delay_us function (timer640.c)?
« Reply #3 on: September 25, 2009, 10:00:55 AM »
I don't have an oscope. I'm just going by the code and the comments.

Not counting the valid timing concerns Webbot mentioned, these are the results of the two different functions.

input  delay_loops   Cycles (5*delay_loops)   Theoretical us (16Mhz = cycles/16)
us      old / new      old / new               old / new
1    =    0 / 3          0 / 15                  0 / 0.9375
2    =   16 / 7         80 / 35                  5 / 2.1875
3    =   16 / 10        80 / 50                  5 / 3.125
4    =   16 / 13        80 / 65                  5 / 4.0625
5    =   16 / 16        80 / 80                  5 / 5
6    =   16 / 19        80 / 95                  5 / 5.9375
7    =   32 / 23       160 / 115                10 / 7.1875

The current code only seems to be valid for rounded multiples of 5us. The new code would have an overflow problem for values > 4095 (at 16MHz), but at that point it would be better to call delay_ms for any multiples of 1000us.
« Last Edit: September 25, 2009, 10:11:34 AM by JadeKnight »

Offline Admin

  • Administrator
  • Supreme Robot
  • *****
  • Posts: 11,666
  • Helpful? 169
    • Society of Robots
Re: Bug in delay_us function (timer640.c)?
« Reply #4 on: October 03, 2009, 12:31:47 PM »
I updated the file based on the new delay_us() suggestion. I didn't test it however as I think my AVR Programmer just fried . . .

It'll be included in the next update.

 


Get Your Ad Here