AP_InertialSensor: fixed Q calculation for notch filters #28765
Conversation
when doing the init() we must use the reference frequency, not the current frequency. Using the current frequency leaves us with an incorrect value for Q. If the current frequency is below the reference frequency (which shouldn't happen in 4.5, but has been seen in 4.3) then the Q can be much too low and massive phase lag can happen. The vulnerability in 4.5.x is that the current frequency could be well above the reference frequency. For example, the user may be doing a motor test at 30s after boot, which is when we stop the sensors_converging() test, and thus is the last time we call init(). In that case we can end up with a Q which is much larger than the one that should come from INS_HNTCH_FREQ and INS_HNTCH_BW, and thus end up with a filter that produces a lot less attenuation than is desired, potentially leading to instability due to high noise. There are other scenarios that can cause this - for example a motor test of a fwd motor at 30s after boot, or a spurious FFT peak due to someone knocking the aircraft, or the vibration of a IC engine.
tridge
force-pushed
the
pr-notch-init-wrong-Q
branch
from
edc6d1a
to
fa828a7
Compare
|
Still an issue - needs to pass CI |
|
So the issue with CI failing is that the notch still needs to attenuate during the warmup time and the attenuation frequency can be (very radically) different to the center frequency used for Q calculation. My concern is that if people are regularly taking off within 30s of boot then they will be flying and will have tuned with a narrower Q (that's what CI shows) and the fix will potentially lead to lower Q and hence additional phase lag. I'm not sure in reality how much of a concern this is, but I know that we have had related reports of notches not working at startup (there is a thread on discuss) which we fixed so the user base is not zero. |
|
If we put this in 4.5 it needs a very visible release note. We also should check that the attenuation problem does not exist in 4.6 |
|
Hmmn, although my fix demonstrably fixes the issue, its not clear why the update after is not working. Maybe something to do with it being a notch per-motor. |
|
Urg, CI still failing |
when doing the notch init() we must use the reference frequency, not the current frequency. Using the current frequency leaves us with an incorrect value for Q. If the current frequency is below the reference frequency (which shouldn't happen in 4.5, but has been seen in 4.3) then the Q can be much too low and massive phase lag can happen. I am also not 100% certain a low freq in init() can't happen in 4.5.x, the route it came in for the 4.3.x crash logs is quite complex.
The way the init() works is we call it continuously up until 30s after boot, then we call it only if the user changes the notch parameters. This means the precise state of the aircraft at the 30s after boot mark is critical. Depending on what is happening to the aircraft at 30s after boot the Q value could be a very long way off.
The confirmed vulnerability in 4.5.x is that the current frequency could be well above the reference frequency. For example, the user may be doing a motor test at 30s after boot, which is when we stop the sensors_converging() test, and thus is the last time we call init(). In that case we can end up with a Q which is much larger than the one that should come from INS_HNTCH_FREQ and INS_HNTCH_BW, and thus end up with a filter that produces a lot less attenuation than is desired, potentially leading to instability due to high noise.
There are other scenarios that can cause this - for example a motor test of a fwd motor at 30s after boot, or a spurious FFT peak due to someone knocking the aircraft, or the vibration of a IC engine.
I've checked, and it is already fixed in 4.6.x