ACC LPF (again)

[HappySundays]

Hi guys,
I haven't posted in like forever... my OpenAero2 and the new OpenAeroVTOL have now 99% unique code - unrelated to MultiWii - with the exception of the IMU code, which is based on older MW code.
However I have been looking at improving this code and sorted through changes up to V2.3. I then came across the rather puzzling change to the ACC LPF code. So as not to waste your time I duly researched the ACC LPF mod and found this thread --> http://www.multiwii.com/forum/viewtopic.php?f=7&t=1573.

Now the claim is that this code:

Code: Select all

accTemp[axis] = (accTemp[axis] - (accTemp[axis] >> ACC_LPF_FACTOR)) + accADC[axis];
accSmooth[axis] = accTemp[axis] >> ACC_LPF_FACTOR;

...is essentially the same as this code:

Code: Select all

accSmooth[axis] = (accSmooth[axis] * (ACC_LPF_FACTOR - 1) + accADC[axis]) / ACC_LPF_FACTOR;

..or its better-rounded cousin:

Code: Select all

accSmooth[axis] = (accSmooth[axis] * (ACC_LPF_FACTOR - 1) + accADC[axis] + (ACC_LPF_FACTOR/2)) / ACC_LPF_FACTOR;

If you read the above thread you can see the conversation drift obliquely away from the actual issue and compare >> to /...
The code in the current MultiWii implementation might kinda sorta work when not examined closely but it is in NO WAY similar to the original code.

The old code MULTIPLIED the ongoing sum by a large amount, then added the new accADC, and then divided the total by an amount fractionally larger than the multiply, leaving the result exactly the same size as before the operation.

The new code subtracts a small fraction of the current sum, adds the new accADC and then strangely, divides the ENTIRE result by again the same amount. The result in accSmooth[axis] will be a small fraction of the original. It is true that an amount of the original signal is retained, but the point of a LPF is to retain a very specific LARGE amount of the original. With the old code the percentage of the old signal vs the new one is guaranteed.
I do understand that the move to rotates was probably driven by a need to increase code execution speed, but I really don't think the code works the same way.

I'm not saying that you don't get a kind of filtering effect from this code if you process the decimated result properly but either I've missed something obvious or there is something wrong with this code.
So can someone look at the above with fresh eyes and tell me I'm wrong? Frankly I want to be wrong but no matter how many times I look at it I can't see it any other way...

[Crashpilot1000]

Hi!

HappySundays I wouldn't think too much about it. Keep your LPF filtering as it is and you will be fine, but with more arduino cycles. On the other hand in your 32 Bit branch I would suggest to break it down to a lpf with Hz cutoff to be cycletime independent.
Like
accSmooth[axis] += ACCFac * (accADC[axis] - accSmooth[axis]); // Do this in loop for all axes

where ACCFac is (precalculated before axes loop):
ACCFac = ACCDeltaTimeINS / (ACC_RC + ACCDeltaTimeINS); // (ACCDeltaTimeINS is in seconds)

and precalculated (at boot) ACC_RC is
ACC_RC = 0.5f / (M_PI * cfg.acc_lpfhz);

The default mwii 1.9/2.0 acc_lpfhz resembles "0.536 Hz" at assumed fixed cycletime of 3ms.
Hope to be helpful (and not too much aside from the topic).
Cheers
Rob

Note: "accADC" are float variables in my code (preprocessing done in "float").

[HappySundays]

Thanks Crashpilot1000,
I do love a good pre-computed number
Also a great idea to make it independent of a fixed cycletime.
Is ACCDeltaTimeINS the system time difference between successive loops?

This is an interesting approach:

Code: Select all

accSmooth[axis] += ACCFac * (accADC[axis] - accSmooth[axis]);

That seems to make sense. The larger the ACCFac, the faster the new data alters the sum.
I'll have a play with the code.

Thanks! and yes - on topic

[Crashpilot1000]

Well the idea is great because it's from wiki "lowpassfilter" and shows how a lpf is normally implemented (look at the pseudocode there). The mwii approach is a simplification and reminds somehow of alexmos (Edit: I wondered at that time why his work wasn't embraced - I think *someone* should know better now - at least we all know it NOW what for a developer mwii has missed. The same goes for alex khoroshko that hasn't received a very warm welcome here as well... plainly stupid move from the mwii side IMHO). It works so it's ok.
The ACCDeltaTimeINS is gathered just after acc readout (first step in getEstimatedAttitude). So it could look like something like this.

currentT = micros();
tmp = (float)(currentT - previousT);
scale = tmp * GyroScale; // left in in this example
ACCDeltaTimeINS = tmp * 0.000001f;
previousT = currentT;

Cheers
Rob