search

quartiq / stabilizer

Firmware and software for the Sinara Stabilizer module with high speed, low latency ADC/DAC data processing and powerful DSP algorithms in between
http://quartiq.de/stabilizer/
Apache License 2.0
111 stars 25 forks source link

IIR filter coefficients set incorrectly #854

Open juhaszp95 opened 8 months ago

juhaszp95 commented 8 months ago

Hello,

Thanks for keeping up the development of the stabilizer. I've just tried to set a low-pass filter using your script at https://github.com/quartiq/stabilizer/blob/main/py/stabilizer/iir_coefficients.py, but I think the IIR BA coefficients are returned with the wrong sign. Specifically, lines 59-61 say

#     Returns:
#       [b0, b1, b2, -a1, -a2] IIR coefficients to be programmed into a
#       Stabilizer IIR filter configuration.

However, in line 144, it's actually +a1, not -a1 being returned. Therefore, this implements the wrong filter.

It seems to me that this is a problem for all filters (+a_i returned instead of -a_i), but it seems OK for the PID (I haven't tested yet).

I'm not a 100% sure why -a_i needs to be sent to the stabilizer, but when I change a1 to -a1 in line 144 I do get the expected, correct behaviour - so it's probably just a matter of updating the Python script.

jordens commented 8 months ago

iir_coefficients is out of date w.r.t. recent changes in idsp.

juhaszp95 commented 8 months ago

Is there any chance you could release a temporary fix for this coefficient sign issue while presumably a more general rewrite lands?

juhaszp95 commented 7 months ago

Further to the above, I think the PID coefficients are also returned with the wrong sign, and the b0 coefficient of the high-pass filter is wrong (it has an extra factor of f0_bar in the numerator).

Furthermore, I'd like to get the PID setpoint (the x offset) working, which it seem to me is not implemented in the this python file - I wonder if there is any description somewhere regarding how that could be done?

jordens commented 7 months ago

Some background on PID/IIRs is here: https://hackmd.io/IACbwcOTSt6Adj3_F9bKuw A proven implementation is here: https://github.com/quartiq/idsp/blob/main/src/iir/pid.rs while the underlying Biquad impl that dual-iir uses is this: https://github.com/quartiq/idsp/blob/main/src/iir/biquad.rs It's on my list to get iir_coefficients back up to speed (i.e. to the state that idsp is in).

juhaszp95 commented 7 months ago

Thanks very much! I've already read the PID primer which was very useful. I'm less familiar with Rust, hence the need for the python interface.