From edc6d1a0c2feac6a57090466852e33e7f45b84ed Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Thu, 28 Nov 2024 16:43:56 +1100 Subject: [PATCH] AP_InertialSensor: fixed Q calculation for notch filters 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. --- libraries/AP_InertialSensor/AP_InertialSensor.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/libraries/AP_InertialSensor/AP_InertialSensor.cpp b/libraries/AP_InertialSensor/AP_InertialSensor.cpp index b0bc11aaca935c..8e87371548dc44 100644 --- a/libraries/AP_InertialSensor/AP_InertialSensor.cpp +++ b/libraries/AP_InertialSensor/AP_InertialSensor.cpp @@ -1059,8 +1059,10 @@ AP_InertialSensor::init(uint16_t loop_rate) notch.filter[i].allocate_filters(notch.num_dynamic_notches, notch.params.harmonics(), double_notch ? 2 : triple_notch ? 3 : 1); // initialise default settings, these will be subsequently changed in AP_InertialSensor_Backend::update_gyro() - notch.filter[i].init(_gyro_raw_sample_rates[i], notch.calculated_notch_freq_hz[0], - notch.params.bandwidth_hz(), notch.params.attenuation_dB()); + notch.filter[i].init(_gyro_raw_sample_rates[i], + notch.params.center_freq_hz(), + notch.params.bandwidth_hz(), + notch.params.attenuation_dB()); } } } @@ -1789,13 +1791,14 @@ void AP_InertialSensor::HarmonicNotch::update_params(uint8_t instance, bool conv (params.tracking_mode() == HarmonicNotchDynamicMode::Fixed && !is_equal(last_center_freq_hz[instance], center_freq)) || converging) { filter[instance].init(gyro_rate, - center_freq, + params.center_freq_hz(), params.bandwidth_hz(), params.attenuation_dB()); last_center_freq_hz[instance] = center_freq; last_bandwidth_hz[instance] = params.bandwidth_hz(); last_attenuation_dB[instance] = params.attenuation_dB(); - } else if (params.tracking_mode() != HarmonicNotchDynamicMode::Fixed) { + } + if (params.tracking_mode() != HarmonicNotchDynamicMode::Fixed) { if (num_calculated_notch_frequencies > 1) { filter[instance].update(num_calculated_notch_frequencies, calculated_notch_freq_hz); } else {