Trouble with Subtraction after ADC

Discussion in 'Embedded Systems and Microcontrollers' started by SteveO, Nov 26, 2009.

  1. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Hi all,

    Using PIC16F688. The idea of the program is to take two ADC inputs (AN6/AN7) and subtract the values. If AN6 > AN7 turn RA0 high, if not turn it low.

    However my code does not perform as expected. It only sets RA0 high if the two voltages on AN6 & AN7 are very close to one another. When either is significantly higher than the other the pin is low.

    I have checked and both conversions seem to working properly so I believe it is in my subtraction, end of code. If anyone has any ideas where I might be going wrong it would be greatly appreciated.

    Thanks.

    Code:

    Code ( (Unknown Language)):
    1. #include <htc.h>
    2. #include <math.h>
    3. #include <stdio.h>
    4. #define _XTAL_FREQ 4000000
    5. //#define bitset(var,bitno) (var|=1<<bitno)
    6. //#define bitclr(var,bitno) (var&=~(1<<bitno))
    7.  
    8.  
    9.  
    10. __CONFIG(INTIO & WDTDIS & PWRTEN & MCLRDIS & UNPROTECT & UNPROTECT & BORDIS);
    11.  
    12. void main(void){
    13.    
    14.     int x = 0;
    15.     int IRone = 0;      //AN6/RC2
    16.     int IRtwo = 0;      //AN7/RC3
    17.     int IRdiff = 0;     //for difference in IR measurements
    18.     PORTA = 0x00;       // PORTA low
    19.     PORTC = 0x00;       // PORTC low
    20.     CMCON0 = 0x07;      // turn off comparators
    21.     ADCON0 = 0b10011001;        //turn on ADC AN6 selected - right justified
    22.     ADCON1 = 0b00010000;        //ADC clock select FOSC/8 - 2us @ 4Mhz
    23.  
    24.     TRISA = 0x00;               //Port A outputs
    25.     TRISC = 0b00001100;         //RC2 + RC3 input pins (AN6/AN7)
    26.     ANSEL = 0b11000000;         //AN6/AN7 selected as analogue input pins
    27.  
    28.     ADIE = 1;       //enable ADC interrupt
    29.     ADIF = 0;       //ensure ADC interrupt flag is cleared
    30.    
    31.     while(1){
    32.  
    33.         ADCON0 = 0b10011001;        //ADC ON - AN6
    34.         ADCON0 = 0b10011011;        //ADC GO - AN6     
    35.             while(ADIF = 0){        //wait for ADC interrupt flag
    36.                 _delay(1);
    37.             }
    38.         ADIF = 0;                   //clear ADC interrupt flag
    39.         IRone = (ADRESH*256) + ADRESL;      //calc IRone - right justified
    40.  
    41.         ADCON0 = 0b10011101;        //ADC ON - AN7
    42.         ADCON0 = 0b10011111;        //ADC GO - AN7
    43.            
    44.             while(ADIF == 0){       //wait for ADC to finish
    45.                 _delay(1);
    46.             }
    47.         ADIF = 0;                   //clear ADC interrupt flag
    48.         IRtwo = (ADRESH*256) + ADRESL;      //calc IRtwo - right justified
    49.        
    50.         IRdiff = IRone - IRtwo;
    51.        
    52.         if(IRdiff >= 0){
    53.             PORTA = 0b00000001;
    54.                 for(x = 0; x <=13; x++){
    55.                     __delay_ms(20);
    56.                 }
    57.         }
    58.         else{
    59.             PORTA = 0b00000000;
    60.         }
    61.     }
    62. }
     
  2. kevins

    New Member

    Nov 26, 2009
    2
    0
    Looking at your calculation '(ADRESH*256) + ADRESL', I suspect that ADRESH is an unsigned char or other 8-bit value, which means the multiplication may overflow an 8-bit range. Try casting it to a integer for your calculations: -

    e.g. IRone/IRtwo = (int)ADRESH * 256 + ADRESL;
     
  3. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Thanks Kevins.

    Haven't thought about that but makes good sense. I'll look into this when I get home this evening and let you know my results.

    Thanks again.
     
  4. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Apparently not the problem, but definitely a good suggestion, thanks again though Kevin.

    After I had some time to do some more trouble shooting, it is right after I change channels on the ADC to AN7 by the code below that things start to get flaky:
    Code ( (Unknown Language)):
    1.         ADCON0 = 0b10011101;        //ADC ON - AN7
    2.             __delay_ms(2);
    3.         ADCON0 = 0b10011111;        //ADC GO - AN7
    By flaky I mean flashing in and out when it should be in a certain stage, and just not behaving as expected. Going to play around with it a little more tonight, and let you know if I get anywhere.

    Thanks.

    Addition:
    I have broken down my code to just set RA0 high if AN6 is > 1/2 the supply.
    When I have the code to run the ADC for AN7 commented out I get good results. However when the code is in the program (if/else statement still only dependent on AN6) I get weird results. AN7 seems to be able to control the output on RA0 as well. When I connect AN7 to ground / supply the LED goes out / stays on and the voltage on AN6 does not affect RA0.
     
    Last edited: Nov 26, 2009
  5. AlexR

    Well-Known Member

    Jan 16, 2008
    735
    54
    Try declaring IRone and IRtwo as "unsigned int". In C the type "int" defaults to a signed number and this could mess up your subtraction.
     
  6. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Thanks Alex. Makes sense and I have changed the code to reflect this. Didn't seem to have an impact but I will leave it this way.

    Cheers.
     
  7. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Ok I think I'm going crazy... The code seems to be doing the opposite...

    Here is the updated while loop.

    Code ( (Unknown Language)):
    1.     while(1){
    2.  
    3.         ADCON0 = 0b10011001;        //ADC ON - AN6
    4.             __delay_ms(2);
    5.         ADCON0 = 0b10011011;        //ADC GO - AN6     
    6.             while(ADIF = 0){        //wait for ADC interrupt flag
    7.                 _delay(1);
    8.             }
    9.         ADIF = 0;                   //clear ADC interrupt flag
    10.         IRone = (ADRESH*256) + ADRESL;      //calc IRone - AN6
    11.  
    12.  
    13.         ADCON0 = 0b10011101;        //ADC ON - AN7
    14.             __delay_ms(2);
    15.         ADCON0 = 0b10011111;        //ADC GO - AN7
    16.            
    17.             while(ADIF == 0){       //wait for ADC to finish
    18.                 _delay(1);
    19.             }
    20.         ADIF = 0;                   //clear ADC interrupt flag
    21.  
    22.         IRtwo = (ADRESH*256) + ADRESL;      //calc IRtwo - AN7
    23.  
    24.         //IRdiff = IRtwo - IRone;
    25.        
    26.         if(IRone >= 512){
    27.             RA0 = 1;
    28.                 for(x = 0; x <=13; x++){
    29.                     __delay_ms(20);
    30.                 }
    31.         }
    32.         else{
    33.             RA0 = 0;
    34.         }
    35.    
    36.         if(IRtwo >= 512){
    37.             RA2 = 1;
    38.                 for(x = 0; x <=13; x++){
    39.                     __delay_ms(20);
    40.                 }
    41.         }
    42.         else{
    43.             RA2 = 0;
    44.         }  
    45.    
    46.     }
    AN6 (IRone in code/Pin 8) is controlling RA2 (pin11) and AN7 (pin 7) is controlling RA0 (pin 13). This completely contradicts what I have written. Am I going crazy, because I'm sure there is a reasonable explination for this.

    Thanks again.
     
  8. eng1

    New Member

    Oct 25, 2009
    13
    0
    I suppose that your delay is for the sample & hold capacitor to charge after changing the input channel. Is the source a low-impedance one?

    If you have checked that the two readings are correct when using AN6 and AN7 separately, consider increasing the delay (this depends on the input inpedance).

    As suggested, cast the A/D results to int:
    Code (Text):
    1.  
    2. IRone = ADRESH;
    3. IRone = IRone << 8 + ADRESL;
    4.  
     
  9. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Thanks everyone I really appreciate the help.Turns out my first ADIF while statement needed to be changed to a == instead of a =. Really silly mistake that took alot of time to figure out. Thanks all for the help. Learnt alot from the comments here.

    Cheers.
     
  10. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Code ( (Unknown Language)):
    1. while(ADIF == 0){       //wait for ADC to finish
    2. _delay(1);
    3. }
    When I do this command the program just waits for the ADC to finish and the interrupt flag to be set. I'm wondering, is there is a better way to accomplish this so that when the interrupt flag is set it will jump to certain code? This way I can be doing other functions while waiting for the ADC to complete.
     
  11. Markd77

    Senior Member

    Sep 7, 2009
    2,803
    594
    You could enable the interrupt, but another way would be to put a call to the other job that needs doing where your _delay(1) is.
     
  12. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Thanks Mark.

    What exactly do you mean by enabling the interrupt? How would I then check if it has been flagged?

    Thanks.
     
  13. Markd77

    Senior Member

    Sep 7, 2009
    2,803
    594
    There is a fairly good bit in the datasheet, section 8.1.7 and have a look at 11.5 as well. They explain it better than I could.
    If you only have one source of interrupt enabled then there is no need to check what caused it.
    Using interrupts can be tricky, but worth it in the long run.
     
  14. BMorse

    Senior Member

    Sep 26, 2009
    2,675
    234
    you have to clear the ADIF bit first before enabling the ADC.....

    You are setting the ADIE before you clear the ADIF

     
  15. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    OK, so it looks like I need to also enable the global interrupt bit of INTCON register. I'm just a little confused with how I can be alerted that there is an interrupt without constantly checking for it.

    Thanks.
     
  16. BMorse

    Senior Member

    Sep 26, 2009
    2,675
    234
    A ISR (Interrupt handler routine) will take care of the tasks "behind" the scene if you have the right interrupts set, you will just have to write your code in the ISR to do what you want when this happens.... see this post here (#5) >> http://forum.allaboutcircuits.com/showthread.php?t=30957

    they are using a F84 but same difference.
    For a F688 to use the ADC with an Interrupt follow these guidelines:


    If you have more than one interrupt enabled, you will have to determine which one caused the interrupt, if checking to see if it is the ADC that caused the interrupt, you can check to see if PIR1,ADIF bit is set if it is, this means an ADC interrupt occured.... then clear this bit before exiting the ISR (ex: bcf PIR1,ADIF)
     
    Last edited: Dec 2, 2009
  17. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Thanks for the help. I think I'm missing something. How does this ISR get called when the interrupt occurs? Is it when the global interrupt flag is first enabled and then set by the program that your code jumps to a specific spot? Is it correct naming of the function or how is it possible to jump to this part in the code without having to consistently check the register? Sorry for the junior questions.

    Thanks.
     
  18. AlexR

    Well-Known Member

    Jan 16, 2008
    735
    54
    There is no standard for how an ISR (interrupt service routine) is defined and each C compiler has its own method. In the case of Hi-Tech C the interrupt handling function is flagged by the function qualifier "interrupt", so it might look like this: (other compilers use different syntax to define the ISR function)
    Code ( (Unknown Language)):
    1. void interrupt my_isr(void)  
    2. {
    3.    // global interrupts are automatically disabled while program is in the interrupt routine
    4.    // check interrupt flags to find source of interrupt
    5.    // do whatever needs to be done
    6.    // clear the interrupt flag
    7. }
    8.    
    9.   // returning from interrupt automatically enables global interrupts
    10.  
    What actually happens at the machine code level is that the compiler places a goto instruction at address 004 pointing to the interrupt function. As soon as in interrupt occurs the current program counter value gets pushed onto the stack and the program counter gets loaded with the interrupt vector value (004) It is steered to the interrupt routine, does whatever it has to do, then when it returns from interrupt it pulls the program counter off the stack and continues along its merry way from where it was before the interrupt occurred.
     
  19. SteveO

    Thread Starter Active Member

    May 15, 2008
    33
    0
    Awesome Alex. Thanks very much for your thorough explanation. I understand know that it knows where to go based on the interrupt function qualifier. Makes perfect sense, I will give this a shot.

    Thanks.
     
Loading...