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.

Sign up for GitHub

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

Jump to bottom

Alazar drivers cleanup #2301

Merged
merged 17 commits into from
Oct 27, 2020
Merged

Alazar drivers cleanup #2301

merged 17 commits into from
Oct 27, 2020

Conversation

jenshnielsen
Copy link
Collaborator

@jenshnielsen jenshnielsen commented Oct 18, 2020 •
edited
Loading

  • Remove redundant get_cmd from all parameters. (This is the default anyway)
  • Remove redundant set_cmd from TraceParameters. TraceParamter implements set_raw so there is no need for this.
  • Rewrite loops over channels and trigger engines to be more explicit.
  • Make dll_path an argument
  • Remove deprecated config method from alazar class

This should be tested on an actual device

@codecov
Copy link

codecov bot commented Oct 18, 2020 •
edited
Loading

Codecov Report

Merging #2301 into master will increase coverage by 0.08%.
The diff coverage is 20.33%.

@@            Coverage Diff             @@
##           master    #2301      +/-   ##
==========================================
+ Coverage   62.02%   62.10%   +0.08%     
==========================================
  Files         199      199              
  Lines       26091    26057      -34     
==========================================
+ Hits        16182    16184       +2     
+ Misses       9909     9873      -36

@trevormorgan trevormorgan mentioned this pull request Oct 19, 2020
Merged
FarBo
FarBo reviewed Oct 23, 2020
View reviewed changes
qcodes/instrument_drivers/AlazarTech/ATS9360.py
for i in ['1', '2']:
self.add_parameter(name='coupling' + i,
get_cmd=None,
for i in range(1, self.channels+1):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think channels is missing to be defined.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No the default value of 2 is set in the super class AlazarTech_ATS

@jenshnielsen
Copy link
Collaborator Author

@astafan8 could you have a look too?

astafan8
astafan8 approved these changes Oct 27, 2020
View reviewed changes
Copy link
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, much cleaner!

@jenshnielsen jenshnielsen merged commit f24e2d2 into microsoft:master Oct 27, 2020
@jenshnielsen jenshnielsen deleted the alazar_cleanup branch October 27, 2020 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astafan8 astafan8 astafan8 approved these changes

@FarBo FarBo FarBo approved these changes

Assignees
No one assigned
Labels
driver
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jenshnielsen @astafan8 @FarBo