Trouble with Subtraction after ADC

[Dalaran]

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:

#include <htc.h>
#include <math.h>
#include <stdio.h>
#define _XTAL_FREQ 4000000
//#define bitset(var,bitno) (var|=1<<bitno)
//#define bitclr(var,bitno) (var&=~(1<<bitno))



__CONFIG(INTIO & WDTDIS & PWRTEN & MCLRDIS & UNPROTECT & UNPROTECT & BORDIS);

void main(void){
	
	int x = 0;
	int IRone = 0;		//AN6/RC2
	int IRtwo = 0;		//AN7/RC3
	int IRdiff = 0;		//for difference in IR measurements
	PORTA = 0x00;		// PORTA low
	PORTC = 0x00;		// PORTC low
	CMCON0 = 0x07;		// turn off comparators
	ADCON0 = 0b10011001;		//turn on ADC AN6 selected - right justified
	ADCON1 = 0b00010000;		//ADC clock select FOSC/8 - 2us @ 4Mhz

	TRISA = 0x00;				//Port A outputs
	TRISC = 0b00001100;			//RC2 + RC3 input pins (AN6/AN7)
	ANSEL = 0b11000000;			//AN6/AN7 selected as analogue input pins

	ADIE = 1;		//enable ADC interrupt
	ADIF = 0; 		//ensure ADC interrupt flag is cleared
	
	while(1){

		ADCON0 = 0b10011001;		//ADC ON - AN6
		ADCON0 = 0b10011011;		//ADC GO - AN6		
			while(ADIF = 0){		//wait for ADC interrupt flag
				_delay(1);
			}
		ADIF = 0;					//clear ADC interrupt flag
		IRone = (ADRESH*256) + ADRESL;		//calc IRone - right justified 

		ADCON0 = 0b10011101;		//ADC ON - AN7
		ADCON0 = 0b10011111;		//ADC GO - AN7
			
			while(ADIF == 0){		//wait for ADC to finish
				_delay(1);
			}
		ADIF = 0;					//clear ADC interrupt flag
		IRtwo = (ADRESH*256) + ADRESL;		//calc IRtwo - right justified
		
		IRdiff = IRone - IRtwo;
		
		if(IRdiff >= 0){
			PORTA = 0b00000001;
				for(x = 0; x <=13; x++){
					__delay_ms(20);
				}
		}
		else{
			PORTA = 0b00000000;
		}
	}
}

 

[Pommie]

From selecting the channel to starting the conversion you need a small delay for the sample and hold capacitor to charge. Normally around 20uS is sufficient.

        ADCON0 = 0b10011001;		//ADC ON - AN6
        //delay here
        ADCON0 = 0b10011011;		//ADC GO - AN6

Mike.

 

[Dalaran]

Thanks Mike. Forgot about that. I tried adding a 50us and even up to 2ms delay to be safe (for both channel AN6 and AN7 selection) and am still getting the same results however.

Thanks for the quick reply.

 

[Pommie]

Are you sure that IRdiff is signed?

Try,

    if(IRone>IRtwo){
        ...

Edit, just checked and HiTech C does default to signed as expected.

Mike.

 

[Dalaran]

Thanks eh.

Same result as before.

To test both ADC calculations I set up two if/else statements turning 'high' 2 different pins when AN6 or AN7 reached a specific value. This way I could have none, either one, or both LEDs on depending what voltages were supplied to AN6/AN7. This worked exactly as expected. I was very surprised when I changed this to one if/else statement using both terms IRone and IRtwo and got wacky results.

Maybe I need to sleep on this almost 4am here.

Thanks again Mike. If you have any other ideas I'm all ears.

 

[Nigel Goodwin]

Thanks Mike. Forgot about that. I tried adding a 50us and even up to 2ms delay to be safe (for both channel AN6 and AN7 selection) and am still getting the same results however.

You don't show your circuit, or what the source impedance feeding the pins is, depending on that value 2mS may be no where near long enough.

 

[Dalaran]

Thanks Nigel. I'm not sure of the impedance of the source supplying the PIC but I can look into this when I get home. Basically the rest of the schematic is RA0 through a 300Ohm resistor, to LED then ground, and AN6/AN7 are each connected to its own voltage divider resistor network for the time being. This is just to simulate different voltages on the pins. I will try increasing the delay and check the source impedance when I get home this evening.

Thanks again.

 

[Nigel Goodwin]

Thanks Nigel. I'm not sure of the impedance of the source supplying the PIC but I can look into this when I get home. Basically the rest of the schematic is RA0 through a 300Ohm resistor, to LED then ground, and AN6/AN7 are each connected to its own voltage divider resistor network for the time being. This is just to simulate different voltages on the pins. I will try increasing the delay and check the source impedance when I get home this evening.

Assuming your dividers go directly to an HT rail, then the impedance depends solely on them, what values are they?.

Basically the analogue input pin has to have enough current to charge (and discharge) the internal sample-and-hold capacitor when you switch channels. This is why my analogue PIC tutorial uses an opamp buffer to drive it.

 

[Dalaran]

They are around 5k. And yes, directly connected to the rail. Good advice, thanks.

I'll have a better look when I get home.

Always appreciated.

 

[Dalaran]

So I've had a chance to do some more trouble shooting on this project this evening. 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.

Any ideas why this might be happening?

I have also added delays between turning on/selecting ADC channel and setting the GO bit, which I had originally left out.

 

[Hayato]

Maybe, just maybe.

Your code is:

while(ADIF [B]=[/B] 0){		//wait for ADC interrupt flag
				_delay(1);
			}

What about:

while(ADIF [B]==[/B] 0){		//wait for ADC interrupt flag
				_delay(1);
			}

And what about polling the GO/#DONE bit instead of using interrupts?

 

[Dalaran]

Hehe, thanks for your reply. I realized shortly after I posted the code the first time my compiler was giving me a warning for that. I changed to '==' and it had no affect. I've been doing all my testing with the == command.

Thanks though.

 

[Dalaran]

Ok I think I'm going crazy... The code seems to be doing the opposite...

Here is the updated while loop.

	while(1){

		ADCON0 = 0b10011001;		//ADC ON - AN6
			__delay_ms(2);
		ADCON0 = 0b10011011;		//ADC GO - AN6		
			while(ADIF = 0){		//wait for ADC interrupt flag
				_delay(1);
			}
		ADIF = 0;					//clear ADC interrupt flag
		IRone = (ADRESH*256) + ADRESL;		//calc IRone - AN6 


		ADCON0 = 0b10011101;		//ADC ON - AN7
			__delay_ms(2);
		ADCON0 = 0b10011111;		//ADC GO - AN7
			
			while(ADIF == 0){		//wait for ADC to finish
				_delay(1);
			}
		ADIF = 0;					//clear ADC interrupt flag

		IRtwo = (ADRESH*256) + ADRESL;		//calc IRtwo - AN7

		//IRdiff = IRtwo - IRone;
		
		if(IRone >= 512){
			RA0 = 1;
				for(x = 0; x <=13; x++){
					__delay_ms(20);
				}
		}
		else{
			RA0 = 0;
		}
	
		if(IRtwo >= 512){
			RA2 = 1;
				for(x = 0; x <=13; x++){
					__delay_ms(20);
				}
		}
		else{
			RA2 = 0;
		}	
	
	}

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.

edit: also using unsigned int declarations for IRone and IRtwo.

 

[kchriste]

IRtwo = (ADRESH*256) + ADRESL;

Since ADRESH is a 8bit variable, you may need to cast the result of the multiplication to int since the result will overflow a char:
IRtwo = (int)(ADRESH*256) + ADRESL;

 

[birdman0_o]

Do you have an LCD or something, actually better idea. Start by creating a code that displays ADRES:HL on 10 leds for both AN6 and AN7 with a delay in between obviously, and see if the values check out, back to the basics.

 

[Dalaran]

Hey, thanks for the reply. Makes sense.
I changed to this where ADCHigh1 and ADCLow1 were cast as int type.

		ADCHigh1 = ADRESH;
		ADCLow1 = ADRESL;
		IRone = (ADCHigh1*256) + ADCLow1;

Again no luck. I thought that was it for sure, and would have made sense for the specific output to function in reverse, but here I actually have the ADC inputs controlling opposite outputs than I have dictated, so it seems. Confused I am heh. Thanks though.

 

[Dalaran]

Hey Birdman, yup looks like I might have to break down to this. Sounds like a tomorrow project since my bedtime is soon approach and I'm most likely a couple LED's short hehe. Good advice and always. Thanks.

 

[Pommie]

In the latest code you posted there is still the same mistake,

	while(1){

		ADCON0 = 0b10011001;		//ADC ON - AN6
			__delay_ms(2);
		ADCON0 = 0b10011011;		//ADC GO - AN6		
			[COLOR="Red"]while(ADIF = 0){[/COLOR]		//wait for ADC interrupt flag
				_delay(1);
			}
		ADIF = 0;					//clear ADC interrupt flag
		IRone = (ADRESH*256) + ADRESL;		//calc IRone - AN6 


		ADCON0 = 0b10011101;		//ADC ON - AN7
			__delay_ms(2);
		ADCON0 = 0b10011111;		//ADC GO - AN7
			
			while(ADIF == 0){		//wait for ADC to finish
				_delay(1);
			}
		ADIF = 0;					//clear ADC interrupt flag

		IRtwo = (ADRESH*256) + ADRESL;		//calc IRtwo - AN7

		//IRdiff = IRtwo - IRone;

Might I suggest you change it to,

    while(1){    
        ADCON0 = 0b10011001;		//ADC ON - AN6
        __delay_ms(2);
        GODONE=1;
        while(GODONE);		//wait for ADC interrupt flag
        IRone = (ADRESH*256) + ADRESL;		//calc IRone - AN6 
        
        ADCON0 = 0b10011101;		//ADC ON - AN7
        __delay_ms(2);
        GODONE=1;
        while(GODONE);		//wait for ADC interrupt flag
        IRtwo = (ADRESH*256) + ADRESL;		//calc IRtwo - AN7

Mike.

 

[Dalaran]

Mike once again to set me free. Your amazing buddy! Seems a second set of eyes never hurts. I changed the first while loop to ADIF == from ADIF = and the problem is solved.

Mike Mike Mike! hehe, thanks mate.

I will take a look at the script you posted as it seems to be alot simpler than what I was using. Didn't realize you could just watch the GODONE pin instead of waiting for an interrupt, this way it is automatically cleared after a conversion.

edit: I guess I should give some love to Hayato too since it appears I don't listen the first time. Again all, the help is much appreciated.

 

[Pommie]

You should really be thanking Hayato as it was him/her that spotted it first.

Mike.