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

fix: mode dependent parameters and change code style #298

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 129 additions & 53 deletions qcodes/instrument_drivers/tektronix/Keithley_2000.py
Original file line number Diff line number Diff line change
@@ -1,117 +1,143 @@
from qcodes import VisaInstrument
from qcodes.utils.validators import Numbers, Ints, Enum, MultiType

import logging
from functools import partial

def parse_output_string(s):
""" Parses and cleans string outputs of the Keithley """
# Remove surrounding whitespace and newline characters
s = s.strip()
from qcodes.instrument.visa import VisaInstrument
from qcodes.utils.validators import Strings as StringValidator
from qcodes.utils.validators import Ints as IntsValidator
from qcodes.utils.validators import Numbers as NumbersValidator

# Remove surrounding quotes
if (s[0] == s[-1]) and s.startswith(("'", '"')):
s = s[1:-1]
from qcodes.utils.validators import Enum, MultiType

s = s.lower()
# %% Helper functions

# Convert some results to a better readable version
conversions = {
'mov': 'moving',
'rep': 'repeat',
}

if s in conversions.keys():
s = conversions[s]
def bool_to_str(val):
'''
Function to convert boolean to 'ON' or 'OFF'
'''
if val:
return "ON"
else:
return "OFF"

return s
# %% Driver for Keithley_2000


def parseint(v):
logging.debug('parseint: %s -> %d' % (v, int(v)))
return int(v)


def parsebool(v):
r = bool(int(v))
logging.debug('parsetobool: %s -> %d' % (v, r))
return r


def parsestr(v):
return v.strip().strip('"')

def parse_output_bool(value):
return 'on' if int(value) == 1 else 'off'

class Keithley_2000(VisaInstrument):
"""
Driver for the Keithley 2000 multimeter.
"""
'''
This is the qcodes driver for the Keithley_2000 Multimeter

Usage: Initialize with
<name> = = Keithley_2700(<name>, address='<GPIB address>', reset=<bool>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Keithley_2000

change_display=<bool>, change_autozero=<bool>)

Status: beta-version.

This driver will most likely work for multiple Keithley SourceMeters.

This driver does not contain all commands available, but only the ones
most commonly used.
'''
def __init__(self, name, address, reset=False, **kwargs):
super().__init__(name, address, **kwargs)

self._modes = ['VOLT:AC', 'VOLT:DC', 'CURR:AC', 'CURR:DC', 'RES',
'FRES', 'TEMP', 'FREQ']
self._averaging_types = ['MOV', 'REP']
self._trigger_sent = False

self.add_parameter('mode',
get_cmd='SENS:FUNC?',
set_cmd="SENS:FUNC {}",
val_mapping={
'ac current': '"CURR:AC"\n',
'dc current': '"CURR:DC"\n',
'ac voltage': '"VOLT:AC"\n',
'dc voltage': '"VOLT:DC"\n',
'2w resistance': '"RES"\n',
'4w resistance': '"FRES"\n',
'temperature': '"TEMP"\n',
'frequency': '"FREQ"\n',
})
get_cmd=':CONF?',
get_parser=parsestr,
set_cmd=':CONF:{}',
vals=StringValidator())
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the meat of the change, I guess - that the actual stored value in self.mode will be essentially the raw command string? That seems harder to use, and impossible for us to standardize across different instruments. I think it would be nice if we could converge on a qcodes standard for these, and while I have no particular attachment the exact ones @Rubenknex put in this val_mapping, they are explicit and lowercase so fit the rest of our code base well.

To make that work with the mode-dependent parameters, you could extract this dictionary into an attribute, can be a class attribute _mode_map = {...} and then in _set/get_mode_param instead of self.mode() you would put self._mode_map[self.mode()]

Side note 1: if we were to leave it as is, we should at least restrict to the exact modes you're allowed to use, which in this case would be vals=Enum(*self._modes). I know the way you have it just mimics the K2700, but we should fix this there as well. Without that it's very hard to figure out what your options are when you want to use the parameter.

Side note 2: If you put terminator='\n' in the super().__init__ call (see K2600 for example) then you won't have to worry about the '\n' getting returned that you have to strip out yourself with parsestr.


# Mode specific parameters
self.add_parameter('nplc',
get_cmd=partial(self._get_mode_param, 'NPLC', float),
get_cmd=partial(self._get_mode_param, 'NPLC',
float),
set_cmd=partial(self._set_mode_param, 'NPLC'),
vals=Numbers(min_value=0.01, max_value=10))
vals=NumbersValidator(min_value=0.01, max_value=10))

# TODO: validator, this one is more difficult since different modes
# require different validation ranges
self.add_parameter('range',
get_cmd=partial(self._get_mode_param, 'RANG', float),
get_cmd=partial(self._get_mode_param, 'RANG',
float),
set_cmd=partial(self._set_mode_param, 'RANG'),
vals=Numbers())
vals=NumbersValidator())

self.add_parameter('auto_range',
get_cmd=partial(self._get_mode_param, 'RANG:AUTO', parse_output_bool),
get_cmd=partial(self._get_mode_param, 'RANG:AUTO',
parser=parsebool),
set_cmd=partial(self._set_mode_param, 'RANG:AUTO'),
vals=Enum('on', 'off'))

self.add_parameter('digits',
get_cmd=partial(self._get_mode_param, 'DIG', int),
set_cmd=partial(self._set_mode_param, 'DIG'),
vals=Ints(min_value=4, max_value=7))
vals=IntsValidator(min_value=4, max_value=7))

self.add_parameter('averaging_type',
get_cmd=partial(self._get_mode_param, 'AVER:TCON', parse_output_string),
get_cmd=partial(self._get_mode_param, 'AVER:TCON',
parsestr),
set_cmd=partial(self._set_mode_param, 'AVER:TCON'),
vals=Enum('moving', 'repeat'))

self.add_parameter('averaging_count',
get_cmd=partial(self._get_mode_param, 'AVER:COUN', int),
get_cmd=partial(self._get_mode_param, 'AVER:COUN',
int),
set_cmd=partial(self._set_mode_param, 'AVER:COUN'),
vals=Ints(min_value=1, max_value=100))
vals=IntsValidator(min_value=1, max_value=100))

self.add_parameter('averaging',
get_cmd=partial(self._get_mode_param, 'AVER:STAT', parse_output_bool),
get_cmd=partial(self._get_mode_param, 'AVER:STAT',
parser=parsebool),
set_cmd=partial(self._set_mode_param, 'AVER:STAT'),
vals=Enum('on', 'off'))

# Global parameters
self.add_parameter('display',
get_cmd='DISP:ENAB?',
get_parser=parse_output_bool,
set_cmd='DISP:ENAB {}',
get_cmd=self._mode_par('DISP', 'ENAB'),
get_parser=parsebool,
set_cmd=self._mode_par_value('DISP', 'ENAB', '{}'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is the only place you use self._mode_par and self._mode_par_value? I don't see that they add anything here, and they make it quite a bit more confusing over the straight command strings that were there before. For example, they make it look like something here is dynamic, but in this particular usage they get resolved to strings when the parameter is created so there's no difference vs the strings that were there previously.

vals=Enum('on', 'off'))

self.add_parameter('trigger_continuous',
get_cmd='INIT:CONT?',
get_parser=parse_output_bool,
get_parser=parsebool,
set_cmd='INIT:CONT {}',
vals=Enum('on', 'off'))

self.add_parameter('trigger_count',
get_cmd='TRIG:COUN?',
set_cmd='TRIG:COUN {}',
vals=MultiType(Ints(min_value=1, max_value=9999), Enum('inf')))
vals=MultiType(IntsValidator(min_value=1,
max_value=9999),
Enum('inf')))

self.add_parameter('trigger_delay',
get_cmd='TRIG:DEL?',
set_cmd='TRIG:DEL {}',
units='s',
vals=Numbers(min_value=0, max_value=999999.999))
vals=NumbersValidator(min_value=0,
max_value=999999.999))

self.add_parameter('trigger_source',
get_cmd='TRIG:SOUR?',
Expand All @@ -128,7 +154,8 @@ def __init__(self, name, address, reset=False, **kwargs):
get_cmd='TRIG:TIM?',
set_cmd='TRIG:TIM {}',
units='s',
vals=Numbers(min_value=0.001, max_value=999999.999))
vals=NumbersValidator(min_value=0.001,
max_value=999999.999))

self.add_parameter('amplitude',
units='arb.unit',
Expand All @@ -141,6 +168,55 @@ def __init__(self, name, address, reset=False, **kwargs):

self.connect_message()

def _determine_mode(self, mode):
'''
Return the mode string to use.
If mode is None it will return the currently selected mode.
'''
logging.debug('Determine mode with mode=%s' % mode)
if mode is None:
mode = self.mode.get_latest() # _mode(query=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

obsolete comment?

if mode not in self._modes and mode not in ('INIT', 'TRIG', 'SYST',
'DISP'):
logging.warning('Invalid mode %s, assuming current' % mode)
mode = self.mode.get_latest()
logging.debug('Determine mode: mode=%s' % mode)
return mode

def _mode_par_value(self, mode, par, val):
'''
For internal use only!!
Create command string based on current mode

Input:
mode (string) : The mode to use
par (string) : Parameter
val (depends) : Value

Output:
None
Copy link
Contributor

Choose a reason for hiding this comment

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

The expected form for the docstrings is to use Args: and Returns: rather than Input: and Output: - and the return is not None, it's a string, the command to be sent.

Also, I'd omit "For internal use only!!" - that's what the leading underscore means.

Same for _mode_par below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, please use triple DOUBLE quotes """ rather than ''' for docstrings.

'''
mode = self._determine_mode(mode)
string = ':%s:%s %s' % (mode, par, val)
return string

def _mode_par(self, mode, par):
'''
For internal use only!!
Create command string based on current mode

Input:
mode (string) : The mode to use
par (string) : Parameter
val (depends) : Value

Output:
None
'''
mode = self._determine_mode(mode)
string = ':%s:%s?' % (mode, par, )
return string

def trigger(self):
if self.trigger_continuous() == 'off':
self.write('INIT')
Expand All @@ -163,4 +239,4 @@ def _get_mode_param(self, parameter, parser):
def _set_mode_param(self, parameter, value):
cmd = '{}:{} {}'.format(self.mode(), parameter, value)

self.write(cmd)
self.write(cmd)