CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

timer1 overflow stupid mistake in reciprocal frequency count

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
Athar Rasul



Joined: 19 Mar 2010
Posts: 13

View user's profile Send private message

timer1 overflow stupid mistake in reciprocal frequency count
PostPosted: Sat Apr 14, 2012 1:14 pm     Reply with quote

I've been working on this reciprocal frequency counter project on PIC16F877A with 16MHz intended to be a 0-400 Hz counter. CCP1 is utilized to capture RE of incoming signal and ticks are counted. It is doing fine with high frequencies but when frequency gets very low (below 20Hz), the counter overflows. I know this shouldn't be any problem as I should be able to keep track of overflows in another variable and then calculating the final counter value but despite of doing that, I still can't get a close enough reading. Here are the code snippets:
Declarations:
Code:
int32 big_count = 0;
int16 count = 0, overflow = 0,  freq = 0;
int1 first = 1;


Setup in Main():
Code:
setup_timer_1(T1_INTERNAL|T1_DIV_BY_4); //1us resolution
setup_ccp1(CCP_CAPTURE_RE);
enable_interrupts(INT_TIMER1);
enable_interrupts(INT_CCP1);
enable_interrupts(GLOBAL);


CCP1 and Timer1 routines:
Code:
void  TIMER1_isr(void)
{
   overflow++;
}


void  CCP1_isr(void)
{
   if(first)
   {
      set_timer1(0); // Clear Timer1
      first=0;
   }
   else
   {
      count = get_timer1();
     
      first=1;
   }
   clear_interrupt(INT_CCP1); // Clear interrupt flag
}


and this is in while(1) loop:
Code:
freq = 1000000/count;
      if(overflow>0)
      {
         big_count = count;
         big_count = big_count + (overflow * 0xFFFF);
         freq = 1000000/big_count;
         overflow = 0;
      }


It gets very unstable at <30Hz if I try to compensate with overflow and at <16Hz if I don't care for the overflow.

Any help with this would be appreciated.
Ttelmah



Joined: 11 Mar 2010
Posts: 19328

View user's profile Send private message

PostPosted: Sat Apr 14, 2012 2:55 pm     Reply with quote

Some comments:

1) You need to clear overflow, where you clear the counter. When you get the INT_CCP, and 'first' is true.
2) Your maths is wrong. Overflow increments by one, when the count wraps from FFFF to 0. Each 'overflow' corresponds to an extra 10000 on the count, not FFFF.
3) Overflow is an int16. 0xFFFF is an int16. If you multiply these together, int16 arithmetic _will_ be used, and the result will overflow....
Much better to use 'make32':

Code:

big_count = make32(overflow,count);
freq = 1000000/big_count;


Vastly faster, smaller etc. etc...

Best Wishes
Athar Rasul



Joined: 19 Mar 2010
Posts: 13

View user's profile Send private message

PostPosted: Sun Apr 15, 2012 2:52 am     Reply with quote

Thanks.
Your points, although valid, still couldn't get me a correct value. Had to poke around a little to figure out where it was getting the false value from. Had to implement another valid flag bit to set things straight. here is updated code:
Code:
void  CCP1_isr(void)
{
   if(first)
   {
      set_timer1(0); // Clear Timer1
      first=0;
      overflow = 0;
   }
   else
   {
      count = get_timer1();
      actual_overflow = overflow;
      first=1;
      valid_count = 1;
   }
   clear_interrupt(INT_CCP1); // Clear interrupt flag
}

and in the loop:
Code:
if(valid_count)
      {
         big_count = make32(actual_overflow,count);
         actual_overflow = 0;
         valid_count = 0;
      }
      freq = 1000000/big_count;
Ttelmah



Joined: 11 Mar 2010
Posts: 19328

View user's profile Send private message

PostPosted: Sun Apr 15, 2012 8:42 am     Reply with quote

You could simplify a bit further.
Currently you save the two 16bit values in the interrupt, and then combine these in the main. The make32 instruction, is efficient. It takes no longer than saving two 16bit values. So why not build the 32bit value in the interrupt?.
So:
Code:

int1 first_edge=TRUE;
int32 count32;

void  CCP1_isr(void) {
   if (first_edge) {
      set_timer1(0); // Clear Timer1
      first_edge=FALSE;
      overflow = 0;
   }
   else
   {
      count32 = make32(overflow,get_timer1());
      first_edge=TRUE;
      valid_count = 1;
   }
   //clear_interrupt(INT_CCP1); // Clear interrupt flag
   //Not needed - unless you specify 'NOCLEAR', the compiler does
   //this for you
}


Four things here:
1) Assemble the values as you read them. No point in copying the numbers twice.
2) Get rid of the 'clear_interrupt'. The compiler already does this, unless you specify 'NOCLEAR' in the interrupt declaration. Wasted instruction.
3) Switch to using an int1 for the 'first' detection - single instruction test and branch for a bit like this.
4) Use 'true'/'false' for logic values. More obvious, and less likely to result in errors than coding 0/1.

Best Wishes
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
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