-
Notifications
You must be signed in to change notification settings - Fork 127
Changed two-tone test assertions #657
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: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Macy Libed <[email protected]>
tfcollins
left a comment
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.
I think this change makes sense but there is an existing bug in case the tones are far outside the tolerance (else condition) then the test will pass erroneously. Can you add an else clause?
Signed-off-by: Macy Libed <[email protected]>
|
Thanks! I simplified the assertion. In this version, if the tones are outside the tolerance, the assertions will still fail as expected, so it won’t pass erroneously. |
|
@macylibed9 please run the precommit formatter |
aafad17 to
e8441c5
Compare
Signed-off-by: Macy Libed <[email protected]>
e8441c5 to
2b9ccef
Compare
done with precommit and lints are okay already. Thanks |
|
Hardware tests are failing. Can you check what is going on? Tests that use the updated functions fail |
Signed-off-by: Macy Libed <[email protected]>
Description
In dds_two_tone, there was a function that originally just grabbed the two highest amplitudes, but that didn’t guarantee they'd be the peaks closest to Frequency1 and Frequency2. So sometimes, it would just give you two peaks from the same frequency.
To fix it, I split the peak detection—one for Frequency1 and one for Frequency2—so each one gets its correct peak. Also, I cleaned up the assertions to make them more straightforward.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How has this been tested?
Tested it on different actual boards, including ZC702 with FMCOMMS2-3 and ZedBoard with FMCOMMS4. I tried different values for frequency, scale, and peak_min to verify the behavior.
Documentation
If this is a new feature or example please mention or link any documentation. All new hardware interface classes require documentation.
Checklist: