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

Driver / Keysight : Add 33510B to model list #1544

Merged
merged 3 commits into from
May 10, 2019
Merged

Driver / Keysight : Add 33510B to model list #1544

merged 3 commits into from
May 10, 2019

Conversation

occoder
Copy link
Contributor

@occoder occoder commented Apr 24, 2019

and make the dict style more diff friendly

and make the tuple style more diff friendly
@WilliamHPNielsen WilliamHPNielsen changed the title Add 33510B to model list Driver / Keysight : Add 33510B to model list Apr 24, 2019
@codecov
Copy link

codecov bot commented Apr 24, 2019

Codecov Report

Merging #1544 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1544   +/-   ##
=======================================
  Coverage   71.33%   71.33%           
=======================================
  Files         104      104           
  Lines       12041    12041           
=======================================
  Hits         8589     8589           
  Misses       3452     3452

@WilliamHPNielsen
Copy link
Contributor

Hi @occoder, and welcome to QCoDeS! Thank you for improving the Keysight driver.

Just to be completely sure, have you tested that the driver works correctly with the 33510B model and that the only difference between that and other models is the max frequency?

@occoder
Copy link
Contributor Author

occoder commented Apr 25, 2019

Hi @WilliamHPNielsen
So far, I just tested a few commands shown in Qcodes/docs/examples/driver_examples/Qcodes example with Keysight 33500B.ipynb on the real 33510B hardware. They all worked sound.
I may not be able to cover all the corner cases at present.
As a start point, this change seems to be sufficient.
Cheers.

@WilliamHPNielsen
Copy link
Contributor

I guess we can do no better than to say "if the example notebook works, everything works".

@astafan8 astafan8 merged commit ce97b7d into microsoft:master May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants