I thought I'd share a piece of code I recently wrote when I had problems getting good readings in an noisy environment.
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.
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.
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.
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:
typedef unsigned int index_t;
Then in your loops you can have a dedicated type for indices!
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.
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.
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.
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 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.
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
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 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.
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);
}
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.
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;
}
hmmm, the original used signed types. @arduinoCoder, you added the unsigned types in the update!
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.
