-
Notifications
You must be signed in to change notification settings - Fork 322
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
Feature/dem 804/implement missing uhfli commands #1500
Feature/dem 804/implement missing uhfli commands #1500
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1500 +/- ##
=======================================
Coverage 73.84% 73.84%
=======================================
Files 92 92
Lines 10459 10459
=======================================
Hits 7723 7723
Misses 2736 2736 |
Will this not only work if you have the Multi-frequency upgrade? See #978 where i started implementing this too |
@jenshnielsen that could be. I'll see if we have a device without the upgrade for further testing/developing. |
@qSaevar We have one without the upgrade so I can test that side but I do not have one with the upgrade. I never finished the other pr because of I couldn't test it probably. |
@jenshnielsen I added a check, as in your PR, to see if MF is enabled. I can test this on a device later today. |
@jenshnielsen I merged the code from PR 978 into this one and tested it on a device with MF enabled and it works. I do not have a device with out MF so I can not test that, |
@qSaevar Thanks, I will test it on our device without mf. Hopefully tomorrow. |
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 have verified that this works correctly without the mf functionality.
@QCoDeS/core If anyone else wants a look please do. I have been involved in
writing some of this code originally so an independent review would be good
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.
The code looks correct. (but there is generally quite some room for refactoring and readability improvements.)
@astafan8 I addressed the comments you made. |
Merge: f95029f 5458a09 Author: Mikhail Astafev <[email protected]> Merge pull request #1500 from qutech-sd/feature/DEM-804/Implement-missing-UHFLI-commands
Changes proposed in this pull request:
signal_outputN_enable
andsignal_outputN_amplitude
tosignalN_outputM_enable1
andsignalN_outputM_amplitude
in ZIUHFLI driver.@WilliamHPNielsen