search

slaclab / pysmurf

Other
2 stars 9 forks source link

Setting feedback limit using `set_feedback_limit_khz` off by 2? #787

Open swh76 opened 9 months ago

swh76 commented 9 months ago

Describe the bug

I think we may have an off-by-2 in the set_feedback_limit_khz function ;https://github.com/slaclab/pysmurf/blob/19d236caa1867e724762c5d15c32de2889614be6/python/pysmurf/client/util/smurf_util.py#L1983C1-L1983C1

Folks at MIT report despite setting the limit to 1.2 MHz (via set_feedback_limit_khz(1200)) seeing tracking limited to +/- 600 kHz. Naively, I'd expect the maximum possible limit to be 1.2 MHz, the useable subband half width (our polyphase filter bank subband useable width is 2.4 MHz).

Poking at it I noticed that despite the fact that the feedbackLimit register is a 16-bit register, setting it via set_feedback_limit_khz(1200) and set_feedback_limit_khz(600) result in settings of 0x4000 and 0x8000 respectively, or 2^14 and 2^15, respectively. I'd have expected set_feedback_limit_khz(1200) to result in 0xffff, the full 2^16 bit range of the feedbackLimit register. Trying to set it higher leads to no change because of this limit;

https://github.com/slaclab/pysmurf/blob/19d236caa1867e724762c5d15c32de2889614be6/python/pysmurf/client/util/smurf_util.py#L2000

I will need to empirically verify the scaling.

If there is an off-by-2 error it's probably on this line;

https://github.com/slaclab/pysmurf/blob/19d236caa1867e724762c5d15c32de2889614be6/python/pysmurf/client/util/smurf_util.py#L2003

where I'm betting the units of the feedbackLimit register should scale inversely to subband_bandwidth/2., not subband_bandwidth, e.g. it should be

desired_feedback_limit_dec = np.floor(desired_feedback_limit_mhz/
            ((subband_bandwidth/2.)/2**16.))

(although that would lead to 1200 kHz => 2^16 bits which would wrap around to zero.

swh76 commented 9 months ago

Currently, you can't set the 16-bit feedbackLimit register higher than half its full range (2^15 bits = 0x8000). To access higher bits for now, set the register directly using set_feedback_limit which expects a 16-bit integer (in [0,2^16] ; if you set it to 2^16 it will wrap back to 0). To open up the limit to the max, this should work

S.set_feedback_limit(0,2**16-1)