12 bit ADC conversion simple code not working as expected

Question

I posted a couple days ago a couple of functions that I wrote to do 10-bit ADC. the problem is I couldn't figure out what was wrong with my code so I wrote a much simpler single function that does a 12bit conversion for an easier troubleshooting of the orginal code. So my code works now for the most part but I have one small issue that I can't figure out by my own. I set 2 leds to light up when input voltage is above 2.5 volts and only one to light up when it is under. The problem is when the voltage is under 1v the two leds still light up. but besides that everything works as expected as soon as I go above 1v the single LED comes on then When I go above 2.5 volts the other 2 LED light up as expected. I would really appreciate any suggestions Thank you for your help. I am using PIC16f1788

  #include <xc.h>

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

/*config1 and config2 settings*/
// CONFIG1
#pragma config FOSC = INTOSC // Oscillator Selection (INTOSC oscillator: I/O function on CLKIN pin)
#pragma config WDTE = OFF // Watchdog Timer Enable (WDT enabled)
#pragma config PWRTE = ON // Power-up Timer Enable (PWRT disabled) (turned on!)
#pragma config MCLRE = OFF // MCLR Pin Function Select (MCLR/VPP pin function is MCLR) (turned off)
#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 = OFF // Low-Voltage Programming Enable (Low-voltage programming enabled) (turned off)


unsigned int analog_reading;
unsigned int i;
unsigned int bit_val;

void main(void)
 {
    //SET UP
ADCON1= 0b00000000; //format setup see page 182 of datasheet
TRISA= 0b11111111; //sets PORTA as all inputs
TRISC= 0b00000000; //sets PORTB as all outputs
PORTC= 0b00000000; //sets all outputs of PORTB off to begin with
ADCON0 =0b00000101;
               // bit0: ADC enabled
               //bit6-2: AN1 enabled for analog input
               //bit7: set for a 12-bit result
               //


while (1)
{
__delay_ms(10);
ADCON0bits.GO_nDONE=1; //do A/D measurement
while(ADCON0bits.GO_nDONE==1); 
bit_val= ADRESH; // store upper 8 bits in a 16bit variable
analog_reading = ((bit_val << 4) | ADRESL); //shift it up by 4 bits then store      ADRESL in the bottom 4bits
                                        // According to page 184 of datasheet   when ADFM=0 ADRESH has the
                                        // upper 8 bits of 12bit conversion results and the lower 4 bits are in ADRESL



 if(analog_reading>512){    // when the voltage is passed 2.5 volts these two  LEDS should come on.
            /*Turn these 2 LED on*/
            PORTCbits.RC2 = 1;
            PORTCbits.RC3 = 1;
            PORTCbits.RC4 = 0;
            }
else{
            /*turn one LED on*/
            PORTCbits.RC2 = 0;
            PORTCbits.RC3 = 0;
            PORTCbits.RC4 = 1;
            }
  }
  }

Answer 1

ADCON1= 0b00000000; //format setup see page 182 of datasheet

I think that's not right, for a start. That register controls more than just the format. Bits 0-2 control the voltage references (000 is ok for that - VDD and VSS). Bit 7 controls the format. But, bits 4-6 control the conversion clock.

You set those to 000 - that equates to \$F_{OSC}/2\$. That's a 16MHz internal clock source divided by 2 (judging by your config settings). So an 8MHz conversion clock. That's faaaast. If you look at Table 17-1:

A 16MHz clock, with \$F_{OSC}/2\$ is 125ns clock period, with a pointer to note 2, which states:

These values violate the minimum required T AD time.

So you are violating your \$T_{AD}\$ time, eh? So maybe the ADC isn't actually able to return any meaningful results then?

Maybe you should try slowing down the ADC so that it will operate within specifications - maybe to \$F_{OSC}/16\$ as the fastest valid setting for a 16MHz clock source.

That would equate to a setting of 0b101, which would make that line of code:

ADCON1= 0b01010000; //format setup see page 182 of datasheet

Another issue with that register is your format. You have bit 7 set to 0. That equates to "Sign and Magnitude" result. It's better to work with the 2's complement result, since that is "normal" maths in a microcontroller. You can see the difference in Table 17-2:

At the moment you are shifting your high value left 4 bits then ORing it with the lower 8 bits. With "Sign and Magnitude" you end up with, for the example there of +2355:

ADRESH = 10010011
ADRESL = 00110000

ADRESH << 4 = 100100110000
OR ADRESL = 100100110000

That is assuming it even keeps the highest 4 bits and doesn't truncate them (not sure off hand which it'll do).

That result is obviously wrong since the lower 4 bits of the ADRESH overwrite the upper 4 bits of ADRESL.

Using 2's complement mode, and shifting 8 bits not 4 results in:

ADRESH = 00001001
ADRESL = 00110011

ADRESH << 8 = 0000100100000000
OR ADRESL = 0000100100110011

And that is your full 12 bits.

So first you need to set that 7th bit of your configuration to 1, so finally it is:

ADCON1= 0b11010000; //format setup see page 182 of datasheet

And then you need to fix your bit shifting:

analog_reading = ((bit_val << 8) | ADRESL); //shift it up by 8 bits then
                                            //store ADRESL in the bottom 8 bits