On Wed, 2007-07-11 at 02:25 +0200, Jiri Benc wrote:
> > + /* Quantize the averages (divide by 16) */
> > + sta->last_rssi = sta->accum_rssi >> 4;
> > + sta->last_signal = sta->accum_signal >> 4;
> > + sta->last_noise = sta->accum_noise >> 4;
>
> First, do not obfuscate the code, please. If you want to divide by 16,
> divide by 16. The compiler is perfectly capable of optimizing it.
No, you're wrong, the compiler can only optimise that if it knows for
sure that the values can't ever be negative. And quoting the patch
further below:
> + int accum_rssi; /* hi-precision running average (rssi * 16) */
> + int accum_signal; /* hi-precision average (signal-quality * 16) */
> + int accum_noise; /* hi-precision running average (noise * 16) */
The compiler does some tricks to avoid the integer division IIRC but it
can't do a simple shift. But in principle you're right, the code should
say "/ 16" because either the variables need to be made unsigned (only
positive values are put into them) or using a shift is wrong (if
negative values can happpen)
Oh btw, is this the difference between "moving average" and "running
average"?
With the algorithm as I see it, each update is done by
update(new_value):
avg = 15/16*avg + new_value
which means that all old samples are still influencing the current
output.
Doing two updates (update(a), update(b)) yields:
avg = 15/16 * (15/16*orig + a) + b = 225/256 * orig + 15/16 * a + b
And after 16 updates the original values still has an influence of about
3.5% on the output value:
update(a_0),...,update(a_15):
avg = (15/16)^16*orig + \sum_{i=0}^{15} (15/16)^i * a_i
= 0.3560*orig + 0.3798*a_0 + ... (sum of coefficients is ~10.30)
Is this really what we want? Wouldn't it be better to have a ringbuffer
of the last 16 values and really do an average over those? [which is
what I understand as "moving average"]
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
