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

Driver / signalhound use standard logging #1154

Merged
merged 9 commits into from
Jul 20, 2018
Merged

Driver / signalhound use standard logging #1154

Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e6f93a5
Change docstrings to double quotes
WilliamHPNielsen Jun 20, 2018
6005932
Use module level logger and update messages
WilliamHPNielsen Jun 20, 2018
a39e038
Remove dead (commented-out) code
WilliamHPNielsen Jun 20, 2018
ca15e12
Add comment justifying print
WilliamHPNielsen Jun 20, 2018
dc97643
Fix typo
WilliamHPNielsen Jun 20, 2018
817c1c6
Merge branch 'master' into fix/driver/signalhound_logging
jenshnielsen Jul 20, 2018
e3dd1e3
Remove bad joke from comments
WilliamHPNielsen Jul 20, 2018
9d674cc
Merge branch 'fix/driver/signalhound_logging' of github.com:WilliamHP…
WilliamHPNielsen Jul 20, 2018
880a751
Merge branch 'master' into fix/driver/signalhound_logging
WilliamHPNielsen Jul 20, 2018
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
97 changes: 45 additions & 52 deletions qcodes/instrument_drivers/signal_hound/USB_SA124B.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,20 @@

from qcodes import Instrument, validators as vals

log = logging.getLogger(__name__)


class SignalHound_USB_SA124B(Instrument):
'''
"""
This is a direct port of the signal hound QTLab driver by Ramiro
Edited by Adriaan Rol
Edited by Adriaan Rol and others (use git blame for details)
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this comment useful for? i'd remove it since "git blame" is supposedly a known feature of git :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was trying to humorously point out that information about who wrote what does not really belong in a driver... From your comment I guess that I failed :) You can remove the whole shebang if you'd like.

Copy link
Contributor

Choose a reason for hiding this comment

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

😸 ok, i get it. well, I think that such a note is good fun for github comments, but should not be part of the code. So, I suggest to remove this line.


Status: Beta version.
This driver is functional but not all features have been implemented.

TODO:
Add tracking generator mode
Copy link
Contributor

@astafan8 astafan8 Jun 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

can this line be reformulate from "TODO" to smth like : "Note that this driver does not support tracking generator mode". and then perhaps make a separate low-prio issue for "enabling tracking generator mode for ??? driver"? This will make the code distraction-free and potential-ambiguity-free

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I think it's better kept as it is. Missing driver features are non-issues until they become issues, if you know what I mean.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, agree. still, when I see "TODO", I understand it as developer-related thing, or a notion of a bug or deficiency. When the statement is formulated as smth like "Note that this driver does not support tracking generator mode", as a user I know that it hasn't been an issue so far and I do not expect the feature to be implemented UNLESS as a user I make an issue out of it on github.

'''
"""
dll_path = 'C:\Windows\System32\sa_api.dll'

saStatus = {
Expand Down Expand Up @@ -57,9 +59,7 @@ def __init__(self, name, dll_path=None, **kwargs):
t0 = time()
super().__init__(name, **kwargs)

self.log = logging.getLogger('Main.DeviceInt')
logging.info(__name__ +
' : Initializing instrument SignalHound USB 124A')
log.info('Initializing instrument SignalHound USB 124A')
self.dll = ct.CDLL(dll_path or self.dll_path)
self.hf = constants

Expand Down Expand Up @@ -141,14 +141,16 @@ def __init__(self, name, dll_path=None, **kwargs):
self.device_type()

t1 = time()
# poor-man's connect_message. We could overwrite get_idn
# instead and use connect_message.
Copy link
Contributor

Choose a reason for hiding this comment

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

does this supposed to be an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah.

Copy link
Contributor

Choose a reason for hiding this comment

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

then why making the comment? :)

print('Initialized SignalHound in %.2fs' % (t1-t0))

@classmethod
def default_server_name(cls, **kwargs):
return 'USB'

def openDevice(self):
self.log.info('Opening Device')
log.info('Opening Device')
self.deviceHandle = ct.c_int(0)
deviceHandlePnt = ct.pointer(self.deviceHandle)
ret = self.dll.saOpenDevice(deviceHandlePnt)
Expand All @@ -166,28 +168,28 @@ def openDevice(self):
self.get('device_type')

def closeDevice(self):
self.log.info('Closing Device with handle num: ',
self.deviceHandle.value)
log.info('Closing Device with handle num: '
f'{self.deviceHandle.value}')

try:
self.dll.saAbort(self.deviceHandle)
self.log.info('Running acquistion aborted.')
log.info('Running acquistion aborted.')
except Exception as e:
self.log.info('Could not abort acquisition: %s', e)
log.info(f'Could not abort acquisition: {e}')

ret = self.dll.saCloseDevice(self.deviceHandle)
if ret != self.saStatus['saNoError']:
raise ValueError('Error closing device!')
print('Closed Device with handle num: ', self.deviceHandle.value)
log.info(f'Closed Device with handle num: {self.deviceHandle.value}')
self.devOpen = False
self.running(False)

def abort(self):
self.log.info('Stopping acquisition')
log.info('Stopping acquisition')

err = self.dll.saAbort(self.deviceHandle)
if err == self.saStatus['saNoError']:
self.log.info('Call to abort succeeded.')
log.info('Call to abort succeeded.')
self.running(False)
elif err == self.saStatus['saDeviceNotOpenErr']:
raise IOError('Device not open!')
Expand All @@ -198,20 +200,20 @@ def abort(self):
raise IOError('Unknown error setting abort! Error = %s' % err)

def preset(self):
self.log.warning('Performing hardware-reset of device!')
self.log.warning('Please ensure you close the device handle within '
'two seconds of this call!')
log.warning('Performing hardware-reset of device!')
log.warning('Please ensure you close the device handle within '
'two seconds of this call!')

err = self.dll.saPreset(self.deviceHandle)
if err == self.saStatus['saNoError']:
self.log.info('Call to preset succeeded.')
log.info('Call to preset succeeded.')
elif err == self.saStatus['saDeviceNotOpenErr']:
raise IOError('Device not open!')
else:
raise IOError('Unknown error calling preset! Error = %s' % err)
raise IOError(f'Unknown error calling preset! Error = {err}')

def _do_get_device_type(self):
self.log.info('Querying device for model information')
log.info('Querying device for model information')

devType = ct.c_uint(0)
devTypePnt = ct.pointer(devType)
Expand Down Expand Up @@ -263,39 +265,30 @@ def initialisation(self, flag=0):
###################################
if err == self.saStatus['saNoError']:
self.running(True)
self.log.info('Call to initiate succeeded.')
log.info('Call to initiate succeeded.')
elif err == self.saStatus['saDeviceNotOpenErr']:
raise IOError('Device not open!')
elif err == self.saStatus['saInvalidParameterErr']:
print('saInvalidParameterErr!')
print('In real-time mode, this value may be returned if the span',
'limits defined in the API header are broken. Also in',
'real-time mode, this error will be returned if the',
' resolution bandwidth is outside the limits defined in',
' the API header.')
print('In time-gate analysis mode this error will be returned if',
' span limits defined in the API header are broken. Also in',
' time gate analysis, this error is returned if the',
' bandwidth provided require more samples for processing',
' than is allowed in the gate length. To fix this, ',
'increase rbw/vbw.')
log.error(
"""
saInvalidParameterErr!
In real-time mode, this value may be returned if the span
limits defined in the API header are broken. Also in
real-time mode, this error will be returned if the
resolution bandwidth is outside the limits defined in
the API header.
In time-gate analysis mode this error will be returned if
span limits defined in the API header are broken. Also in
time gate analysis, this error is returned if the
bandwidth provided require more samples for processing
than is allowed in the gate length. To fix this
increase rbw/vbw.
"""
)
raise IOError('The value for mode did not match any known value.')
# This error code does not exist!??
# elif err == self.saStatus['saAllocationLimitError']:
# print('This value is returned in extreme circumstances. The API',
# ' currently limits the amount of RAM usage to 1GB. When',
# ' exceptional parameters are provided, such as very low ',
# 'bandwidths, or long sweep times, this error may be ',
# 'returned. At this point you have reached the boundaries of',
# ' the device. The processing algorithms are optimized for',
# ' speed at the expense of space, which is the reason',
# ' this can occur.''')
# raise IOError('Could not allocate sufficent RAM!')
elif err == self.saStatus['saBandwidthErr']:
raise IOError('RBW is larger than your span. (Sweep Mode)!')
self.check_for_error(err)
# else:
# raise IOError('Unknown error setting initiate! Error = %s' % err)

return

Expand Down Expand Up @@ -347,7 +340,7 @@ def configure(self, rejection=True):
span = self.get('span')
center = ct.c_double(frequency)
span = ct.c_double(span)
self.log.info('Setting device CenterSpan configuration.')
log.info('Setting device CenterSpan configuration.')

err = self.dll.saConfigCenterSpan(self.deviceHandle, center, span)
self.check_for_error(err)
Expand Down Expand Up @@ -379,21 +372,21 @@ def configure(self, rejection=True):
self.check_for_error(err)

# Reference Level configuration
self.log.info('Setting device reference level configuration.')
log.info('Setting device reference level configuration.')
err = self.dll.saConfigLevel(
self.deviceHandle, ct.c_double(self.get('ref_lvl')))
self.check_for_error(err)

# External Reference configuration
if self.external_reference():
self.log.info('Setting reference frequency from external source.')
log.info('Setting reference frequency from external source.')
err = self.dll.saEnableExternalReference(self.deviceHandle)
self.check_for_error(err)

if self.device_mode() == 'sweeping':
# Sweeping Configuration
reject_var = ct.c_bool(rejection)
self.log.info('Setting device Sweeping configuration.')
log.info('Setting device Sweeping configuration.')
err = self.dll.saConfigSweepCoupling(
self.deviceHandle, ct.c_double(self.get('rbw')),
ct.c_double(self.get('vbw')), reject_var)
Expand Down Expand Up @@ -461,11 +454,11 @@ def sweep(self):
return np.array([freq_points, datamin, datamax])

def get_power_at_freq(self, Navg=1):
'''
"""
Returns the maximum power in a window of 250kHz
around the specified frequency.
The integration window is specified by the VideoBandWidth (set by vbw)
'''
"""
poweratfreq = 0
for i in range(Navg):
data = self.sweep()
Expand Down