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

B1500: Add measurement status to current/voltage parameters of B1517A SMU for spot measurements #2051

Merged
merged 7 commits into from
Jun 17, 2020
Merged

B1500: Add measurement status to current/voltage parameters of B1517A SMU for spot measurements #2051

merged 7 commits into from
Jun 17, 2020

Conversation

astafan8
Copy link
Contributor

No description provided.

astafan8
astafan8 commented Jun 16, 2020
View reviewed changes
qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py Outdated
@@ -622,6 +650,8 @@ def _set_current(self, value: float) -> None:
)
self.write(msg.message)

self.current._measurement_status = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jenshnielsen @lakhotiaharshit shall i extract a parameter class with these get/set methods in order to make it clearer and NOT access private _measurement_status attribute in the B1517A CMU class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jenshnielsen @lakhotiaharshit just finished the exctraction - could you have another look?

@codecov
Copy link

codecov bot commented Jun 16, 2020 •
edited
Loading

Codecov Report

Merging #2051 into master will decrease coverage by 0.76%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master    #2051      +/-   ##
==========================================
- Coverage   71.40%   70.63%   -0.77%     
==========================================
  Files         149      149              
  Lines       20032    20054      +22     
==========================================
- Hits        14304    14166     -138     
- Misses       5728     5888     +160

lakhotiaharshit
lakhotiaharshit approved these changes Jun 16, 2020
View reviewed changes
Copy link
Contributor

@lakhotiaharshit lakhotiaharshit left a comment

Choose a reason for hiding this comment

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

this is great! Thanks :)

@astafan8 astafan8 merged commit 6776722 into microsoft:master Jun 17, 2020
@astafan8
Copy link
Contributor Author

dang! i accidentally pressed merged while i just wanted to update the branch. i'm very sorry. i'm gonna test this on the instrument tomorrow. in the meantime if you have any review comments let me know, i will fix them in another PR.

@astafan8 astafan8 deleted the spot-status branch June 18, 2020 08:51
This was referenced Jun 18, 2020
Closed
Closed
Merged
@astafan8
Copy link
Contributor Author

tested on the instrument and it does work. also the master branch build successfully. so the merge of this branch luckily actually went well :)

astafan8 added a commit that referenced this pull request Jun 22, 2020
@astafan8
Merge pull request #2051 from astafan8/spot-status
2136a8b 
B1500: Add measurement status to current/voltage parameters of B1517A SMU for spot measurements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenshnielsen jenshnielsen jenshnielsen approved these changes

@lakhotiaharshit lakhotiaharshit lakhotiaharshit 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
@astafan8 @jenshnielsen @lakhotiaharshit