Maker Pro
Maker Pro

Help with ADC conversion registers in PIC or possible logic in code flaw

chopnhack

Apr 28, 2014
1,576
Joined
Apr 28, 2014
Messages
1,576
Hi all I am trying to get a rather simple program cobbled together to run some tests on my run delay board. Since that thread is a bit long and was mostly dedicated to producing the board as well as a running commentary on the project, I will break this problem out as it can stand alone.

Can anyone spot any reasons as to why I am not getting any output as expected from my LED's?

Long and short - I am feeding in a 2.5vDC signal with a 2.5VAC signal riding on top of it into my ADC. After conversion, I am expecting ADRESL to be put into my variable and then the if statements to actuate from there.

My concerns are that perhaps I have incorrectly setup a SFR or that there is a timing mismatch between my internal clock at 4MHz and the ADC clock which I set to internal, but there is a max listed of 500KHz for the ADC.
I am not sure if that is the issue...

Any thoughts appreciated, thanks!

Link to PIC12F675 datasheet.


Code:
// CONFIG
#pragma config FOSC = INTRCIO   // Oscillator Selection bits (INTOSC oscillator: I/O function on GP4/OSC2/CLKOUT pin, I/O function on GP5/OSC1/CLKIN)
#pragma config WDTE = OFF       // Watchdog Timer Enable bit (WDT disabled)
#pragma config PWRTE = OFF      // Power-Up Timer Enable bit (PWRT disabled)
#pragma config MCLRE = ON       // GP3/MCLR pin function select (GP3/MCLR pin function is MCLR)
#pragma config BOREN = OFF      // Brown-out Detect Enable bit (BOD disabled)
#pragma config CP = OFF         // Code Protection bit (Program Memory code protection is disabled)
#pragma config CPD = OFF        // Data Code Protection bit (Data memory code protection is disabled)

#include <stdio.h>
#include <stdlib.h>
#include <xc.h>


#define _XTAL_FREQ 4000000      // Setting internal clock to 4MHz

int main(void)

{
  // Variable Declaration
  int AN3_AD_IN_110 = 0;                        // variable to hold value of A/D input from U2 Allegro chip, 110v line.
  int AN2_AD_IN_220 = 0;                        // " from U3 Allegro chip, 220v line.
                                                // This signal is baseline of 2.5VDC with a 0-2.5VAC signal riding on top.  
  int max = 511, min = 511,    
          max1 = 511, min1 = 511;               // variables for min/max comparisons on both channels 110/220 
  int temp_value = 0, temp_value1= 0, 
          peak = 0, temp_peak = 0;              // Hold temp values to compare ADC against
  int count = 0;
  //ADC Setup
  ADCON0bits.ADFM = 0;                          // Left justified A/D results
  ADCON0bits.VCFG = 0;                          // Voltage reference set to Vdd
  ADCON0bits.CHS0 = 1;                          // Channel select 0 register set to 1 to select AN1
  ADCON0bits.ADON = 1;                          // Turn A/D converter on
  TRISIO = 0b000010;                            // AN1 is input :old comments //0b001100;     // Only AN2 and AN3/MCLR are inputs all else outputs.
  GPIO = 0b000000;                              // Turn off all outputs initially
  //Analog Select Register Setup
  ANSELbits.ADCS = 0b011;                       // Clock derived from internal osc - max of 500kHz - problem with 4MHz xtal freg??
  ANSELbits.ANS1 = 1;                           // Sets AN1 as analog input (page 46 of pic675 manual states this and trisio need to be set)
   
  // Start A/D conversion process
  ADCON0bits.GO = 1;                          
   
 while(1) 
 {          
    while (ADCON0bits.nDONE){}                  // Wait for conversion to be done
    AN3_AD_IN_110 = ADRESL + (ADRESH << 8);     // 1024 bit resolution 5v/1023 = 4.88mV per bit
      if (AN3_AD_IN_110 >= max)                 // check for 511 (1/2 Vcc) or greater
        { 
          max = AN3_AD_IN_110;                  // set variable max to 511 or greater
        }
      if (AN3_AD_IN_110 <= min)                 // check variable min for 511 or less
        { 
          min = AN3_AD_IN_110;                  // set var. min to 511 or less
        }
   
     temp_peak = max - min;                     // temporary peak value established and used to compare later  
   
     if (count < 128)                           // set counter to prevent infinite loop
        {  
            count = count + 1;                  // incrementing counter
        } 
     
     if (count == 128)
        {
         __delay_ms(1000);
         GPIO4 != GPIO4;
         __delay_ms(1000);
     //    write a =! GPIO statement to turn led GP4 on and off every 128 cycles
        }
     if ((temp_peak >= 144) && (count == 128)   // control statement - if peak value is above threshold and we iterated 129 times
           && (max >= 584) && (min <= 438))     // and if 0.354V above and below Vcc - (110v threshold) then proceed
        { 
          __delay_ms(2000);                            // 2 second delay before turning on
          //GPIO = 0b000001;                    // set GP0 high to turn on relay
          GPIO = 0b100000;  //REMOVE ONLY FOR TESTING // this is to check code on dev. board - GP5 outermost LED to corner should light
          //ADCON0bits.GO_DONE = 1;             // do A/D measurement
          count = 0;                            // reset count variable to zero for next go around
        }  
               
         
           

    if ((temp_peak <= 130) && (max <= 560) && (min >= 490)) 
            {
               __delay_ms(5000);                // statement checks to see if current has dropped below threshold
               GPIO = 0b000000;                 //waits 5 seconds then shuts off GP5
            } 
    // Start A/D conversion process
    ADCON0bits.GO = 1;          
  }
}
 
Last edited:

Arouse1973

Adam
Dec 18, 2013
5,178
Joined
Dec 18, 2013
Messages
5,178
Hello John.
You have two banks in this micro. Not sure about C but in assembler you have to switch banks using banksel. This allows you to amend the registers you need. Then you switch back to bank 1 after that. Just a thought.
Cheers
Adam
 

chopnhack

Apr 28, 2014
1,576
Joined
Apr 28, 2014
Messages
1,576
Good thoughts Adam, I looked through the manual and it says that the RP0 register is only used for direct addressing. I have accessed the ADC before successfully with directly addressing it. As you mentioned this pic only has two banks, one for the GPR and one for the SFR's.

On another forum, someone mentioned to place 15us delays when the first starting the converter to make sure that state machine has enough time to 'get' ready. I implemented that and still no luck.
 

Arouse1973

Adam
Dec 18, 2013
5,178
Joined
Dec 18, 2013
Messages
5,178
Its not just for the A/D. Look where the TRIS register is. Its in bank 2, so dont you have to switch banks to set the ports up?
Adam
 

chopnhack

Apr 28, 2014
1,576
Joined
Apr 28, 2014
Messages
1,576
Its not just for the A/D. Look where the TRIS register is. Its in bank 2, so dont you have to switch banks to set the ports up?
Adam
On this pic and the pic10LF320 I have not had to switch banks that I am aware of to do anything with them as of yet. I will look through the XC8 compiler manual to see if there is anymore information on this. I have seen issues before mentioned on various web searches of pic and pic programming of bank select being used and the problems with assuming that you are in the correct bank. I am thinking that C might take care of this detail in the background.

On a positive note, it was brought to my attention that this line was conflicting with my left shifting of ADRESH!
Code:
ADCON0bits.ADFM = 0;                          // Left justified A/D results
Apparently I was negating what I was trying to accomplish!!! So with changing this bit to 1 (ADFM), I now get the led to come on after sufficient power is sensed and about a three second delay - however, the rest of the program doesn't seem to actuate as the LED stays on throughout. So more research is necessary to find out what is going wrong in my loop.
Thanks mate!
 

BobK

Jan 5, 2010
7,682
Joined
Jan 5, 2010
Messages
7,682
I am not sure I trust the C compiler to handle (ADRESH << 8) correctly. It should be converted to int before the shift, but if it is not, you would always get 0 as the result. Try changing it to ((int)ADRESH << 8)

Bob
 

chopnhack

Apr 28, 2014
1,576
Joined
Apr 28, 2014
Messages
1,576
You don't have to explicitly switch banks when using C. That is what compilers are for :)

Bob
LOL, thanks Bob. Looks like that is correct for this pic anyway.
Edit - Turns out the compiler auto promotes to the proper type.

I am not sure I trust the C compiler to handle (ADRESH << 8) correctly.
I got the code to work, but this is something to keep in mind also.

At this point I am looking into a potential 'bounce' that is causing the circuit to not perform 100% of the times. Sometimes it will shut off the LED other times it will not shut off. Here is the corrected code.

Code:
#define _XTAL_FREQ 4000000      // Setting internal clock to 4MHz

int main(void)

{
  // Variable Declaration
  int AN3_AD_IN_110 = 0;                        // variable to hold value of A/D input from U2 Allegro chip, 110v line.
  int AN2_AD_IN_220 = 0;                        // " from U3 Allegro chip, 220v line.
                                                // This signal is baseline of 2.5VDC with a 0-2.5VAC signal riding on top. 
  int max = 511, min = 511,   
          max1 = 511, min1 = 511;               // variables for min/max comparisons on both channels 110/220
  int temp_value = 0, temp_value1= 0,
          peak = 0, temp_peak = 0;              // Hold temp values to compare ADC against
  int count = 0;
  
  //ADC Setup
  ADCON0bits.ADFM = 1;                          // Right justified A/D results
  ADCON0bits.VCFG = 0;                          // Voltage reference set to Vdd
  ADCON0bits.CHS0 = 1;                          // Channel select 0 register set to 1 to select AN1
  ADCON0bits.ADON = 1;                          // Turn A/D converter on
  __delay_us(15);
  TRISIO = 0b000010;                            // AN1 is input :old comments //0b001100;     // Only AN2 and AN3/MCLR are inputs all else outputs.
  GPIO = 0b000000;                              // Turn off all outputs initially
  //Analog Select Register Setup
  ANSELbits.ADCS = 0b101;                       // Clock derived from internal osc - max of 500kHz - problem with 4MHz xtal freg??
  ANSELbits.ANS1 = 1;                           // Sets AN1 as analog input (page 46 of pic675 manual states this and trisio need to be set)
      
while(1)
{ 
    // Start A/D conversion process
    ADCON0bits.GO = 1; 
    __delay_us(15);
    while (ADCON0bits.nDONE){}                  // Wait for conversion to be done
    AN3_AD_IN_110 = ADRESL + (ADRESH << 8);     // 1024 bit resolution 5v/1023 = 4.88mV per bit
      if (AN3_AD_IN_110 >= max)                 // check for 511 (1/2 Vcc) or greater
        {
          max = AN3_AD_IN_110;                  // set variable max to 511 or greater
        }
      if (AN3_AD_IN_110 <= min)                 // check variable min for 511 or less
        {
          min = AN3_AD_IN_110;                  // set var. min to 511 or less
        }
  
     temp_peak = max - min;                     // temporary peak value established and used to compare later 
  
     if (count < 128)                           // set counter to prevent infinite loop
        { 
            count = count + 1;                  // incrementing counter
        }
        
     if ((temp_peak >= 144) && (count >= 80))   // control statement - if peak value is above threshold and we iterated 129 times
                                                 // and if 0.354V above and below Vcc - (110v threshold) then proceed
        {
          __delay_ms(2000);                            // 2 second delay before turning on
          //GPIO = 0b000001;                    // set GP0 high to turn on relay
          GPIO5 = 1;  //REMOVE ONLY FOR TESTING // this is to check code on dev. board - GP5 outermost LED to corner should light
          //ADCON0bits.GO_DONE = 1;             // do A/D measurement 
          max = 511;
          min = 511;
        } 
       
      if ((temp_peak <= 70) && (count >= 100))   // statement checks to see if current has dropped below threshold
        {
          //__delay_ms(3000);                              //waits 3 seconds then shuts off GP5
          GPIO5 = 0;
        }
    
      if (count >= 120)
        {
         GPIO4 = 1;                        //    write a =! GPIO statement to turn led GP4 on and off every 128 cycles
         count = 0;                        // reset count variable to zero for next go around
         __delay_ms(1000);
         GPIO4 = 0;
        }
    
    }
}
 
Last edited:

chopnhack

Apr 28, 2014
1,576
Joined
Apr 28, 2014
Messages
1,576
To expand on my bounce theory:

I am using my development board as a means to test this code using a pic12f675 (same as intended project). I am using GP4 as an indicator LED to let me know each time the program has cycled through at least 120 times. GP5 is my targeted on/off for my relay, which is currently just toggling an LED. When I turn on the project, GP4 blinks every second, the off time is very short, just barely perceptible and the on time is the majority of the second. When power goes through the allegro chip and the adc reads in a value higher than the threshold, GP4 turns off within a second and does not come back on until GP5 turns on three seconds later. GP4 also then blinks at a different rate, 1 second on while 3 seconds off. This blink would indicate to me that the last if statement actuates, giving 1 second on time, goes through the program and apparently actuates the 2nd to last if statement getting that 3 second delay before coming back on.

-The Bounce-
Now if I turn off the load to the circuit while GP4 is off, GP5 will turn off after 3 seconds as its supposed to. If I wait until GP4 flashes on and turn off the load, GP5 remains on. Is this a case of bounce at the pins? Should I add a delay or am I misinterpreting this and I still need to improve my coding skills!
 

Arouse1973

Adam
Dec 18, 2013
5,178
Joined
Dec 18, 2013
Messages
5,178
Ok if they switch thats great. But you have a compiler for assembler and they dont do that lol.
Adam
 

NorthGuy

Mar 24, 2016
53
Joined
Mar 24, 2016
Messages
53
If you get your temp_peak between 70 and 144 at any point after count > 120 and the load is off, then temp_peak will neither decrease (because min and max won't reset) nor increase (because there's no power). Thus, none of your "if" statements involving temp_peak will be called.

If you want to detect the power in a rollng manner, best use some sort of measure which doesn't require re-setting min and max.

Alternatively, calculate min and max over a period of time, then get your temp_peak at the end of the period and immediately reset min and max.
 

chopnhack

Apr 28, 2014
1,576
Joined
Apr 28, 2014
Messages
1,576
Thank you for input NorthGuy - I was having trouble tracing the flaw in the logic. Frankly, I still am, because the desired behavior is for GP5 to stay on while current is above the threshold and to only turn off after it falls below. The rounds around the count variable are such as to prevent false triggering and to allow enough sampling of the incoming AC wave.

If you want to detect the power in a rollng manner, best use some sort of measure which doesn't require re-setting min and max.

The purpose of resetting the min/max values is to sense when power is removed. Since min and max are only updated with new highs and lows, they remain at those higher values, without resetting them, there would be no way for the program to check the next incoming input to see if power is still there.

Alternatively, calculate min and max over a period of time, then get your temp_peak at the end of the period and immediately reset min and max.

That is a very good idea, thank you! Compartmentalize the min/max, spit out the results temporarily to temp_peak and recycle min/max. This would allow the other parts of the program to still use the count variable as a loop to choose their behaviors.

I will work on that tomorrow, thanks for the food for thought. Programming is something I have had exposure to over the years, but I have never had to code very much.
 

NorthGuy

Mar 24, 2016
53
Joined
Mar 24, 2016
Messages
53
The purpose of resetting the min/max values is to sense when power is removed. Since min and max are only updated with new highs and lows, they remain at those higher values, without resetting them, there would be no way for the program to check the next incoming input to see if power is still there.

Exactly, therefore the temp_peak is not working very well when you update it continuously.

Another measure, such as exponential average, which doesn't require re-setting would work better. For example, you could read a value and calculate x = abs(v-512)*a + x*(1-a). "a" is between 0 and 1 here. "x" would change continuously and will go higher when there's more power and lower when it's no power. It works the same as a rectifier ("abs" function) followed by low-pass RC filter. Probably not a good idea for 12F675.

That is a very good idea, thank you! Compartmentalize the min/max, spit out the results temporarily to temp_peak and recycle min/max.

The important thing is to set the duration of the period so that it includes both positive and negative peaks at any possible AC frequencies.
 

chopnhack

Apr 28, 2014
1,576
Joined
Apr 28, 2014
Messages
1,576
Probably not a good idea for 12F675.
Indeed, its a great idea, but memory space in 'C' is already very limited! Good to keep in the toolbox for larger pics though, thank you!

I was able to implement this part of your suggestion:
Alternatively, calculate min and max over a period of time, then get your temp_peak at the end of the period and immediately reset min and max.

EDIT: Not quite, somehow with prolonged testing, GP5 which should stay lit is resetting to a low status, and GP4 is not consistently blinking as expected as it goes through each cycle of the routine. Instead sometimes GP4 will have a longer off time followed by GP5 going off! I am a little lost as to what could be causing the glitch.

Code:
int main(void)

{
  // Variable Declaration
  int AN3_AD_IN_110 = 0;                        // variable to hold value of A/D input from U2 Allegro chip, 110v line.
  int AN2_AD_IN_220 = 0;                        // " from U3 Allegro chip, 220v line.
                                                // This signal is baseline of 2.5VDC with a 0-2.5VAC signal riding on top.
  int max = 511, min = 511,  
          max1 = 511, min1 = 511;               // variables for min/max comparisons on both channels 110/220
  int temp_value = 0, temp_value1= 0,
          peak = 0, temp_peak = 0;              // Hold temp values to compare ADC against
  int count = 0;
 
  //ADC Setup
  ADCON0bits.ADFM = 1;                          // Right justified A/D results
  ADCON0bits.VCFG = 0;                          // Voltage reference set to Vdd
  ADCON0bits.CHS0 = 1;                          // Channel select 0 register set to 1 to select AN1
  ADCON0bits.ADON = 1;                          // Turn A/D converter on
  __delay_us(15);
  TRISIO = 0b000010;                            // AN1 is input :old comments //0b001100;     // Only AN2 and AN3/MCLR are inputs all else outputs.
  GPIO = 0b000000;                              // Turn off all outputs initially
  //Analog Select Register Setup
  ANSELbits.ADCS = 0b101;                       // Clock derived from internal osc - max of 500kHz - problem with 4MHz xtal freg??
  ANSELbits.ANS1 = 1;                           // Sets AN1 as analog input (page 46 of pic675 manual states this and trisio need to be set)
     
while(1)
{
    // Start A/D conversion process
    ADCON0bits.GO = 1;
    __delay_us(15);
    while (ADCON0bits.nDONE){}                      // Wait for conversion to be done
 
    while (count < 64)                              // set counter to prevent infinite loop
     
     {
        AN3_AD_IN_110 = ADRESL + (ADRESH << 8);     // 1024 bit resolution 5v/1023 = 4.88mV per bit
 
          if (AN3_AD_IN_110 >= max)                 // check for 511 (1/2 Vcc) or greater
            {
              max = AN3_AD_IN_110;                  // set variable max to 511 or greater
            }
 
          if (AN3_AD_IN_110 <= min)                 // check variable min for 511 or less
            {
              min = AN3_AD_IN_110;                  // set var. min to 511 or less
            }
        count = count + 1;                          // incrementing counter              
     }
       
     temp_peak = max - min;                     // temporary peak value established and used to compare later
     min = 511;                                 // reset variable
     max = 511;                                 // reset variable
   
     if (temp_peak >= 80)                       // control statement - if peak value is above threshold
                                                // and if 0.354V above and below Vcc - (110v threshold) then proceed
        {
          __delay_ms(2000);                     // 2 second delay before turning on
          //GPIO = 0b000001;                    // set GP0 high to turn on relay
          GPIO5 = 1;  //REMOVE ONLY FOR TESTING // this is to check code on dev. board - GP5 outermost LED to corner should light
          //ADCON0bits.GO_DONE = 1;             // do A/D measurement          
        }
         
      if (temp_peak <= 40)
        {
          __delay_ms(3000);
          GPIO5 = 0;
        }
   
      if (count == 64)
        {
         GPIO4 = 1;                        //    write a =! GPIO statement to turn led GP4 on and off every 128 cycles
         __delay_ms(50);
         GPIO4 = 0;
        }
      count = 0;                           // reset count variable to zero for next go around
    }
}
 
Last edited:

NorthGuy

Mar 24, 2016
53
Joined
Mar 24, 2016
Messages
53
EDIT: Not quite, somehow with prolonged testing, GP5 which should stay lit is resetting to a low status, and GP4 is not consistently blinking as expected as it goes through each cycle of the routine. Instead sometimes GP4 will have a longer off time followed by GP5 going off! I am a little lost as to what could be causing the glitch.

You probably need to move the ADC acquisition code (marked "Start ADC conversion process") inside the "while (count < 64)" loop.

The 2 and 3 second delays (when you turn GP5 on or off) stop the whole processes, so GP4 stops blinking while these delays are performed. If you want it to blink during delays, you need to use timers instead of "__delay_ms()". Or, you can replace big delays with a series of small delays interleaved with GP4 blinking.
 

chopnhack

Apr 28, 2014
1,576
Joined
Apr 28, 2014
Messages
1,576
You probably need to move the ADC acquisition code (marked "Start ADC conversion process") inside the "while (count < 64)" loop.
Thank you!! I looked at this for some time today and came up blank. I reasoned that I started the acquisition process and paused for the conversion to be done, but failed to realize that this needs to be done each time around the loop - so simple too... :oops:

so GP4 stops blinking while these delays are performed.
Makes sense as the code flows linearly. GP4 is solely for visual verification that the code is looping. Once debugged, it will be removed as I only need on port to go high.

Many thanks again North! :D:D I will amend the code and return with questions, I am sure.
 
Top