PIC18F1220 ADC problem again

Discussion in 'Embedded Systems and Microcontrollers' started by c_calvin, Aug 17, 2009.

  1. c_calvin

    Thread Starter New Member

    Aug 16, 2009
    4
    0
    I'm trying to use 2 ADC on this PIC to monitor 2 voltage, if either one of the voltage drop below 3, the LED will turn off. The problem i'm facing now is if I apply 4V on AN2, the LED will turn off, but if i apply voltage on AN0, the LED wun turn off.

    What more weird is when i change the statement
    if (result <3 || result2 <3)

    PORTB =0b11111111; //LED ON

    to

    if (result <3 && result2 <3)

    PORTB =0b11111111; //LED ON

    then it would work. i apply 4V to either one of the pin, the LED will turn off. Is there any problem with my code? Any help is appreciated. thanks.

    //program to turn on LED if voltage above the preset voltage-PIC18F1220

    #include <p18cxxx.h>
    #include <delays.h>
    #pragma config WDT = OFF //watchdogtimer off
    #pragma config LVP = OFF //lowvoltageprogramming off
    #pragma config OSC = INTIO2 //select internal oscillator
    #pragma config BOR = OFF // brownout reset off
    #pragma config MCLRE = OFF //master clear off
    #pragma code

    float result;
    float result2;

    void getVoltage(void) //func to read ADC
    {
    ADCON0 =0x0; //select Vdd Vss as reference, select input AN0,
    ADCON2=0x27; // left justified, use internal conversion clock
    ADCON0bits.ADON = 1 ;
    ADCON0bits.GO = 1; //start ADC
    Delay100TCYx (2);
    while(ADCON0bits.GO == 1); //wait for completion
    result=(ADRESH*0.018); // x with 0.018 to convert back to actual value

    Delay100TCYx (30);
    ADRESH = 0x0;
    ADCON0bits.ADON = 0 ;
    Delay100TCYx (2);

    ADCON0 =0x8; //select Vdd Vss as reference, select input AN2
    ADCON2=0x27; // left justified, use internal conversion clock
    ADCON0bits.ADON = 1 ; //enable ADC
    ADCON0bits.GO = 1; //start ADC
    Delay100TCYx (2);
    while(ADCON0bits.GO == 1); //wait for completion
    result2=(ADRESH*0.018); // x with 0.018 to convert back to actual value

    }

    void main (void)
    {
    OSCCON = 0x43; //set internal freq 1Mhz
    ADCON1=0x70; //select AN0 and AN2 as analog

    TRISA = 1; //set portA as input
    TRISB = 0; //set port B as output
    PORTB = 0b00000000; //reset port B

    while (1)
    {
    getVoltage();
    if (result <3 || result2 <3)

    PORTB =0b11111111; //LED ON

    else
    PORTB = 0b00000000; //LED OFF

    }

    }
     
    Last edited: Aug 17, 2009
  2. rjenkins

    AAC Fanatic!

    Nov 6, 2005
    1,015
    69
    Hi,
    check the TRISA setting - if you are actually writing '1', only bit 0 is set and the other TRIS bits will be at 0 and have the output drivers enabled.

    That could be messing up the ADC input pins.


    I'd do the ADC scaling differently; use smaller step, say 100mV rather than 1 volt, and divide the ADC result by an integer rather than multiplying by a fractional float.

    One thing is an integer operation is faster than a float, the other is that unless you cast all variables to float and then the result back to int, some compilers may 'optimise' the math to int * int and round the fraction to zero, so your result would always be zero.
     
  3. c_calvin

    Thread Starter New Member

    Aug 16, 2009
    4
    0
    Thanks for the help. the program is working fine now.
    i did what u say by setting TRISA = 0b11111111 and also clear PORTA in the main program.

    i don't quite understand what u mean by do the ADC scaling differently, can u give me a simple example? I'm using Vdd(4.6v) and Vss as my Vref. so what i did was divide 4.6 by 256 which is 0.018

    what is the better way for me to do the scaling?
     
  4. rjenkins

    AAC Fanatic!

    Nov 6, 2005
    1,015
    69
    Ok on the scaling methods.
    Sorry, it's me confusing things, I'd not noticed that the 'result' values were declared as float, I was assuming ints. Doh..

    I tend to stick with ints for speed, so I'd have done it slightly differently but using floats your method is fine.

    A couple of very trivial bits - cosmetic more than functional?
    Possibly change the comparisons to use float constants;
    if (result < 3.0 || result2 < 3.0)

    and explicitly cast the ADC to float:
    result=(float)ADRESH*0.018; // x with 0.018 to convert back to actual value

    I'ts good to be consistent with types, it avoids the compiler doing any unintended automatic type conversions, which sometimes cause unexpected results and can be a serious pain to find.

    (Before spotting the float declaration, I thought the compiler could be converting everything to int so you were multiplying by 0, hence the query on the scaling).
     
Loading...