[AVR] ADC oversampling function

[Simon]

I have written the following little function to oversampe an AVR ADC. Anyone see any obvious flaws ?

Code: [Select]

int16_t ADC_os_read (uint8_t bits) //return the current ADC value with a resolution increase of x "bits", the function will wait until the a conversion has ended.
{
uint16_t samples = 4^bits;
uint32_t adc_sum = 0;
uint16_t i;

for (i = samples; i > 0; --i)
{
bit_s(ADCSRA, ADSC); //start a conversion
while( ADCSRA & (1<<ADSC));   // Wait until ADC conversion is complete..
adc_sum = adc_sum + ADC; //
}

adc_sum >> bits;
return adc_sum;
}


[bktemp]

4^bits does not do what you expect it to do: ^ is the C operator for binary XOR, but you want a left shift, so 4<<bits is the correct expression.

[Simon]

I think I meant it to be 4 "to the power of", how do I express an exponent in C ?

[Avacee]

adc_sum is 32bits but you return a 16bit

no check if someone passes in a large value for bits - eg 255 - if you only want to allow 4 bit oversampling then prevent values > 5

^ is XOR.. use pow(x,y)

why declare samples when you just assign the value to i - can just do "for (i = pow(x,y)) .. save a few clock cycles allocating samples (unless the compiler is dumb and evaluates this every time in the for loop, it should do it once)

[senso]

Or just << pow will use a floating point routine and add more than 2K of code.

[Simon]

Ah yes, I changed the sum value to 32 bits as it could need that many bits in the calculations but of course the function "header" needs updating. I should really recast it as a 16 bit as I'm not expecting more than 16 bits from an oversampled 10 bit ADC

[Avacee]

Or just << pow will use a floating point routine and add more than 2K of code.

Yep - got me there  .. I blame doing too much C# recently, makes one lazy

<< ftw

[Simon]

Or just << pow will use a floating point routine and add more than 2K of code.

I don't quite understand

[Simon]

Code: [Select]

pow(4,bits)

has been accepted by the compiler

[Simon]

New version, I could not find a way of recasting from one type to another so brute force it by using a 16 bit and a 32 bit variable and transfer to the 16 bit variable before returning. Do I assume correctly that the downsize will be from the right so any extra on the left will "fall off" ?

Code: [Select]

int16_t ADC_os_read (uint8_t bits) //return the current ADC value with a resolution increase of x "bits", the function will wait until the a conversion has ended.
{
uint16_t samples = pow(4,bits);
uint32_t adc_sum32 = 0;
uint16_t i;
uint16_t adc_sum16 = 0;

for (i = samples; i > 0; --i)
{
bit_s(ADCSRA, ADSC); //start a conversion
while( ADCSRA & (1<<ADSC));   // Wait until ADC conversion is complete..
adc_sum32 = adc_sum32 + ADC; //
}


adc_sum32 >> bits;
adc_sum16 = adc_sum32;
return adc_sum16;
}


[senso]

http://www.atmel.com/webdoc/AVRLibcReferenceManual/group__avr__math_1ga41b41c307b8f96760e9c0c17180b241b.html

double pow(
   double __x,
   double __y)

In its core its a floating point function, it adds 2K of floating point libs to your code when a simple left shift will achieve the same result using just a couple instructions.

[Andy Watson]

adc_sum32 =>> bits;

[helius]

Or just << pow will use a floating point routine and add more than 2K of code.

arithmetic shift is a multiplication by a power of 2. please don't tell me you thought 4<<9 raises 4 to the power 9
However, \$ 4^n=2^{2n} \$, so you can correctly simplify the code pow(4,bits) to 1<<bits*2.

[Simon]

So we now have:

Code: [Select]

int16_t ADC_os_read (uint8_t bits) //return the current ADC value with a resolution increase of x "bits", the function will wait until the a conversion has ended.
{
uint16_t samples = 1<<2*bits;
uint32_t adc_sum32 = 0;
uint16_t i;
uint16_t adc_sum16 = 0;

for (i = samples; i > 0; --i)
{
bit_s(ADCSRA, ADSC); //start a conversion
while( ADCSRA & (1<<ADSC));   // Wait until ADC conversion is complete..
adc_sum32 = adc_sum32 + ADC; //
}


adc_sum32 >> bits;
adc_sum16 = adc_sum32;
return adc_sum16;
}


[Simon]

adc_sum32 =>> bits;

thanks, I missed that

It actually needs to be:

Code: [Select]

adc_sum32 = adc_sum32 >> bits;

else the compiler does not like it.

[nctnico]

I think you should re-read the appnote on oversampling. Don't forget you will need noise for oversampling to actually work (it might not). You may get an extra bit using 4x oversampling IF the ADC in the uC is decent but beyond that you will be better off using an (external) ADC with more bits.

[Brutte]

You are trying to do a FIR "kinda" filtering with decimation.

bn*y[n] = a1*x[1]+ a2*x[2]+... + an*x[n]

So bn = n and a1=a2=a3=...=an=1 and all previous samples before x[1] are ignored: a0=0

First problem is that this routine is not sampling on regular time base as you are triggering adc S/H by software quite randomly, if irqs are on. You get jitter and instead a=[0 0 0 1 1 1 1 1 1 1 1 ] you get a=[1 1 1 0 1 1 1 1 0 1 0].
Either trigger ADC synchronously or put assert(irqs disabled) in the code.

Second thing, with a fixed point FIR, you were supposed to divide the accumulated result by bn and not chopping off log2(bn) bits, methinks.

Chopping off adds random noise to the result.
For n=8, if x = [7 7 7 7 7 7 7 6] then y = 55.0/8.0=6.875 which after rounding should give fixed point 7. By chopping off, the result is a fixed point 6 and that does not look like an average to me.

Also on the last but one line you are doing "brave cast", different signedness, different sizes, without even testing if that is doable.

And there is a question of dithering but that is an analog part of the play so it does not applear in the code. You need some decent variance in the signal to increase ADC resolution.

[Andy Watson]

It actually needs to be:

Code: [Select]

adc_sum32 = adc_sum32 >> bits;

else the compiler does not like it.

That's because I'm an idiot and should have typed:

Code: [Select]

adc_sum32 >>=  bits;


[mikerj]

Second thing, with a fixed point FIR, you were supposed to divide the accumulated result by bn and not chopping off log2(bn) bits, methinks.

Chopping off adds random noise to the result.
For n=8, if x = [7 7 7 7 7 7 7 6] then y = 55.0/8.0=6.875 which after rounding should give fixed point 7. By chopping off, the result is a fixed point 6 and that does not look like an average to me.

Also on the last but one line you are doing "brave cast", different signedness, different sizes, without even testing if that is doable.

And there is a question of dithering but that is an analog part of the play so it does not applear in the code. You need some decent variance in the signal to increase ADC resolution.

n=8 is not a valid number of samples for this function.  The integer value passed in is the extra number of bits of resolution required, and the number of samples is then calculated as 4 raised to the power of n i.e. 1 extra bit requires 4 samples, two extra bits requires 16 samples etc.

Also you are performing a simple averaging function above which is not what is required.  Dividing the accumulated sum by the number of samples (as an integer operation) throws away the extra resolution which you are trying to get, though it will reduce noise.  If you accumulate four samples, you divide the result by 2, and you get an extra bit of resolution (maybe).  For 16 samples you'd divide the result by 4, netting two additional bits in the result.

[Simon]

You are trying to do a FIR "kinda" filtering with decimation.

bn*y[n] = a1*x[1]+ a2*x[2]+... + an*x[n]

So bn = n and a1=a2=a3=...=an=1 and all previous samples before x[1] are ignored: a0=0

First problem is that this routine is not sampling on regular time base as you are triggering adc S/H by software quite randomly, if irqs are on. You get jitter and instead a=[0 0 0 1 1 1 1 1 1 1 1 ] you get a=[1 1 1 0 1 1 1 1 0 1 0].
Either trigger ADC synchronously or put assert(irqs disabled) in the code.

Second thing, with a fixed point FIR, you were supposed to divide the accumulated result by bn and not chopping off log2(bn) bits, methinks.

Chopping off adds random noise to the result.
For n=8, if x = [7 7 7 7 7 7 7 6] then y = 55.0/8.0=6.875 which after rounding should give fixed point 7. By chopping off, the result is a fixed point 6 and that does not look like an average to me.

Also on the last but one line you are doing "brave cast", different signedness, different sizes, without even testing if that is doable.

And there is a question of dithering but that is an analog part of the play so it does not applear in the code. You need some decent variance in the signal to increase ADC resolution.

From what AVR app note 121 says table 3-1 on page 8, the result is right shifted by as many bits as you are adding to the existing resolution of the ADC.

My idea for this peice of code is as much about filtering as it is about gaining a few extra bits. I have seen gittery results on an ADC before and thought it was time to at least filter it to get a more stable reading. If I do it as an oversample I'll either get some trailing "0's" or maybe a bit more resolution. Ultimatey 10 bits is enough for what I want.

No i have not studied the analogue side of it and no there is no timing in the readings (how would I sync the readings to atificial noise ?). I am just relying on natural noise but with a 1m cable I'm sure I'll pick some crap up, it may help the oversampling or it may just be noise that I filter out.

Yes I know I'm being a bit brave with my type casting. I would never expect this to work anywhere near 16 bits in terms of increasing resolution and providing I don't try to oversample to more than 16 bits my result to return should never be more than 16 bits.

[Brutte]

If you accumulate four samples, you divide the result by 2, and you get an extra bit of resolution (maybe).

I am not referring to the number of samples but to the chopping as such. It does not matter how many samples the algorithm takes.
So for n=4 there is: b4=2, a1=a2=a3=a4=1
If you feed [7 7 7 6] you get y[n]=27.0/2.0=13.5 and fixed point result after chopping off least significant bit becomes 13. So actually we get:

Code: [Select]

float noise, y[4];
....
return (int)(y[3]+noise); //with no rounding on castwhere noise is always positively biassed as you chop either 0.0 or 0.5 each time and if those two appear with equal probability, the average chop is +0.25  methinks.
 
Or I do not get the deeper sense of that chopping.

[hamster_nz]

I've been thinking a little on this, and come the the conclusion that unless you tell us what signal you are filtering and what you want to achieve it is very difficult to offer any advice outside of the basic implementation code.

Maybe the input signal has only very low frequency components (e.g. monitoring a battery voltage), in that case taking four samples very quickly and averaging them is a valid way to remove random conversion noise between samples. If the random noise has the correct statistical properties it might even be able to add a bit or two of effective resolution.

However, if the ADC's input signal contains any high frequency signals (relative to output the sample rate), then averaging three out of four samples is bad. You don't see the difference between (50, 50, 50, 50) or (100, 0, 100, 0), (100, 100, 0, 0), (67,67,66,0) or (200, 0, 0, 0) as they all average out to 50. Because of this it really depends on signal conditioning (analogue filtering) before it goes into the ADC.

One easy test would be to print out a dozen samples - if the ADC reading is stable in the LSB then there there is no more information that can be gained through averaging it.

[BrianHG]

Yes, for the over sampling trick to work, your 4 samples needs to be significantly faster than the final desired total result sample rate and you source signal needs to be low in bandwidth as well.  If you don't care about the HF response of your sampler, like using the result to measure a DC voltage of load cell or temperature gauge, you code should work fine unless the CPU's ADC has 0 noise on the LSB when it samples.  You can always cheat this be using a noisy opamp to feed your ADC input.

[Simon]

Oh it's just a temperature signal from a thermistor so no massive changes in a hurry. My last device that used a temperature to generate a pwm had the pwm jumping all over the place (but would have been filtered down stream) so clearly there is an amount of noise to be removed and potential resolution to be harvested, at worse I'll get a cleaner reading

[Andreas]

Hello,

when doing the wiring right (ground star in the analog section) there is no noise that you can average at least with a 10 Bit ADC.

When your PWM-signal influences the measurement you are probably sampling at the wrong time.
I would sample at the end of the passive PWM phase.

The only case where averaging might be useful is when you have a sensor sensitive to line frequency hum.
But in this case you would have to sample at e.g. 16-20 equidistant time points within the PLC (power line cycle).

In the attached picture I have averaged around 350 measurement values over 1 minute for 1 measurement point
from a NTC with a 10 Bit ADC (5V).
(X-Axis: number of samples, Y-axis voltage in mV).
You can clearly see the 5 mV steps of the ADC.
Only when the temperature is exactly between 2 steps there is a small range where intermediate averages can be seen.

With best regards

Andreas