1
\$\begingroup\$

I am working on a school project where I need to read a bunch of analog values and convert them to digital with PIC16F1788(datasheet). I wrote the function using C and xc8 compiler. The problem is when I test it it doesn't behave as It should. When I remove the Potentiometer the LED stays on eventhough everything should be off since there is no analog input anymore. Here is my code thank you for your help.

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

 #include <xc.h>

// Config word
#define _XTAL_FREQ 32000000


/*config1 and config2 settings*/
void InitADC(void)
 {
 // CONFIG1
#pragma config FOSC = INTOSC // Oscillator Selection (INTOSC oscillator: I/O f   unction on CLKIN pin)
#pragma config WDTE = ON // Watchdog Timer Enable (WDT enabled)
#pragma config PWRTE = OFF // Power-up Timer Enable (PWRT disabled)
#pragma config MCLRE = ON // MCLR Pin Function Select (MCLR/VPP pin function is MCLR)
#pragma config CP = OFF // Flash Program Memory Code Protection (Program memory code protection is disabled)
#pragma config CPD = OFF // Data Memory Code Protection (Data memory code protection is disabled)
#pragma config BOREN = ON // Brown-out Reset Enable (Brown-out Reset enabled)
#pragma config CLKOUTEN = OFF // Clock Out Enable (CLKOUT function is disabled. I/O or oscillator function on the CLKOUT pin)
#pragma config IESO = ON // Internal/External Switchover (Internal/External Switchover mode is enabled)
#pragma config FCMEN = ON // Fail-Safe Clock Monitor Enable (Fail-Safe Clock Monitor is enabled)

// CONFIG2
 #pragma config WRT = OFF // Flash Memory Self-Write Protection (Write protection off)
 #pragma config VCAPEN = OFF // Voltage Regulator Capacitor Enable bit (Vcap functionality is disabled on RA6.)
 #pragma config PLLEN = ON // PLL Enable (4x PLL enabled)
 #pragma config STVREN = ON // Stack Overflow/Underflow Reset Enable (Stack Overflow or Underflow will cause a Reset)
 #pragma config BORV = LO // Brown-out Reset Voltage Selection (Brown-out Reset Voltage (Vbor), low trip point selected.)
 #pragma config LPBOR = OFF // Low Power Brown-Out Reset Enable Bit (Low power brown-out is disabled)
 #pragma config LVP = ON // Low-Voltage Programming Enable (Low-voltage programming enabled)





     //** Initalise Ports FOR ADC **//
     PORTA = 0x00; //Set ports to low
     PORTB = 0x00;
     TRISA = 0xFF; //Port A is all inputs. (set to 1)
     //** Set Up ADC Parameters **//
     ANSELA =0x2F ; // (0b00101111)All AN0-AN4(RA4 and RA7 have no ADC) of register A are set to analog input

     //ANSELH = 0x00; //Set the analog high bits to 0
     ADCON1 = 0x00; // Sets ADRESL to contain the first 7 bits of conversion, ADRESH will have the final 3 bits. And all the rest to default (FOSC/2 2 prescaler)
 } // void InitADC(void)



// Do the ADC convertion only for the channel indicated
// result of ADC is returned
 unsigned int ReadADC(unsigned char channel)
{
  unsigned int AN_Val;

     // automaticaly determines the input channel it will read the value from
     switch (channel)
         {
         case 1: // AN1 1000 0101
             ADCON0 = 0x85;
             break;
         case 2: // AN2 1000 1001
             ADCON0 = 0x89;
             break;
         case 3: //AN3 1000 1101
             ADCON0 = 0x8D;
             break;
  case 4: //AN4 1001 0001
             ADCON0 = 0x91;
             break;
         default: // Any other value will default to AN0
             ADCON0 = 0x81; // (1000 0001) - AN1 set up ADC ADCON0

 // sets the ADCON0 register for each port depending on the value of channel.
 // the result is 10-bit , The ADC is set to on, No convertion is in progress

         } // switch (channel)

     // Channel selected proceed with ADC convertion
     __delay_us(10); // sampling time
     ADCON0 = ADCON0 | 0x02; //This sets the go/!done bit that starts conversion. Bit will be cleared when ADC is complete
     while (ADCON0 & 0x02); //wait here until conversion is complete
    AN_Val = ((ADRESH << 8) + ADRESL) & 0x03ff; //result is 16 bits with 10-bits for measurement. Shift upper 8 bits left 8 bits into high byte and add low byte.
     return AN_Val;
} // int ADC(unsigned char channel)


 void main (void){

 //** Initalise Ports FOR ADC **//


  unsigned int AN1_Result;
  unsigned int AN0_Result;
  unsigned int AN2_Result;

   PORTC = 0x00;
   TRISC = 0x00; //Port C all output
  // TRISB = 0x00;

// in the following function I am trying to test and see if the code works by using a POT and changing the value and see if LEDs in port C would react to that. 
  do{
    AN0_Result = ReadADC(0);
    PORTC = AN0_Result; 

    }while(1);
}

Update #1 :

based on some of the suggestions I updated my code as follows:

The problems that I am still having are that even when there are no analog iput the led connected to PORTC is stays on. Even when I ground the input using a resistor to ground. That just tells me that there is something wrong with my ReadADC function. I went over my code again and I suspect that there is something wrong with the following lines that I dont get yet:

ADCON0 = ADCON0 | 0x02; //This sets the go/!done bit that starts conversion. Bit will be cleared when ADC is complete
while (ADCON0 & 0x02);  //wait here until conversion is complete
AN_Val = ((ADRESH << 6) + ADRESL)& 0x03ff ;  // shift the ADRESh register by 6 bits to get rid of extra 0's and then add the bottom 8bits to it which gives the whole 10bit answer. (somebody suggested to & with 0x03ff still dont know why this could be wrong)

other then that I changed the code as some of you noted and moved the adc init inside the ReadADC function and I added a couple of lines that made sense to me here it is 

 #include <xc.h>

// Config word
#define _XTAL_FREQ   500000 // set it to match internal oscillator
#DEVICE ADC=10  // set ADC to 10 bit resolution

/*config1 and config2 settings*/
 // CONFIG1
#pragma config FOSC = INTOSC // Oscillator Selection (INTOSC oscillator: I/O f   unction on CLKIN pin)
#pragma config WDTE = OFF // Watchdog Timer Enable (WDT enabled)
#pragma config PWRTE = OFF // Power-up Timer Enable (PWRT disabled)
#pragma config MCLRE = ON // MCLR Pin Function Select (MCLR/VPP pin function is MCLR)
#pragma config CP = OFF // Flash Program Memory Code Protection (Program memory code protection is disabled)
#pragma config CPD = OFF // Data Memory Code Protection (Data memory code protection is disabled)
#pragma config BOREN = ON // Brown-out Reset Enable (Brown-out Reset enabled)
#pragma config CLKOUTEN = OFF // Clock Out Enable (CLKOUT function is disabled. I/O or oscillator function on the CLKOUT pin)
#pragma config IESO = ON // Internal/External Switchover (Internal/External Switchover mode is enabled)
#pragma config FCMEN = ON // Fail-Safe Clock Monitor Enable (Fail-Safe Clock Monitor is enabled)

// CONFIG2
 #pragma config WRT = OFF // Flash Memory Self-Write Protection (Write protection off)
 #pragma config VCAPEN = OFF // Voltage Regulator Capacitor Enable bit (Vcap functionality is disabled on RA6.)
 #pragma config PLLEN = ON // PLL Enable (4x PLL enabled)
 #pragma config STVREN = ON // Stack Overflow/Underflow Reset Enable (Stack Overflow or Underflow will cause a Reset)
 #pragma config BORV = LO // Brown-out Reset Voltage Selection (Brown-out Reset Voltage (Vbor), low trip point selected.)
 #pragma config LPBOR = OFF // Low Power Brown-Out Reset Enable Bit (Low power brown-out is disabled)
 #pragma config LVP = ON // Low-Voltage Programming Enable (Low-voltage programming enabled)


// Do the ADC convertion only for the channel indicated
// result of ADC is returned
 unsigned int ReadADC(unsigned char channel)
{
  unsigned int AN_Val;
  unsigned int bit_val;

    //** Initalise Ports FOR ADC **//
     PORTA = 0x00; //Set ports to low
     PORTB = 0x00;
     TRISA = 0xFF; //Port A is all inputs. (set to 1)
     //** Set Up ADC Parameters **//
     ANSELA =0x2F ; // (0b00101111)All AN0-AN4(RA4 and RA7 have no ADC) of register A are set to analog input

     ADCON1 = 0x00; // Sets ADRESL to contain the first 7 bits of conversion, ADRESH will have the final 3 bits. And all the rest to default (FOSC/2 2 prescaler)
     // automaticaly determines the input channel it will read the value from
     switch (channel)
         {
         case 1: // AN1 1000 0101
             ADCON0 = 0x85;
             break;
         case 2: // AN2 1000 1001
             ADCON0 = 0x89;
             break;
         case 3: //AN3 1000 1101
             ADCON0 = 0x8D;
             break;
  case 4: //AN4 1001 0001
             ADCON0 = 0x91;
             break;
         default: // Any other value will default to AN0
             ADCON0 = 0x81; // (1000 0001) - AN1 set up ADC ADCON0

 // sets the ADCON0 register for each port depending on the value of channel.
 // the result is 10-bit , The ADC is set to on, No convertion is in progress

         } // switch (channel)

     // Channel selected proceed with ADC convertion
     __delay_us(10); // sampling time
     ADCON0 = ADCON0 | 0x02; //This sets the go/!done bit that starts conversion. Bit will be cleared when ADC is complete
     while (ADCON0 & 0x02); //wait here until conversion is complete
    bit_val= ADRESH
    AN_Val = ((bit_val << 8) | ADRESL); //store the ADRESH into the 16 bit int then shit it up 8 spaces after that add the ADRESL values using the and operator.
     return AN_Val;
} // int ADC(unsigned char channel)


 void main (void){

 //** Initalise Ports FOR ADC **//


  unsigned int AN1_Result;
  unsigned int AN0_Result;
  unsigned int AN2_Result;

   PORTC = 0x00;
   TRISC = 0x00; //Port C all output


// in the following function I am trying to test and see if the code works by using a POT and changing the value and see if LEDs in port C would react to that. 
  do{
    AN0_Result = ReadADC(0);
    LATC = AN0_Result; 

    }while(1);
}

UPDATE # 2

Now The ADC works, but the output is not quite correct. I tested it using 3 LED's Two leds would come on when the voltage is above 2.5 volts and a single LED would light up if it is under 2.5v. The problem is the opposite happens Two leds would come on if the voltage is under and a single one would light up if the voltage is over 2.5V. I also checked my circuit and everything seems correct.

#include <xc.h>

 // Config word
  #define _XTAL_FREQ   2000000 // set it to match internal oscillator

 // Do the ADC convertion only for the channel indicated
 // result of ADC is returned
  unsigned int ReadADC(unsigned char channel)
  {
 unsigned int AN_Val;
 unsigned int bit_val;

//** Initalise Ports FOR ADC **************************************************************//
 PORTA = 0x00; //Set ports to low
 PORTB = 0x00;
 TRISA = 0xFF; //Port A is all inputs. (set to 1)

 //** Set Up ADC Parameters **//
 ANSELA =0x2F ; // (0b00101111)All AN0-AN4(RA4 and RA7 have no ADC) of register A are set to analog input


 ADCON1 = 0xD0; // (0b11010000)
                //format setup see page 182 of datasheet
                // bit7: set for 2'complement format
                // bit6-4 : set FOSC/16
    //********************************************************************************************//

 // automaticaly determines the input channel it will read the value from
 switch (channel)
     {
     case 1: // AN1 1000 0101
         ADCON0 = 0x85;                  // bit0: ADC enabled
                                        //bit6-2: AN1 enabled for analog input
                                       //bit7: set for a 10-bit result(when its 1)
         break;
     case 2: // AN2 1000 1001
         ADCON0 = 0x89;
         break;
     case 3: //AN3 1000 1101
         ADCON0 = 0x8D;
         break;
     case 4: //AN4 1001 0001
         ADCON0 = 0x91;
         break;
     default: // Any other value will default to AN0
         ADCON0 = 0x81; // (1000 0001) - AN1 set up ADC ADCON0

    // sets the ADCON0 register for each port depending on the value of channel.
    // the result is 10-bit , The ADC is set to on, No convertion is in progress

     } // switch (channel)

 // Channel selected proceed with ADC convertion
__delay_us(10); // sampling time
ADCON0 = ADCON0 | 0x02; //This sets the go/!done bit that starts conversion. Bit will be cleared when ADC is complete
while (ADCON0 & 0x02); //wait here until conversion is complete
bit_val= ADRESH; // store upper 2 bits in a 16 bit int
AN_Val = ((bit_val << 8) | ADRESL); //store the ADRESH into the 16 bit int then shit it up 8 spaces after that add the ADRESL values using the and operator.
return AN_Val;
} // int ADC(unsigned char channel)



void main (void){

//** Initalise Ports FOR ADC **//


 unsigned int AN1_Result;
 unsigned int AN0_Result;
 unsigned int AN2_Result;

 PORTC = 0x00;
 TRISC = 0x00; //Port B all output
 //TRISB = 0x00;
 //ANSELB = 0x00; // all as port b digital as in/out
 ANSELC = 0x00;
while(1){
AN0_Result = ReadADC(1);
if(AN0_Result > 512){    // when the voltage is passed 2.5 volts these two LEDS should come on.
            /*Turn these 2 LED on*/
            LATCbits.LATC2 = 1;
            LATCbits.LATC3 = 1;
            LATCbits.LATC4 = 0;
            }
 if (AN0_Result < 512){
            /*turn one LED on*/
            LATCbits.LATC2 = 0;
            LATCbits.LATC3 = 0;
            LATCbits.LATC4 = 1;
            }
}
}
\$\endgroup\$
5
  • \$\begingroup\$ Why are your configuration settings inside of InitADC()? \$\endgroup\$
    – Dan Laks
    Mar 17, 2015 at 20:06
  • \$\begingroup\$ Have you gotten this working yet? I found a few more oddities in your code. \$\endgroup\$
    – Dan Laks
    Mar 19, 2015 at 5:03
  • \$\begingroup\$ The conversion works but the output is not correct. I am going through it and also found a couple of errors. Let me know what errors you found thanks. \$\endgroup\$
    – sam brosteain
    Mar 19, 2015 at 5:22
  • \$\begingroup\$ Let's go back to the chat room \$\endgroup\$
    – Dan Laks
    Mar 19, 2015 at 5:24
  • \$\begingroup\$ What values are you reading from the ADC when you are grounding the input (and expecting the ADC reading to be near zero)? In my experience, it is not unusual to get a value of a few counts (0-10 on a 10bit ADC) when the input is supposedly at ground potential. \$\endgroup\$
    – Morten Jensen
    Feb 22, 2017 at 19:43

2 Answers 2

Reset to default
2
\$\begingroup\$

A couple problems with your code:

  1. Your configuration settings are defined inside of InitADC() for some reason. According to the XC8 user's manual, those need to go at the very top of your code (below #include xc.h, I believe).

  2. You never actually call InitADC() anywhere.

  3. You left the watchdog timer (WDT) on, but don't clear the timer in your code. This will cause the 16F1788 to constantly reset itself. Turn off the WDT.

  4. Your configuration settings are set to use the internal oscillator. The default internal oscillator frequency is 500kHz. Yet you defined _XTAL_FREQ to be 32MHz. I believe XC8 using the _XTAL_FREQ value to calculate timing for the __delay_us() function, so your delays will be extremely inaccurate. In other words, the compiler thinks the program is running at 32MHz, but in reality, it's running at 500kHz.

  5. Always use the LAT registers to write values to the ports. Only use the PORT registers to read from the ports. The LAT registers were introduced to overcome the Read-Modify-Write problem.

  6. The compiler defines ADRESH as an 8-bit unsigned char. Left-shifting an 8-bit value 8 times will cause the value to equal 0x00 every time. Therefore, your assignment of AN_Val will always be incorrect.

update by Sam Brosteain:

  1. Most importantly set The ADCON2 register. The lower nibble of the ADCON2 register selects the source of the negative channel, which is the negative voltage reference (Vref-) for the A/D. The POR default is to use the voltage from AN0 pin as Vref-. ADCON2=0xF set Vref- to that selected by the ADNREF bit of the ADCON1 register, which in your code is configured to connect Vref- to Vss. In other words, you are converting a single-ended analog input signal referencing to ground (Vss).
\$\endgroup\$
12
  • \$\begingroup\$ I updated my post by taking into consideration what you said thanks \$\endgroup\$
    – sam brosteain
    Mar 17, 2015 at 23:53
  • \$\begingroup\$ I see you correct 1, 4, and 5. However, you still haven't actually called the InitADC() function anywhere in your code, nor disabled the WDT, nor fixed the assignment of AN_Val. All of those things are individually fatal. \$\endgroup\$
    – Dan Laks
    Mar 18, 2015 at 0:01
  • \$\begingroup\$ Ok, I see you just copied the contents of InitADC() and pasted it into ReadADC(). Not really the best way to that, but it won't break your code. The other problems in my comment above are still valid. \$\endgroup\$
    – Dan Laks
    Mar 18, 2015 at 0:04
  • \$\begingroup\$ Sorry for the WDT i did it in my code and forgot to reflect it here as for the initADC i removed it and initialized the ADC inside the ReadADC function is that fine??. Also I am really confused on the AN_Val part I shifted the bits by 6 to get rid of the upper 6 0' then I added the lower 8 bits to it is that correct? \$\endgroup\$
    – sam brosteain
    Mar 18, 2015 at 0:05
  • 1
    \$\begingroup\$ @sambrosteain, what are the values of ADRESH and ADRESL now in the debugger? \$\endgroup\$
    – Dan Laks
    Mar 20, 2015 at 4:33
2
\$\begingroup\$

There is always an analog input.

When the potentiometer is connected the input value is whatever the potentiometer is set to.

When the potentiometer is disconnected the input value is whatever it fancies being. Usually somewhere around the middle of the range, maybe a little below.

With the input disconnected it's just picking up noise from the surrounding area and using that as the input.

You need to replace the potentiometer with a resistor to ground when the potentiometer is not connected.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.