Problem with making function for reading ADC multi channel.

Go To Last Post
12 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The ADC only reads one channel whereas I've change the ADMUX. It works fine if I use only one channel.
Please help me . . .
I'm using F_CPU 1MHz
Here is the header file:

uint16_t ADValue;
void InitializeADC(void)
{
	ADCSRA|=1<<ADEN;//turn on ADC feature
	ADMUX |=(1<<ADLAR) | (1<<REFS0);//Left adjustment, VCC as reference 0b0110 0xxx
	ADCSRA|=1<<ADIE;
	ADCSRA|=1<<ADPS2;//prescaler div fac 16
	sei();
	ADCSRA|=1<<ADSC;
}

ISR(ADC_vect)
{
	uint8_t low=ADCL;
	uint8_t high=ADCH;
	ADValue= (high<<2)|(low>>6);
	ADCSRA|=1<<ADSC;
}

uint16_t Getadc(unsigned int channel)
{
	switch(channel)
	{
		case 0:
		ADMUX &=~(0x0F);
		return ADValue;
		break;
		case 1:
		ADMUX =(0x01)|ADMUX &(0xF0);
		return ADValue;
		break;
		case 2:
		ADMUX =(0x02)|ADMUX &(0xF0);
		return ADValue;
		break;
		case 3:
		ADMUX =(0x03)|ADMUX &(0xF0);
		return ADValue;
		break;
		case 4:
		ADMUX =(0x04)|ADMUX &(0xF0);
		return ADValue;
		break;
		case 5:
		ADMUX =(0x05)|ADMUX &(0xF0);
		return ADValue;
		break;
		case 6:
		ADMUX =(0x06)|ADMUX &(0xF0);
		return ADValue;
		break;
		case 7:
		ADMUX =(0x07)|ADMUX &(0xF0);
		return ADValue;
		break;
		default:
		ADMUX &=0xF0;
		return ADValue;
		break;
	}
}

and the c file is:

volatile char adcResult[4],adcResult1[4];
int main(void)
{
	InitializeLCD(OFF,Unblink);
	InitializeADC();
    while(1)
    {	
        volatile uint16_t buffer;
		volatile uint16_t buffer1;
		
		buffer1=Getadc(4);
		Gotoxy(1,2);
		itoa(buffer,adcResult1,10);
		Send_String("ADC:");
		Send_String(adcResult1);
		Send_String(" CH4");
		Send_String("             ");
		
		
		buffer=Getadc(3);
		itoa(buffer,adcResult,10);
		Gotoxy(1,1);
		Send_String("ADC:");
		Send_String(adcResult);
		Send_String(" CH3");
		Send_String("             ");
		
		sei();
    }
}

about clearing the ADMUX, should "ADMUX =(0x01)|ADMUX &(0xF0);" do the job?
Or what else might be issue here in the code?
Thank You![/code]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sorry, it was indeed a silly mistake.

buffer1=Getadc(4);
		itoa(buffer,adcResult,10);
		Gotoxy(1,2);
		Send_String("ADC:");
		Send_String(adcResult);
		Send_String(" CH4");
		Send_String("             ");

in the code above, I forgot to change "buffer" in the itoa() to "buffer1". It works now . . . :)

addition: It's better using sprintf rather than itoa in this case.

Last Edited: Tue. Jan 28, 2014 - 01:27 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why do you bother with ADLAR and then that <<2 >>6 stuff? Why not simply forget ADLAR and then just read ADC (not separate ADCL/ADCH)?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

just for fun, Sir. I try to implement things I've read and learnt.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I try to implement things I've read and learnt.

Yes but the art of microcontroller programming is usually to try and make things as small, fast and efficient as possible. You can increment a variable using:

int x = 5;
x = x + pow(cos(49.0 * 3.1416 / 180.0), 2.0) + pow(sin(49.0 * 3.1416 / 180.0), 2.0);

but most people would use:

int x = 5;
x++;

(actually they wouldn't - if they knew they wanted 6 in it they'd probably put it there in the first place!)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

int x = 5;
x = x + pow(cos(49.0 * 3.1416 / 180.0), 2.0) + pow(sin(49.0 * 3.1416 / 180.0), 2.0); 

So, how did you ever figure that out?

The largest known prime number: 282589933-1

Without adult supervision.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
uint16_t Getadc(unsigned int channel)
{
   switch(channel)
   {
      case 0:
      ADMUX &=~(0x0F);
      return ADValue;
      break;
      case 1:
      ADMUX =(0x01)|ADMUX &(0xF0);
      return ADValue;
      break; 
...

Wow--all that to set the channel?

A) You are resetting the channel in this function. But you process the data in the ISR, and start the next conversion. How are you sure which channel you are reading?

B) You are much smarter than I am, knowing that in the "ADMUX =(0x01)|ADMUX &(0xF0); " expression that the & is a higher precedence than the | . (In the long run, a recipe for disaster IME/IMO)

C) Why not just rebuild ADMUX each time?

ADMUX = (1<<ADLAR) | (1<<REFS0) | (unsigned char)channel

That replaces your whole function code.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
So, how did you ever figure that out?

It's simply the equation of a circle ;-)

You probably know x^2 + y^2 = r^2? Well it's also true that X = r * cos(theta) and y = r * sin(theta) as well. And a conversion from degrees to radians is theta * Pi / 180.

A unit circle has r = 1.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@clawson: Lol, you are too much. I mean, when I see the code, I will know what ADLAR is. In case for long next time that I forget.
@ theusch

Quote:
A) You are resetting the channel in this function. But you process the data in the ISR, and start the next conversion. How are you sure which channel you are reading?

I am recently learning about multiplexer in my lecture and know a little bit about it.
I've simulate it in the proteus and it works. However, not in real micro yet.

Quote:
B) You are much smarter than I am, knowing that in the "ADMUX =(0x01)|ADMUX &(0xF0); " expression that the & is a higher precedence than the | . (In the long run, a recipe for disaster IME/IMO)

I don't know either, but I know that the C will process operator in the right first in case they are as powerful.

Quote:
C) Why not just rebuild ADMUX each time?

ADMUX = (1<<ADLAR) | (1<<REFS0) | (unsigned char)channel

That replaces your whole function code.

Why should I? What would be the effect If I replace the code?

Btw, thanks for all the response. I was inspired by the BasicCompiler. I see that in Basic, we can simply get the ADC just by using "getADC" function.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Why should I? What would be the effect If I replace the code?

It's a single assignment not a read-modify-write. Apart from anything else that makes it atomic which is sometimes/often important.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I don't know either, but I know that the C will process operator in the right first in case they are as powerful.


:roll:
http://en.wikipedia.org/wiki/Ope...

Quote:

...
10 & Bitwise AND Left-to-right
11 ^ Bitwise XOR (exclusive or) Left-to-right
12 | Bitwise OR (inclusive or) Left-to-right

...


And looking at the table, it is maybe half left-to-right. I'm glad that, besides the precedence, you have all of that info in rote memory as well.

Quote:

Why should I? What would be the effect If I replace the code?


Instead of your 50 lines of switch/case, that may or may not be exhaustive of all channels, you replace with one assignment statement. Lots faster development time. Easier to read/maintain, IMO. Eliminates any question about precedence. No RMW situation. A few AVR instructions; maybe 4--I'd wager your routine is 100 or more. 100 words is 5% of a Mega48. Suit yourself.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oh, yeah, Proteus can tell you whatever you want to hear. The fact remains that with the code you posted, you will almost always (but not always--might even be worse) get the result of the previous channel and not the current channel.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.