-
Notifications
You must be signed in to change notification settings - Fork 7
fix iir clamping #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
fix iir clamping #29
Conversation
from artiq.coredevice import phaser
from artiq.experiment import *
class Test(EnvExperiment):
def build(self):
self.setattr_device("core")
self.setattr_device("core_dma")
self.phaser = self.get_device("phaser0")
@kernel
def run(self):
self.core.reset()
self.phaser.init()
ch = self.phaser.channel[0]
ch.set_att(0.0)
self.phaser.write8(phaser.PHASER_ADDR_ADC_CFG, 0b1) # +- 1V inputs
ch.oscillator[0].set_frequency(100 * MHz)
ch.oscillator[0].set_amplitude_phase(1.0, 0.0)
profile = 0
ch.set_iir(profile=profile, kp=0.0, ki=0., g=0., y_offset=-0.2)
ch.set_servo(profile=profile, enable=1, hold=0) leads to full output with current master, no output (as expected) with this PR. |
Standard Migen assignment does sign extensin. Please find a way to use that without manual sign extension. This might also be related to one of the migen issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please find a way to do this with less code and especially without manual sign extension.
Length-matching the inputs works. Maybe related to m-labs/migen#102? This made me realize the following: This would have the downside that it further reduces effective resolution of the setpoint when used as a PI controller in a realistic scenario. There you already often get effective offsets of eg. 25 when stabilizing to half positive input range. |
4b83097
to
1faf6f3
Compare
In order to not complicate things I reverted to simply hardcoding the correct input sizes. This fixes the problem in a non-invasive way and without changes necessary elsewhere. |
The clamping mechanism in the IIR was broken. It would clamp the positive side to positive fullscale but actually flip a negative output to positive fullsacale instead of clamping at 0 as intended originally.
The problem was that apparently the negative words in the DSP registers don't get their sign bit extended to the full register sizes. Therefore the MSB of the output never changed. I added manual replication of the sign bit to fill the registers. Now both sides of clamping work as intended.
DSP usage and timing didn't change.