fix to multi-usrp-rfnoc 'set_master_clock_rate' function#914
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
@TalFel Which device did you test this on? I'm guessing E320? |
I’ve added a DUC/DDC to an E310. |
|
@TalFel Just a quick note to say sorry we haven't handled this yet, we're in the middle of preparing a release (which this will sadly not be part of), but we'll come back to this soon. |
mbr0wn
left a comment
There was a problem hiding this comment.
I was thinking if property propagation shouldn't take care of this... but of course the main issue is the cached values. This looks good.
|
@TalFel there was a missing corner case when you don't have DDCs or DUCs. I updated the PR internally and when we merge into master, this will get closed. You don't need to do anything. Thanks again for this fix! |
Description
RFNoCsmulti_usrpdid not fully propagateset_master_clock_rate()to the DUC/DDC chain.Before this change,
set_master_clock_rate()updated only the radio rate. If a channel had a DUC or DDC, the adjacent converter rate was left stale:That caused follow-up
set_tx_rate()/set_rx_rate()calls on the samemulti_usrpobject to operate on outdated DUC/DDC state. In practice, after changing the master clock, setting the channel sample rate could be coerced unexpectedly because interpolation/decimation was derived from the old converter-side rate.This change updates
multi_usrp_rfnocso thatset_master_clock_rate():Which devices/areas does this affect?
multi_usrp_rfnocobject functionality.Testing Done
Initialized
multi_usrpwithmaster_clock_rate=12.345e6set_master_clock_rate(10e6)
set_tx_rate(1e6) // <- before this change, this was unexpectedly coerced even though the new master clock(10e6) should be interpolated by 10.
After the change, there is no coercion.
Checklist
MPM compat, noc_shell, specific RFNoC block, ...)