I am facing some problem with code regarding ADC

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

I am trying to implement a simple adc filter which take 8 samples & returns rms value of those samples.
Problem is that when I introduced this function into my code & call the function in main it doesn't show any value in LCD but when I bypass the function my code running correctly.

Here is my code:

#include
#include
#include
#include"lcd.h"

void adc_init(void)
{
	ADMUX |=(1<<REFS0);
	ADCSRA |=(1<<ADPS1)|(1<<ADPS0)|(1<<ADFR)|(1<<ADEN);
}

uint16_t adc_read(void)
{
	uint16_t adc_tmp=0;
	ADCSRA |=(1<<ADSC);
	while(ADCSRA & (1<<ADSC))
	{	
		adc_tmp=ADCL;
		adc_tmp +=(ADCH<<8);
		return(adc_tmp);
	}
}

uint16_t adc_filter(void)
{
	uint8_t i=0;
	uint16_t adc_value=0;
	uint32_t tmp_value=0;

	for(i=0; i>8; i++)
	{
		adc_value=adc_read();
		tmp_value +=(adc_value)*(adc_value);
		_delay_us(10);
	}
	tmp_value=(tmp_value>>3);
	adc_value=(sqrt(tmp_value));
	return(adc_value);
	
}		


int main(void)
{
	uint16_t tmp, adc_result=0;

	char i_buffer[10];

	adc_init();
	lcd_init(LCD_DISP_ON_CURSOR);
	lcd_puts("V=");
	_delay_ms(50);
	tmp=adc_read();
	while(1)
	{
		adc_result=adc_filter();
		itoa(adc_result, i_buffer, 10);
		lcd_gotoxy(2,0);
		lcd_puts(i_buffer);
		_delay_ms(50);
		lcd_gotoxy(0,1);
		lcd_puts("ADC Testing-2");
		_delay_ms(150);
	}	
	return(0);
}

uC=Atmega8L, clk=1MHz, AVR gcc 4.19

Plz advice me to sort out my problem or suggest better way to implement function 'adc_filter'.

Thanks

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

What do you expect to happen here?

for(i=0; i>8; i++) 

I think you made a very small, but killing typing error here, or you do not understand how the for loop works ;)

regards

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
   while(ADCSRA & (1<<ADSC))
   {   
      adc_tmp=ADCL;
      adc_tmp +=(ADCH<<8);
      return(adc_tmp);
   } 

And this is another loop with an ill-logic.

Stefan Ernst

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

Look here for an example code
http://maxembedded.com/2011/06/2...

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

meslomp wrote:
What do you expect to happen here?

for(i=0; i>8; i++) 

I think you made a very small, but killing typing error here, or you do not understand how the for loop works ;)

regards

Thank you for pointing my mistake. I can assure you I fully understand how loops works in C. Now I wondering how I miss this. However I corrected. Now when I am going to compile my code in AVR gcc a new thing happens in compiler. A new window automatically pop-up programming window automatically switch to disassmbler window. I haven't any clue about that. Is that normal? Why this happens & what is the reason behind this?

My screen shot

Thank you very much again

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

Last Edited: Mon. May 27, 2013 - 03:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sternst wrote:

   while(ADCSRA & (1<<ADSC))
   {   
      adc_tmp=ADCL;
      adc_tmp +=(ADCH<<8);
      return(adc_tmp);
   } 

And this is another loop with an ill-logic.

I have already tried this function another way

while(ADCSRA & (1<<ADSC))
   return(ADC);

& found some warning

../mintu.c: In function 'adc_read':
../mintu.c:19: warning: control reaches end of non-void function

both time.
Now I am requesting to all if someone suggest me better logic for 'adc_read' function so that I can avoid that warning.

Thanks

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Visovian wrote:
Look here for an example code
http://maxembedded.com/2011/06/2...

Thanks for your answer. Actually I have already read it.

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Do

while (a); // or while(a) {}
do_something;

and

while (a) {
do_something;}

do the same thing?

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

Thanks
Ok I will do.
But what about my other problem

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Do

while(ADCSRA & (1<<ADSC))
   return(ADC); 

and

while(ADCSRA & (1<<ADSC)){}
return(ADC); 

have the same meaning?

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

Quote:
Actually I have already read it.
Then why not try adc_read() function from there?

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

It is a better function, as it is flexible enough to select a MUX channel... and the waiting_for_DAC_ready loop does not return early;

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

dbrion0606 wrote:
Do

while(ADCSRA & (1<<ADSC))
   return(ADC); 

and

while(ADCSRA & (1<<ADSC)){}
return(ADC); 

have the same meaning?

No of course not.

Thanks

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Is performance important? Or do you sort of do the calc 'batch mode' where you take the 8 samples first, then calc the rms? If you want to calc the rms of the last 8 samples on the fly on a per sample basis, I'd keep an array of the samples squared and their total. On ea new sample, subtract off the old sample from the total, add in the new one, and save the sampsquared sample where the old one was. An add and a subtract is faster than 8 adds. The payoff would be bigger if the record was longer I guess.

Imagecraft compiler user

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

Quote:

No of course not.

You wrote the first when you meant the second ;-)

Just one additional semicolon would have done the trick.

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

I probably think my problem is solved.

Thank you for bearing me.
But if I face any problem regarding ADC I will post here & will abuse your kindness.

Till then thanks to all again

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

First of all while I was writing my last post in this thread reply from "bobgardner" & "clawson" were not updated this page on that time(courtesy to slow server response from my ISP)so I missed their reply. Plz don't consider it my ignorance & do forgive me for this.

bobgardner wrote:
Is performance important? Or do you sort of do the calc 'batch mode' where you take the 8 samples first, then calc the rms? If you want to calc the rms of the last 8 samples on the fly on a per sample basis, I'd keep an array of the samples squared and their total. On ea new sample, subtract off the old sample from the total, add in the new one, and save the sampsquared sample where the old one was. An add and a subtract is faster than 8 adds. The payoff would be bigger if the record was longer I guess.

This is great idea!!
Actually readings from ADC are not stable in 1st phase of my code so I try to introduce a filter some extent in 'adc_filter'. Primary job of my uC isn't signal processing but depending on ADC reading it will have to do lots of other works. But if anyone has any idea for better filtering (not too extensive)is always welcome here.

clawson wrote:
Quote:

No of course not.

You wrote the first when you meant the second ;-)

Just one additional semicolon would have done the trick.

Yes. I corrected it.

Thank you very much both of you for your reply.

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Filtering is usually some sort of low pass filter rather than an rms calculation. I use something like averagevalue=averagevalue + .125*(current value - average value). I used .125 because its a divde by 8 which is a right shift 3. Thanks for the complement on my rms routine. It runs like a scalded dog.

Imagecraft compiler user

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

I am seriously considering your idea.

averagevalue=averagevalue + .125*(current value - average value)

What do you think how many samples would be sufficient?
If I increase or decrease the value of (.125) how does it affects the result?
Can you explain a little bit on algo of this formula?

Thanks

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Suppose your average value is 1 and the input becomes zero; the first value your linear, infinite response filter will give you will be (1-0.125), the second (1-0.125)** 2, the Nth (1-0.125)**N (and it will never reach zero, as it has an infinite memory. Computations are straigtforwards if you modify .125 (used to avoid a division).
OTOH, if you have a 8/10 samples buffer, you can do a finite memory filter: with trivial means, the output will decay linearly towards zero, and the true value will be reached within 8 steps.... (it wonot be reached with an infinite response filter)...

You can also do non linear filtering; Arduino sodar library newping (http://code.google.com/p/arduino... )does median filtering (instead of the average, they compute the median with a 2N+1 buffer: it needs to sort the incoming buffer). This resists to outliers, untill there are more than 50% of them...

ex median (0,1,2,444,5,6,7 ) is 5 -average would be greater than 60-
If one feels/bets there are less outliers -say 1%-, a solution is to compute the maximum and the mininimu from the buffer and to compute a mean from the remaining values -ie, the values whose positions are not the two extremes : this formulation is tricky, but allows for multiple maxima, say...- .

ex : trimmed_mean(0,1,2,444,5,6,7) is 4.2

If your avr has an usart and you have time and skills to operate it, you may transmit the values to your PC, plot them and (with Excel-maybe-, R, octave/matlab) find out the best filter you need (if any)

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

Thanks for your explanation

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

According to this formula

avg=avg+0.125*(currentvalue-avg)

my code

uint16_t adc_filter(void)
{
	uint8_t i=0;
	uint16_t avg=0, current_value=0;
	while(i<8)
	{
		current_value=adc_read();
		avg +=((current_value-avg)>>3);
		i++;
	}
	return(avg);
}

Is my code calculating value as well as the formula or I have done something wrong?Actually I am no getting any value. :(

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Quote:

Is my code calculating value as well as the formula or I have done something wrong?

Is there a question here? Does it not work the way you expect? What results did you get? What results did you expect?

Your code takes eight readings, and returns a value. Let's say the signal is constant at 800 ADC counts. What value does your function return with that signal?

Now, consider what happens to your function when the unsigned subtraction is done, and current_value is less than avg?

i	avg	current_value
	0	
0	100	800
1	188	800
2	264	800
3	331	800
4	390	800
5	441	800
6	486	800
7	525	800

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

I am expecting result 80. But I am getting 0. Before introducing this function I got the value between 77-82 & with too much fluctuation. I am new in uC programming. I am trying to learn something.
What do you suggest then? What should I do?

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Suppose you do signed substractions/additions: the last three bits of your 'average' never will change (instead of a 10 bits converter, you'll transform it into a ... 7 bits converter). Is it what you hoped?

A solution,regardless of optimization, would be :
to send the values to a PC -exactly the values-(I bet Pc exist, and you can manage an UART), decide (with Excel, Octave/matlab, R, Fortran or ... C -the latter solution would help you to reuse code) which processing is the best; once you have found the best, porting to an AVR -if avrs are not enough, there are xmegas, arms- would demand some efforts, but you will be sure they do not lead to weird results - which would lead to wildly guess another filter, and getting wilder results-

Edited : 1rst paragraph has an error (the way shift works), but, if your inputs vary slowly and in aweak range (say, average +/-4) average will never be modified...

Last Edited: Wed. May 29, 2013 - 03:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Now my code is

uint16_t adc_filter(void)
{
	uint8_t i=0;
	int8_t current_value=0;
	int8_t avg=0;
	while(i<8)
	{
		current_value=adc_read();
		avg +=((current_value-avg)>>3);
		i++;
	}
	return(avg);
}	

But still getting nothing. Now I am becoming confused.

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Your return value does not have the same type as what you compute...

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

Try int16_t. Need 10 bits for the current value. In the real program, I'd do one calc per program loop rather than doing 8 calcs in the loop as you have shown. It is always adding "an eighth of the difference" between the new reading and the average. The plot is sort of exponential concave down as it goes up slower and slower, and exponential concave up as it starts down fast, then slows down.

Imagecraft compiler user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
uint16_t adc_filter(void)
{
   uint8_t i=0;
   int16_t current_value=0;
   int16_t avg=0;
   while(i<8)
   {
      current_value=adc_read();
      avg +=((current_value)>>3);
      i++;
   }
   return(avg);
} 

compute the average (and has a finite memory: if some outlier arrives, it will be forgotten the next time the function is called. If one wants a infinite memory, one should use average as input of the fuction *** and*** as output -and not reset it to zero inside the function.

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

Hi dbrion. What is your first name again? Anyway, my suggestion was a "first order lag" where you always add a piece of the error. I see your version doesn't calc the error, so its either something better or worse than my suggestion, but I cant tell which.

Imagecraft compiler user

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

Now my code is

int16_t adc_filter(void)
{
	uint8_t i=0;
	int16_t current_value=0;
	int16_t avg=0;
	while(i<8)
	{
		current_value=adc_read();
		avg +=((current_value-avg)>>3);
		i++;
	}
	return(avg);
}

But still getting nothing.

Quote:

I'd do one calc per program loop rather than doing 8 calcs in the loop as you have shown. It is always adding "an eighth of the difference" between the new reading and the average.

May be I am such a moron so that I can't understand clearly.
Can you explain on algo basis please.
What I have done wrong???

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

If you want to compute an ordinary average, just change:
avg +=((current_value-avg)>>3);
into
avg +=((current_value)>>3); (and the >>3 should be put outside the loop :it is faster and avoids rounding issues)

If you want to compute an autorecursive filter, and get its results after 8 samples,
the function prototype mightbe:
uint16_t adc_filter( uint16_t previous_average)
its internal value avg should not be reset to zero, but to pervious_average
int16_t avg=0; becomes
int16_t avg= previous_average;
and its invocation would look like:
avg=filter(avg);

Bob: my first name is Denis ; there is no superiority -nor inferiority- of finite response filters vs infinite response (I am more accustomed to the 1rst varity, as it is easier to remove/hide outliers and as they do not introduce weird behaviors for georeferenced data(2D, on limited domains -data are expensive to gather-).

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

Thanks for reply.
How can I get

Quote:

uint16_t previous_average

Can you explain a bit more plz.

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Here is how I do a rolling average (low pass filter). I think avg of 32 samps eats the dither in the bottom 5 lsbs. I think.

//----struct used for rolling average------
#define NUMSAMP 32
typedef struct{
  char ndx;
  long int  tot;
  int  avg;
  int  dat[NUMSAMP]; //avg over NUMSAMP readings
}Trollavg;

Trollavg rollavg[8];

//--------------------
void initrollavg(Trollavg *p){
//init rollavg struct
char i;

  p->ndx=0;
  p->tot=0;
  p->avg=0;
  for(i=0; i < NUMSAMP; i++){
    p->dat[i]=0;
  }  
}

//--------------------
int calcrollavg(Trollavg *p, int w){
//return rollavg of last NUMSAMP readings

  p->tot-=p->dat[p->ndx]; //subtract old value from tot
  p->tot += w;            //add in new value
  p->dat[p->ndx++]=w;     //remember new value, bump ndx
  if(p->ndx==NUMSAMP) p->ndx=0; //rewind
  p->avg = p->tot >> 5;   //2^5=32=NUMSAMP; avg is tot/NUMSAMP
  return p->avg;
}

//---------------------
unsigned int readadchan(char n){
//read ch n of internal a/d  10 bit unsigned

  ADMUX= 0x40 + n; //select vcc + channel n
  ADCSRA |= 0x40;  //init conversion
  while((ADCSRA & 0x40) != 0){}; //wait for conv complete
  return ADC;
}

//--------------------
void readadloop(void){
//read internal 10 bit ad 0-0x3ff  0..1023
char c,n;
unsigned int tmp;

  cprintf("a/d\n");
  cprintf("0    1    2    3    4    5    6    7 \n");
  c=0;
  while(c!='q'){
    if(kbhit()){
      c=getchar();
    }  
    for(n=0; n < 8; n++){
      tmp=readadchan(n);
      addat[n]=calcrollavg(&rollavg[n],tmp);
      cprintf("%4d ",addat[n]);
//      cprintf("%4d ",tmp);
    }
    cprintf("\r");
    delnms(1);  
  }
}

Imagecraft compiler user

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

Quote:
How can I get previous_average:

At the beginning of times, it is obviously unknown. There are two solutions:
a) let your compiler initialize to zero
b) take a sample (a call to adc_read) and betit is better than a)

Suppose you take the lazy solution a) , fake adc_read in order it always returns, say, 1000;
after 8 infinite reponse filter step -your function, with the changes I suggested might do it?-, you should get an output like 1000 * (1-(1. -.125)**8 )= 656
the next call, you should get 1000 * (1-(1. -.125)**16 ) :885.... and it would slowly -that makes me unhappy, but perhaps it is fast enough?- converge towards 1000;

Edited: this digital filter looks like an analogous filter(I suppose you sample at 14khz and time to calculate is small: it leads to RC =5E-4; with a 100nF capacity and a 5 k resistor, you could perhaps do the same thing without eating any cpu cycle -and test whether things get improved-

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

@bob & Denis let me digest & properly understand yours codes & ideas. Then I will let you know what I am getting.

Thanks again both of you.

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.

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

Why do you not take a giant step back wards.....

for a start:
does your adc_read function return anything else then 0?
so start with just returning the value of ADC_read.

if that gives the values you think you should be getting.....
return the sum of the values and see what happens there.
as the ADC value is 10 bit and you have 16 bit adding 8 times 0x3FF should still nicely fit in the 16 bit

btw make the return value unsigned integer ( uint16_t) . I assume you do not use a differential channel and thus the value will always be positive never negative

if again that works last step is to see what happens when you return the end value.

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

There are two questions which might been answered before coding -cutting and pasting code-:
a) is digital filtering necessay (as it amounts to simulating a RC input filter upstreams the ADC, recursive infinite response is redundant with analog filtering: if one fears it eats cpu cycles, analog filtering does not use **any** cycle and is , at least when prototyping, easier to implement)

b) is root mean square equivalent to linear means?? (and is root mean square -in the original post- necessary?)

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

Last few hours I was trying to digest @bob's code I can't totally, specially last function of his code. So I can't use it. Sorry @bob.

@Denis I also tried your methods several times but I failed & found difficulties on implementation of recursion. Sorry@Denis

I failed both the ways because my poor level of C languages.

Quote:
coding -cutting and pasting
Until or unless I don't understand the code I always try to avoid it.

For filtering issues I knew hardware filters always save some space & cycles from uC. Actually I want to learn something real stuff. But what I found until I good at C language its better to avoid stuff like it.

Thanks again to all to bear with me.

When you do ask questions, you may look stupid.
When you do NOT ask questions, you will STAY stupid.