PIC Period Measurement Program

Discussion in 'Embedded Systems and Microcontrollers' started by The_Fleertz, Oct 19, 2012.

  1. The_Fleertz

    Thread Starter New Member

    Apr 23, 2012
    8
    0
    Hey, I'm trying to write code to measure an input and determine if it is within five cents of the period of a guitar note. The input from a function generator is 110 Hz and the buttons are set up correctly. What doesn't work is the comparison if(tau>period_low & tau<period_high) which should cause an LED to blink. I noticed that the code also never enters the if statement if(capreg_new>capreg_old) either. I don't know if there is a timing error somewhere? Thanks for the help.

    Code ( (Unknown Language)):
    1.  
    2. //Tuner Program/\/\/\
    3.  
    4. #define  _LEGACY_HEADERS
    5. #include <htc.h>
    6.  
    7. __CONFIG(INTIO & WDTDIS & PWRTDIS & BORDIS & BORDIS & LVPDIS & DEBUGEN & DUNPROTECT & UNPROTECT);
    8.  
    9. #define stop 0
    10. #define forward 1
    11. #define backward 2
    12. //void motor_forward(void);
    13. //void motor_backward(void);
    14. unsigned long long get_period(void);
    15.  
    16. volatile unsigned int notes[4][6]=  {12134, 9091, 6811, 5102, 4050, 3034,               //E Standard
    17.                                     13620, 9091, 6811, 5102, 4050, 3034,                //Drop D
    18.                                     13620, 9091, 6811, 5102, 4545, 3405,                //DADGAD
    19.                                     12857, 9631, 7216, 5405, 4290, 3214 };              //Eb
    20.  
    21. volatile unsigned long long capreg_new, capreg_old;
    22. volatile char overflow;
    23. volatile unsigned long long tau,period, period_high, period_low;
    24. unsigned int portb=0;
    25. char num_times=0;
    26.  
    27.  
    28.  
    29. void main(void){
    30.     ANSEL=0x00;
    31.     ANSELH=0x00;
    32.     PORTD=0;
    33.     TRISB=0b11111111;
    34.     TRISC=0b00000010;           //CCP2 pin as input
    35.     TRISD=0b00000000;
    36.     T1CON=0x01;
    37.     PEIE=1;
    38.     GIE=1;
    39.     CCP2IF=0;
    40.     CCP2IE=0;
    41.     TMR1IF=0;
    42.     TMR1H=0;
    43.     TMR1L=0;
    44.     TMR1IE=0;
    45.     CCP2CON=0b00000111;
    46.    
    47.     while(1){
    48.         period=get_period();
    49.  
    50.         period_high=(period*4108)>>8;  //calculating 5 cent off period (4108=2^12*1.00289)
    51.         period_low=(period*4084)>>8;   //(4084=2^12*(1/1.00289)
    52.  
    53.         TMR1IE=1;
    54.         CCP2IE=1;
    55.  
    56.         while(num_times==0);
    57.         capreg_old=(CCPR2H<<8)+CCPR2L;
    58.         while(num_times==1);
    59.         capreg_new=(CCPR2H<<8)+CCPR2L;
    60.         CCP2IE=0;
    61.         TMR1IE=0;
    62.         num_times=0;
    63.  
    64.         if(capreg_new>capreg_old){    //Code never enters here
    65.             tau=(capreg_new-capreg_old)+65536*overflow;  //calculate period
    66.         }
    67.         else{
    68.             tau=65536-(capreg_old-capreg_new)+65536*overflow;
    69.         }
    70.         if(tau>period_low & tau<period_high){
    71.             PORTD=PORTD^0b11111111;
    72.         }
    73.     }          
    74. }
    75.  
    76. void interrupt isr(void){
    77.  
    78.     if(TMR1IF=1){               //If Timer 1 overflows and nothing has been captured do not increase overflow
    79.         TMR1IE=0;
    80.         if(num_times!=0){
    81.             overflow++;
    82.         }
    83.         TMR1IF=0;
    84.         TMR1IE=1;
    85.     }
    86.  
    87.     if(CCP2IF=1){
    88.         CCP2IE=0;
    89.         CCP2IF=0;
    90.         num_times++;
    91.     }
    92. }
    93.  
    94. unsigned long long get_period(void){
    95.     char portb, string, i;
    96.     unsigned long long period=0;    //Function to get the period associated with
    97.     portb=PORTB;              //the string being tuned
    98.  
    99.     for(i=0;i<4;i++){
    100.         if((portb&0b11000000)==(i<<6)){
    101.             for(string=0;string<6;string++){
    102.                 if(portb&(0b00000001<<string)){
    103.                     period=notes[i][string];
    104.                 }
    105.             }
    106.         }
    107.     }
    108.     return period;
    109. }[/i]
     
  2. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,395
    1,607
    Heh... give yourself a smack on the back of the head. :D

    Inside your ISR you have these two statements:

    if(TMR1IF=1)...

    if(CCP2IF=1)...

    I do believe you intended comparisons instead of assignments.

    I would freely donate body parts for a C compiler that would issue a warning if it ever sees this situation. It's my most common error.
     
  3. spinnaker

    AAC Fanatic!

    Oct 29, 2009
    4,887
    1,019

    I wouldn't need it because I NEVER make this mistake. :eek:

    BTW OP You also have

    if(tau>period_low & tau<period_high){

    //I suspect it should be

    if(tau>period_low && tau<period_high){

    a single & is a bit operation.
     
  4. t06afre

    AAC Fanatic!

    May 11, 2009
    5,939
    1,222
    Just a note to the OP. It is perfectly OK to use
    if(TMR1IF)... as if(TMR1IF==1)

    if(!CCP2IF)... as if(!CCP2IF==0)...
    I like this style much better. You can also use this for a zero or non zero evaluation on any integer variable
     
  5. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,395
    1,607
    Another note: I love whitespace (extra spaces tabs and linefeeds) to parse elements on a line. It makes the mental task of catching simple things so much easier.

    Which group of code lines would be easier to read?

    These:
    Code ( (Unknown Language)):
    1.  
    2.   if(TMR1IF=1);
    3.   Sum=Rat_A+Rate_B+Rate_C;
    4.   if(!CCP2IF==0);
    5.  
    Or these:
    Code ( (Unknown Language)):
    1.  
    2.   if( TMR1IF = 1 );
    3.   Sum = Rat_A + Rate_B + Rate_C;
    4.   if( !CCP2IF == 0 );
    5.  
    (two of these lines have typos)
     
    Last edited: Oct 20, 2012
  6. The_Fleertz

    Thread Starter New Member

    Apr 23, 2012
    8
    0
    You guys are life savers. I gotta remember to check that kind of stuff. :) Anyone have any tips to speed up my code a bit? for 82.4 Hz, low E note, it is really slow at capturing. Also if someone could double-check my tau calculation in the else statement. else{ tau = 65536 - (capreg_old-capreg_new)+65536*overflow;} I just want to be sure that that math will give the correct answer.
    Thanks
     
  7. Markd77

    Senior Member

    Sep 7, 2009
    2,803
    594
    If your PIC doesn't have a hardware multiplier, you could use bit bashing to get much faster multiply times, eg in this line:
    Code ( (Unknown Language)):
    1. period_high=(period*4108)>>8;  //calculating 5 cent off period (4108=2^12*1.00289)
    you could replace the *4108)>>8 with this or the C equivalent, link at bottom to calculate more:

    Code ( (Unknown Language)):
    1. ; ACC = ACC * 16.0469
    2. ; Temp = TEMP
    3. ; ACC size = 16 bits
    4. ; Error = 0.01 %
    5. ; Bytes order = little endian
    6. ; Round = no
    7.  
    8. ; ALGORITHM:
    9. ; Clear accumulator
    10. ; Add input * 16 to accumulator
    11. ; Add input / 32 to accumulator
    12. ; Add input / 64 to accumulator
    13. ; Move accumulator to result
    14. ;
    15. ; Approximated constant: 16.0469, Error: 0 %
    16.  
    17. ;     Input: ACC0 .. ACC1, 16 bits
    18. ;    Output: ACC0 .. ACC2, 21 bits
    19. ; Code size: 46 instructions
    20.  
    21.     cblock
    22.     ACC0
    23.     ACC1
    24.     ACC2
    25.     TEMP0
    26.     TEMP1
    27.     TEMP2
    28.     endc
    29.  
    30. ;copy accumulator to temporary
    31.     movf    ACC1, w
    32.     movwf    TEMP1
    33.     movf    ACC0, w
    34.     movwf    TEMP0
    35.  
    36.  
    37. ;shift accumulator right 1 times
    38.     clrc
    39.     rrf    ACC1, f
    40.     rrf    ACC0, f
    41.  
    42. ;add temporary to accumulator
    43.     addwf    ACC0, f
    44.     movf    TEMP1, w
    45.     skpnc
    46.     incfsz    TEMP1, w
    47.     addwf    ACC1, f
    48.  
    49. ;shift accumulator right 5 times
    50.     swapf    ACC0, w
    51.     andlw    0x0F
    52.     movwf    ACC0
    53.     swapf    ACC1, w
    54.     movwf    ACC1
    55.     andlw    0xF0
    56.     xorwf    ACC1, f
    57.     iorwf    ACC0, f
    58.     skpnc
    59.     bsf    ACC1, 4
    60.     clrc
    61.     rrf    ACC1, f
    62.     rrf    ACC0, f
    63.  
    64. ;shift temporary left 4 times
    65.     swapf    TEMP1, f
    66.     movf    TEMP1, w
    67.     andlw    0x0F
    68.     xorwf    TEMP1, f
    69.     movwf    TEMP2
    70.     swapf    TEMP0, f
    71.     movf    TEMP0, w
    72.     andlw    0x0F
    73.     xorwf    TEMP0, f
    74.     iorwf    TEMP1, f
    75.  
    76. ;add temporary to accumulator
    77.     clrf    ACC2
    78.     movf    TEMP0, w
    79.     addwf    ACC0, f
    80.     movf    TEMP1, w
    81.     skpnc
    82.     incfsz    TEMP1, w
    83.     addwf    ACC1, f
    84.     movf    TEMP2, w
    85.     skpnc
    86.     incfsz    TEMP2, w
    87.     addwf    ACC2, f
    88.  
    89.  
    90. ; Generated by www.piclist.com/cgi-bin/constdivmul.exe (1-May-2002 version)
    91. ; Sat Oct 20 18:12:13 2012 GMT
     
  8. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,395
    1,607
    First off, don't do anything twice you don't have to do.

    Next, don't calculate anything you can put into a look up table.

    In your main while(1) loop, you check the buttons and return the "unsigned long long period," yet most times you do this the same button is down and the present answer is the same as the last answer. So save the last state of the buttons and don't calculate is present == last.

    Next, can the high and low ranges be per-calculated? (I'd tell you but I didn't read your code that close) (has too many wide tabs for me to see it).
     
  9. THE_RB

    AAC Fanatic!

    Feb 11, 2008
    5,435
    1,305
    Agreed! In the "good old days" classrooms taught C programmers to write it like this;
    if(1==CCP2IF)

    as the comparison operator gives the same result but if you erroneously use an assignment operator it always signals an error in code.

    Now if only I had a dollar for every time I should have coded like that but never bothered...

    (to the OP); Re measuring the period, I think your measurement procedure is not a good way to do it. A better way is to use the capture compare module CCP1 and a 16bit timer, to automatically give you a capture value representing the period.

    Another way is to clear the 16bit TMR1 on the pulse / edge, then when you detect the next pulse / edge grab the timer value manually. That can be an easier way for long period where you can also count the times that TMR1 has rolled over to give long period captures of periods > 65536 counts.
     
    Last edited: Oct 21, 2012
Loading...