slaclab / pysmurf

Other
2 stars 9 forks source link

Remove errant factor of 4 in `smurf_control.setup()` call to `set_lms_delay()` #790

Closed swh76 closed 3 months ago

swh76 commented 3 months ago

Addresses issue https://github.com/slaclab/pysmurf/issues/789, which fixes SO issue https://github.com/simonsobs/sodetlib/discussions/423. This is an edge case that only effects users who don't provide lmsDelay in their pysmurf cfg files and don't run the smurf_util.estimate_phase_delay routine, which triggers the low level rogue setBandDelay routine (https://github.com/slaclab/cryo-det/blob/797fa80aa0e43bc4c0939921702a053f3f69ed2b/firmware/python/CryoDet/DspCoreLib/CryoDetCmbHcd/_CryoFreqBand.py#L661) which properly sets the lmsDelay register.

The lmsDelay register matches the system latency for the tracking feedback. The readout has both actuator and sensor delay, so this delay is needed to compensate the feedback. Actuator delay is the delay from DSP -> synthesis filter bank -> JESD -> DAC -> RF tracked freq out. Sensor delay is the delay from RF in resonator -> ADC input -> JESD -> filter bank -> demod (edited). The delay is needed because we are playing out a FM waveform on the RF DACs and it takes microseconds to get the results. In production SMuRF firmware, lmsDelay should be set equal to refPhaseDelay, and both are integers that count 2.4 MHz ticks. If none provided in the pysmurf cfg, enforce that constraint. If provided in cfg, override with provided value.

The bug is the factor of 4 here in https://github.com/slaclab/pysmurf/blob/e8db8010a6698beedae4e55928ab858f6c7cd0d6/python/pysmurf/client/base/smurf_control.py#L551 ; the history is that the SMuRF firmware used to process four tones in every polyphase filter bank (PFB) subband, but the production firmware we use now processes only one tone in every PFB subband. The 4* was only relevant for older versions of the firmware where there was more delay due to each subband needing to do four times more processing.

lmsDelay was also increased from a 5-bit to 6-bit register here - https://github.com/slaclab/cryo-det/commit/2aaedcbfe7ac08e39cf2d5db7a585b27e9627d17. Adjusted pysmurf cfg schema entry to reflect this.

No interface changes in this PR.