Go Down

Topic: Averaging analogRead function (Read 2 times) previous topic - next topic

arduinoCoder

I thought I'd share a piece of code I recently wrote when I had problems getting good readings in an noisy environment.
Code: [Select]
int analogReadAverage(int channel){
  int sampleBuf[16] = {analogRead(channel)};
  char k;
  for (byte i = 1; i<16; i++){
    word reading = analogRead(channel);
    char k;
    for (k = i-1; (k >= 0) && (reading < sampleBuf[k]); k--){
      sampleBuf[k+1] = sampleBuf[k];
    }
    sampleBuf[k+1] = reading;
  }
  sampleBuf[0] = 0;
  for (k=4;k<12;k++){
    sampleBuf[0]+=sampleBuf[k];
  }
  return (sampleBuf[0] >> 3);

This function takes 16 readings, throws the highest 4 and lowest 4 out and returns the average of the remaining 8.  I got the idea on another forum (Cygnal forum if I remember correctly) and implemented it for the Arduinos.

Enjoy!

robtillaart

thanks for sharing,

check running median - http://playground.arduino.cc/Main/RunningMedian -
similar idea, but it does the math after every new reading
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

PaulS

Code: [Select]
  int sampleBuf[16] = {analogRead(channel)};
An array of 16 elements, with one initializer. The other 15 contain junk. Hmmm.

Code: [Select]
    word reading = analogRead(channel);
The analogRead() function does not return a word. It returns an int.

Code: [Select]
    char k;
    for (k = i-1; (k >= 0) && (reading < sampleBuf[k]); k--){

Using a char as an array index makes no sense. A byte is the same size, and does make sense.

Why are there two local variables named k?

pYro_65


Code: [Select]
 int sampleBuf[16] = {analogRead(channel)};
An array of 16 elements, with one initializer. The other 15 contain junk. Hmmm.


It is initialized like that due to the fact its a waste of space to initialize variables that are not needed.
The algorithm collects 15 more reads in the first loop. Spreading out the reads over the duration of the loop may also give a noisy spike time to dissipate.

Quote
Code: [Select]
   word reading = analogRead(channel);
The analogRead() function does not return a word. It returns an int.


And a word is typically a short type, which on the Arduino is the same size as an int.

Quote
Code: [Select]
   char k;
   for (k = i-1; (k >= 0) && (reading < sampleBuf[k]); k--){

Using a char as an array index makes no sense. A byte is the same size, and does make sense.


A char is the clearer option, it is a standard C++ type, whereas byte is not. If you wanted to make a clearer type to yourself, you could do something along the lines of:
Code: [Select]
typedef unsigned int index_t;
Then in your loops you can have a dedicated type for indices!

Quote
Why are there two local variables named k?

They are only local to their defining scope, and they do not reside in the same scope. Each 'k' is used as a loop counter in different loops.

Different names could be used for no particular advantage, apart from ease of understanding.

PaulS

Quote
Different names could be used for no particular advantage, apart from ease of understanding.

The point was that only one of them is actually used.

A word is a bastardization invented by Microsoft. It is one of many obfuscations that they use to make coding far more difficult than it needs to be. It was a colossal mistake for the Arduino team to have incorporated it in. It's use should not be encouraged in any fashion. There are other types that are more appropriate, and whose sizes are standard.

Quote
A char is the clearer option

In my humble opinion, you are wrong. Can you remember, without looking, on which platforms char is signed and on which platforms it is unsigned? I can't. I expect char variables to hold values that represent characters. The values in the loop index are not characters.

pYro_65

#5
Feb 16, 2014, 09:35 pm Last Edit: Feb 16, 2014, 09:46 pm by pYro_65 Reason: 1

Quote
Different names could be used for no particular advantage, apart from ease of understanding.

The point was that only one of them is actually used.

Quote
A char is the clearer option

In my humble opinion, you are wrong. Can you remember, without looking, on which platforms char is signed and on which platforms it is unsigned? I can't. I expect char variables to hold values that represent characters. The values in the loop index are not characters.


Well, Paul, you are wrong on both accounts. Both k variables are used, and char is default signed unless a compiler flag is set in all modern standard C++ compilers.  Why whinge about word types, then back up byte. I bet you can assure yourself that typing unsigned/signed char will be far more acceptable than expecting byte to be there to hold your hand.

Thinking that chars are only for text is a misconception. Character literals are actually integer types, 'char' being one of them.

robtillaart

Quote
Thinking that chars are only for text is a misconception. Character literals are actually integer types, 'char' being one of them.

agree, using a char for characters (e.g. a..Z) is just a (very often used) representation of the (8 bit) integer. In theory one could make a mapping between floats and characters, but it would not be efficient to start with ;)
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

arduinoCoder

Hi All

Thanks for all the remarks (didn't realize it sparked such a debate).  Though I must admit, I did mix up int, word and byte,char without realizing it.  I did not have a lot of time to do write this function.  But if I have it correctly it does not matter as none of the values will be negative or bigger than what the signed integers can handle (the code was tested and is working).

As for the array initialization, I was under the impression that the compiler automatically will set the rest of the array to zero. i.e. to initialize an entire array to zero you only have to type
Code: [Select]
int arr[16] ={}; to have everything zeroed.  If this was not the case the code would not have worked correctly, but fortunately it does work.

I agree that it would be better to use the same type (as in int or word) but in terms of the code working or not, it does not matter which one.  The ADC on the mega does not do negative numbers and it is right aligned, so the value will not exceed 1023.  If it was left aligned it would have been a different story.  Then  the maximum can be 65472, which will cause problems (actually using an int in stead of word).

I did not even spot the two declarations of k...  Thanks for pointing it out.  Again, because the loop counter variable never exceeds the max for a char (127), I does not matter which one is used.  I am afraid to say it, but I prefer bytes to chars for array loop counters because I often use buffers larger than 127 bytes.

I have cleaned it a bit and here the new code.  I have only edited it for the forum and not actually tested it, but it should still work.  Please tell me if you spot a new problem.

Code: [Select]
int analogReadAverage(int channel){
  int returnVal = 0;
  int sampleBuf[16] = {analogRead(channel)};
  for (byte i = 1; i<16; i++){
    int reading = analogRead(channel);
    for (byte k = i-1; (k >= 0) && (reading < sampleBuf[k]); k--) sampleBuf[k+1] = sampleBuf[k];
    sampleBuf[k+1] = reading;
  }
  for (byte k=4;k<12;k++) returnVal+=sampleBuf[k];
  return (returnVal >> 3);
}



robtillaart

sampleBuf[k+1] = reading;  // k will not be defined


Code: [Select]

int analogReadAverage(int channel)
{
  int sampleBuf[16];
  sampleBuf[0] = analogRead(channel);

  for (byte i = 1; i  < 16; i++)
  {
    int reading = analogRead(channel);

   byte k = i-1;
   while ( k >= 0  &&  reading < sampleBuf[k] )
   {
      sampleBuf[k+1] = sampleBuf[k];
      k--;
    }
    sampleBuf[k+1] = reading;
  }

  int returnval =0;
  for (byte k = 4;k < 12; k++)
  {
    returnVal += sampleBuf[k];
  }
  return returnVal / 8;
}
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

arduinoCoder

So its 6 months down the line and I had to use this function and I found it still is flawed...  The definition of k has to be a signed type (in this case char) as the test for exiting the while is >= 0.  Unsinged types will never be less than 0.

NOTE: I have changed it to take 8 samples in stead of 16 to save on RAM.

Code: [Select]
int analogReadAverage(int channel)
{
  int sampleBuf[8];
  sampleBuf[0] = analogRead(channel);

  for (byte i = 1; i  < 8; i++)
  {
    int reading = analogRead(channel);

   char k = i-1;
   while ( k >= 0  &&  (reading < sampleBuf[k]) )
   {
      sampleBuf[k+1] = sampleBuf[k];
      k--;
    }
    sampleBuf[k+1] = reading;
  }

  int returnval = 0;
  for (byte k = 2;k < 6; k++)
  {
    returnval += sampleBuf[k];
  }
  return returnval / 4;
}

robtillaart

well spotted!  that is indeed a flaw  - I should have tested  :smiley-red:   :)

Quote
NOTE: I have changed it to take 8 samples in stead of 16 to save on RAM.

as these are local variables they only exist for the duration of the function call.
But 8 samples will do for most apps and it is twice as fast .
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

pYro_65

hmmm, the original used signed types.
@arduinoCoder, you added the unsigned types in the update!

Quote
I did not even spot the two declarations of k...  Thanks for pointing it out.  Again, because the loop counter variable never exceeds the max for a char (127), I does not matter which one is used.  I am afraid to say it, but I prefer bytes to chars for array loop counters because I often use buffers larger than 127 bytes.



Go Up