 |
 |
| View previous topic :: View next topic |
| Author |
Message |
Gerhard
Joined: 30 Aug 2007
Posts: 144
Location: South Africa
|
| Help with timing |
 Posted: Thu May 01, 2008 5:42 pm |
 |
|
I am struggling to get the right timing in my program. Acording to my calculations the following should be true.
| Quote: | 4
-----------=1us/clock cycle
(4000000)
rtcc_div_4 on 8 bit timer thus 4us/count and 1024us/overflow or interupt aproximately every 1ms |
However in the following program the timing seems to be out by a factor of 10
| Code: | #include <16F886.H>
#fuses INTRC, NOWDT, NOPROTECT, BROWNOUT, PUT, NOLVP
#use delay(clock = 4000000)
#include <Flex_LCD420.c>
boolean msBool = false;
unsigned int sec_counter = 0;
float x=0,ax=0,vx=0,ux=0,y=0,z=0;
int xa[5]={0};
int ya[5]={0};
int za[5]={0};
void setup()
{
set_TRIS_A(0xff);
setup_adc_ports(sAN0|sAN1|sAN2);
setup_adc( ADC_CLOCK_DIV_8 );
setup_timer_0(RTCC_INTERNAL | RTCC_DIV_4);
set_rtcc (8);
set_timer0(0);
enable_interrupts(INT_TIMER0);
enable_interrupts(global);
lcd_init();
}
void clearLCD()
{
printf(lcd_putc, "\f");
delay_ms(500);
}
void getvalue();
void calculate();
void updateLCD();
void calculate();
//===================================
void main()
{
setup(); //PIC setup
clearLCD();
while(1)
{
if(msBool == true)
{
getvalue();
calculate();
msBool = false;
sec_counter = sec_counter+1;
}
if(sec_counter==100)////should be 1000 for a second but somewhere i am missing a factor of 10
{
updateLCD();
sec_counter=0;
}
}
}
// Timer ISR overflows every 1 ms
#INT_TIMER0 // This function is called every time
void clock_isr()
{
set_timer0(0);
msBool=true;
|
}
In the main program i get a sec_count every second where it should be every 0.1s.
Can someone spot where i am missing it?? |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Thu May 01, 2008 6:22 pm |
|
|
| Quote: | if(msBool == true)
{
getvalue();
calculate();
msBool = false;
sec_counter = sec_counter+1;
} |
How long do these two functions take to execute ? Are interrupts
disabled in those functions ?
The incrementing of the seconds counter really should be done inside
the RTCC isr. Then it won't be dependent upon the execution time of
other code.
Also, your comments state that sec_counter should be 1000. But you've
declared it as an 'unsigned int', which can only hold 0 to 255 in CCS.
You need to make it be an int16 if you want it to count to 1000. Then
when you read the int16 in main(), you need to temporarily disable
interrupts.
| Quote: | #INT_TIMER0
void clock_isr()
{
set_timer0(0);
msBool=true;
}
|
The line in bold is not needed. The RTCC counts up and automatically
rolls over from 0xFF to 0x00 when the interrupt occurs. You don't need
to manually load it with 0x00.
| Quote: | set_rtcc (8);
set_timer0(0); |
These two functions do the same thing. They are just two different
names for the same function. You only need one of them. |
|
|
Gerhard
Joined: 30 Aug 2007
Posts: 144
Location: South Africa
|
|
| Posted: Fri May 02, 2008 6:58 am |
|
|
I cahnged what you recomended PCM Programmer and I am also now using an external oscilator at 20Mhz. However my timing still remains wrong. I dont like posting the whole code as it is always a struggle to go through code but i am running out of ideas so if someone can just look at my code and see if a mistake can be spotted.
| Code: | #include <16F886.H>
#fuses XT, NOWDT, NOPROTECT, BROWNOUT, PUT, NOLVP
#use delay(clock = 20000000)
#include <Flex_LCD420.c>
boolean msBool = false;
boolean sBool = false;
int16 sec_counter = 0;
float x=0,ax=0,vx=0,ux=0,y=0,z=0,s=0,su=0;
int xa[5]={0};
int ya[5]={0};
int za[5]={0};
void setup()
{
set_TRIS_A(0xff);
setup_adc_ports(sAN0|sAN1|sAN2);
setup_adc( ADC_CLOCK_DIV_8 );
setup_timer_0(RTCC_INTERNAL | RTCC_DIV_32);
set_rtcc (100);
enable_interrupts(INT_TIMER0);
enable_interrupts(global);
lcd_init();
}
void clearLCD()
{
printf(lcd_putc, "\f");
delay_ms(1000);
}
void getvalue();
void calculate();
void updateLCD();
void calculate();
//===================================
void main()
{
setup(); //PIC setup
clearLCD();
while(1)
{
if(msBool == true)
{
getvalue();
calculate();
msBool = false;
}
if(sec_counter==1000)////should be 1000 for a second but somewhere i am missing a factor of 10
{
sec_counter = 0;
updateLCD();
}
}
}
// Timer ISR overflows every 1 ms
#INT_TIMER0
void clock_isr()
{
msBool=true;
sec_counter=sec_counter+1;
}
void getvalue()
{
int i;
x=0;
for(i=4;i>0;i--)
{
xa[i]=xa[i-1];
}
set_adc_channel(0); //X direction
delay_us(20);
xa[0]= read_adc();
for(i=0;i<5;i++)
{
x = xa[i]+x;
}
x = x*0.2;
y=0;
for(i=4;i>0;i--)
{
ya[i]=ya[i-1];
}
set_adc_channel(1); //X direction
delay_us(20);
ya[0]= read_adc();
for(i=0;i<5;i++)
{
y = ya[i]+y;
}
y = y*0.2;
z=0;
for(i=4;i>0;i--)
{
za[i]=za[i-1];
}
set_adc_channel(2); //X direction
delay_us(20);
za[0]= read_adc();
for(i=0;i<5;i++)
{
z = za[i]+z;
}
z = z*0.2;
}
void calculate()
{
if(x<84&&x>80)
{
ax = 0;
}
else if(x>83||x<81)
{
ax=(x-81)*0.58;
}
vx = ux+ax*0.01;
//s= su*1 + 0.5*ax;
//su = s;
ux = vx;
}
void updateLCD()
{
lcd_gotoxy(1,1);
printf(lcd_putc, "An");
lcd_gotoxy(5,1);
printf(lcd_putc, "Acc");
lcd_gotoxy(11,1);
printf(lcd_putc, "Speed");
lcd_gotoxy(17,1);
printf(lcd_putc, "Disp");
lcd_gotoxy(1,2);
printf(lcd_putc,"%3.0f", x);
lcd_gotoxy(6,2);
printf(lcd_putc,"%3.0f",ax);
lcd_gotoxy(12,2);
printf(lcd_putc,"%3.0f",vx);
lcd_gotoxy(17,2);
printf(LCD_PUTC,"%3.0f",s);
lcd_gotoxy(1,3);
printf(lcd_putc,"%3.0f",y);
lcd_gotoxy(6,3);
printf(lcd_putc,"%3.0f",x);
lcd_gotoxy(12,3);
printf(lcd_putc,"%3.0f"x);
lcd_gotoxy(17,3);
printf(LCD_PUTC, "%3.0f",x);
lcd_gotoxy(1,4);
printf(lcd_putc,"%3.0f", z);
lcd_gotoxy(6,4);
printf(lcd_putc,"%3.0f", x);
lcd_gotoxy(12,4);
printf(lcd_putc,"%3.0f",x);
lcd_gotoxy(17,4);
printf(LCD_PUTC, "%3.0f",x);
} |
|
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Fri May 02, 2008 8:27 am |
|
|
I don't see an obvious bug that explains a factor 10 speed error.
A call to setup_wdt() would explain the Timer0 prescaler to be changed but this is not in your code.
What is your compiler version?
| Code: | if(sec_counter==1000)
{
sec_counter = 0;
... other stuff
} |
Change the '==' to '>='. Your main loop is running asynchronous to the interrupts and there is a small chance the counter will be increased to a value >1000 before you reach this test.
Another small error risk is that while you are evaluating sec_counter a timer interrupt occurs messing up the comparison. Sec_counter is a 2-byte value and hence it will take the 8-bit PIC processor multiple instructions to execute the compare, best is to momentarily disable the timerinterrupt: | Code: | disable_interrupts(INT_TIMER0);
if(sec_counter>=1000)
{
sec_counter = 0;
enable_interrupts(INT_TIMER0);
... other stuff
}
else
enable_interrupts(INT_TIMER0); |
| Code: | if(x<84&&x>80)
{
ax = 0;
}
else if(x>83||x<81) <== will always be true
{
ax=(x-81)*0.58;
} |
The if statement after the else will always be true and can be removed. In the first if-statement you test the range 81-83, so the second test will only be reached for values outside of this range, no need to do the test again.
| Code: | | setup_adc( ADC_CLOCK_DIV_8 ); |
Check table 9-1 of the datasheet. At 20MHz you should use ADC_CLOCK_DIV_32 for correct conversions.
By default the CCS compiler will take care of setting the TRIS register for you so this line can be omitted (it might even mess with the compiler's internal bookkeeping). For speed intensive code tt is possible to manually control the TRIS register by setting '#use fast_io'.
| Code: | int xa[5]={0};
int ya[5]={0};
int za[5]={0}; |
This will only set the first member of the array to zero, all other values will be random. This is going to mess up your averaging algorithm for the first 4 readings. Try {0,0,0,0,0} instead. |
|
|
Gerhard
Joined: 30 Aug 2007
Posts: 144
Location: South Africa
|
|
| Posted: Fri May 02, 2008 8:44 am |
|
|
| The getvalue function is one i wrote myself and is included in the shown program. Maybe i am misunderstanding you but i am not using any getc() function. Only the function getvalue which is the one i wrote to get the analog value placed in an array and then to find the moving average of the array. |
|
|
Ttelmah
Guest
|
|
| Posted: Fri May 02, 2008 9:18 am |
|
|
A couple of other comments:
If you have switched to an external 20MHz source, is this an _oscillator_ (module needing power), or a crystal?. If the former, your clock fuse should change to 'EC', if the latter, it should change to HS. You say 'oscillator', but a lot of people here, mean crystal, when they say this. If it is a crystal, there is a good chance that it is running on a lower harmonic, with the wrong fuse selected.
The timing calculation is screwed, but not by a factor of 10.
At 20MHz, with /32 prescale, the interrupt will occur every 1.6384mSec. This gives 610 counts per second, rather than 1000.
Personally, why not switch to doing this count in a int8. If you increase the prescale to /128, and count to 152, this reduces the work that has to be done in the interrupt (only occurs at 1/4 the frequency), and removes the need to disable interrupts while checking.
So, the interrupt code becomes:
| Code: |
int8 tick;
// Timer ISR overflows every 1 ms
#INT_TIMER0
void clock_isr() {
msBool=true;
if (tick) --tick;
}
//and the loop test becomes:
while(1) {
if(msBool) {
//reset the msbool value as soon as possible
msBool=false;
getvalue();
calculate();
if(sec_counter==0) {
sec_counter = 152;
updateLCD();
}
}
}
|
There is no point in testing 'sec_counter', unless msBool has updated.
Now, the getval function, is going to take a _long_ time. You have three 20uSec delays, plus significant arithmetic, including some that is floating point, and a lot of array ccesses. I'd 'guess' at several thousand uSec in total. It may well be, that this is taking longer than is available between your ticks.
Use a different approach. Maintain a rolling average, updating it each time through the code, with binary arithmetic. So, something like:
| Code: |
void getvalue(void) {
int i;
static int16 adsumx=0,adsumy=0,adsumz=0;
set_adc_channel(0); //X direction
delay_us(5); //The Tacq for this ADC, is under 5uSec
adsumx+=read_adc();
set_adc_channel(1); //X direction
//Perform the averaging, rather than a delay, to save some time
x=adsumx/8;
adsumx-=x;
delay_us(3);
adsumy+=read_adc();
set_adc_channel(2);
y=adsumx/8;
adsumy-=y;
delay_us(3);
adsumz+=read_adc();
z=adsumx/8;
adsumy-=z;
}
|
This should take a tiny fraction of the time needed for your 'getval', and maintain a good average.
Consider if you can get rid of the floating point arithmetic totally. I'm sure that you could use scaled integers instead of the FP, in the calculate routine as well. This makes a _huge_ speed difference.
Best Wishes |
|
|
Gerhard
Joined: 30 Aug 2007
Posts: 144
Location: South Africa
|
|
| Posted: Tue May 06, 2008 1:08 pm |
|
|
Hi Ttelmah.
Would you please explain this part of the averaging that you recommend doing, and why is it adsumx/8. I am trying to use less processing time so i will be trying all.
| Code: | x=adsumx/8;
adsumx-=x; |
I am using a crystal so i changed the fuse to HS but if i place a delay of around 1000ms it remains very much longer than 1s. I am using two 105 caps on the crystal but i am not sure what is the recommended type to use. |
|
|
Ttelmah
Guest
|
|
| Posted: Tue May 06, 2008 2:44 pm |
|
|
Eeek.
105 caps, are about 4.5 orders of magnitude too large.....
You need to check the specification for your crystal. However typical values will be something like 22pF to 30pF. Do a search here, for how to calculate the capacitor for a given loading spec. I have posted it at least four times in the relatively recent past...
The point about my code, is that divisions by a binary number (2, 4, 8), are performed on an integer, by simply shifting the value. Only takes a couple of machine instructions, against hundreds for divisions by a 'normal' value. It also only performs one ADC reading on the loop, but maintains a 'rolling' average, based on the past readings taken.
Best Wishes |
|
|
Gerhard
Joined: 30 Aug 2007
Posts: 144
Location: South Africa
|
|
| Posted: Tue May 06, 2008 2:52 pm |
|
|
I had a misunderstanding with the caps but i placed the correct ones in and the timing seems to be working much better. Guess another schoolboy error can cause an ugly headache. Your code for the getvalue function works very nicely but i am struggling to understand what you are doing in regard to the rolling average.
| Code: | int i;
static int16 adsumx=0,adsumy=0,adsumz=0;
set_adc_channel(0); //X direction
delay_us(5); //The Tacq for this ADC, is under 5uSec
adsumx+=read_adc();
set_adc_channel(1); //Y direction
x=adsumx/8; /// This is unclear as i dont understand why division by 8??
adsumx-=x; // This is also unclear why you set adsumx = adsumx-x
delay_us(3);
adsumy+=read_adc();
set_adc_channel(2);
y=adsumx/8;
adsumy-=y;
delay_us(3);
adsumz+=read_adc();
z=adsumx/8;
adsumy-=z; |
|
|
|
Ttelmah
Guest
|
|
| Posted: Tue May 06, 2008 3:43 pm |
|
|
OK.
adcsum, is static. Kept all the time.
You take a reading (say 500), and it is added to this sum (starting at zero), so it is now 500 You then take the sum/8, to get 62 as the first 'damped' reading.
This is then subtracted from the sum.
Repeating through the loop (with the adc reading 500 each time), you get:
62
117
165
207
243
275
Basically, the sum, 'closes' on the incoming reading, at a rate of 1/8th of the 'difference' between the actual reading, and the last average returned. Now, drop the division, and the closure is faster (twice as fast).
It gives a 'chasing' average. Instead of having to take a whole load of readings, you just have the 'historical' sum of all the readings in the past, less all the damped values returned. With just /2 (lowest possible damping), it'll go:
250
375
437
469
484
492
496
and take 9 samples to 'close' to the 500 value (may be damped enough for you). However the really good things about it, are that it tends to damp larger changes more than a simple sum and divide, while requiring a _lot_ less arithmetic and time. This makes it a very good algorithm for noise rejection on typical 'sensor' signals.
Best Wishes |
|
|
Gerhard
Joined: 30 Aug 2007
Posts: 144
Location: South Africa
|
|
| Posted: Tue May 06, 2008 4:28 pm |
|
|
Thanks Ttelmah.
This is an excelent algorithm which will definitely be used in the future.
Do you know of any good site that would be able to tell me in how many machine cycles a certain instruction takes to excecute? And is there a function that can be used to determine the time a set of instructions takes to excecute. For instance at the moment i read a new analog value every 1ms and that is the time i use to change the analog value which is an acceleration to a speed and displacement. Instead of using a fixed value i would like to use the actual time value between measurents to find the correct speed and displacement. Such as in the formula
speed = previousspeed + acceleration*time and
displacement = previous displacement + speed*time
| Code: | vx = ux+ax*0.001;
ux = vx;
s= su + vx*0.001;
su = s;
ux = vx; |
|
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
