Skip to content
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

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

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

swh76
Copy link
Collaborator

@swh76 swh76 commented Mar 28, 2024

Addresses issue #789, which fixes SO issue simonsobs/sodetlib#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

band, int(4*self._ref_phase_delay[band]),
; 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.

…n pysmurf cfg file. Adjusted cfg file schema to reflect expansion of lmsDelay reg from 5 to 6 bits.
@swh76 swh76 requested a review from tristpinsm March 28, 2024 06:45
@swh76 swh76 added the bug Something isn't working label Mar 28, 2024
@github-actions github-actions bot added the client Changes to the client code label Mar 28, 2024
@swh76 swh76 self-assigned this Mar 28, 2024
@swh76 swh76 merged commit 1959443 into main Mar 28, 2024
17 checks passed
@swh76 swh76 deleted the issue789 branch March 28, 2024 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working client Changes to the client code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant