-
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
B1520A: CVSweepMeasurement class works with all impedance models #2047
B1520A: CVSweepMeasurement class works with all impedance models #2047
Conversation
# Conflicts: # qcodes/tests/drivers/keysight_b1500/b1500_driver_tests/test_b1520a_cmu.py
Codecov Report
@@ Coverage Diff @@
## master #2047 +/- ##
==========================================
+ Coverage 71.34% 71.41% +0.07%
==========================================
Files 149 149
Lines 19939 19985 +46
==========================================
+ Hits 14226 14273 +47
+ Misses 5713 5712 -1 |
qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1500_module.py
Outdated
Show resolved
Hide resolved
qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1500_module.py
Show resolved
Hide resolved
qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1520A.py
Outdated
Show resolved
Hide resolved
qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1520A.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Mikhail Astafev <[email protected]>
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.
Looks great! one question - does the output of the "capacitance" parameter also change with the change of the impedance model? if so, shall we also update the label/unit of that parameter upon model change? or we just restrict that parameter to the Cp_D model and do the enhancement in another PR? (i think i like the latter for now)
I think this is not the part of this PR. Even before this PR user can change the impedance model and get the capacitance; capacitance parameter was not restrained to Cp_D. I think this whole problem should be fixed in a separate PR. |
B1520A: CVSweepMeasurement class works with all impedance models
Earlier driver supported only Cp_D (Capacitance, Dissipation) impedance model. This was hardcoded in the driver. This PR removes these constraints and supports all possible impedance models.
@astafan8 @jenshnielsen