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

Add read until terminator support for ip instrument #971

Closed
wants to merge 9 commits into from
56 changes: 50 additions & 6 deletions qcodes/instrument/ip.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ class IPInstrument(Instrument):
r"""
Bare socket ethernet instrument implementation.

Before using the IPInstrument you should strongly consider if you can
use Visa over raw sockets connecting to an address in the form:
`TCPIP[board]::host address::port::SOCKET`

Args:
name (str): What this instrument is called locally.

Expand All @@ -25,6 +29,11 @@ class IPInstrument(Instrument):

terminator (str): Character(s) to terminate each send. Default '\n'.

read_terminator: Character(s) to look for at the end of each read.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if the read_terminator should not default to terminator

Defaults to None in which case only one recv is done
Otherwise keeps reading until it gets the termination
char(s).

persistent (bool): Whether to leave the socket open between calls.
Default True.

Expand All @@ -40,13 +49,16 @@ class IPInstrument(Instrument):

def __init__(self, name, address=None, port=None, timeout=5,
terminator='\n', persistent=True, write_confirmation=True,
read_terminator=None,
**kwargs):
super().__init__(name, **kwargs)

self._address = address
self._port = port
self._timeout = timeout
self._terminator = terminator
self._read_terminator = read_terminator

self._confirmation = write_confirmation

self._ensure_connection = EnsureConnection(self)
Expand All @@ -56,6 +68,22 @@ def __init__(self, name, address=None, port=None, timeout=5,

self.set_persistent(persistent)

@property
def write_terminator(self):
return self._terminator

@write_terminator.setter
def write_terminator(self, value):
self._terminator = value

@property
def read_terminator(self):
return self._read_terminator

@read_terminator.setter
def read_terminator(self, value):
self._read_terminator = value

def set_address(self, address=None, port=None):
"""
Change the IP address and/or port of this instrument.
Expand Down Expand Up @@ -142,10 +170,25 @@ def _send(self, cmd):
self._socket.sendall(data.encode())

def _recv(self):
result = self._socket.recv(self._buffer_size)
if result == b'':
log.warning("Got empty response from Socket recv() "
"Connection broken.")
result = b''
while True:
partresult = self._socket.recv(self._buffer_size)
result += partresult
if partresult == b'':
log.warning("Got empty response from Socket recv() "
"Connection broken.")
if self.read_terminator == None:
break
rtl = len(self.read_terminator)
rtloc = result.find(self.read_terminator)
if rtloc >= 0:
if rtloc + rtl < len(result):
log.warning("Multiple (partial) results received. "
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can have a look at this to see how we can handle receiving multiple partial messages: https://gist.github.com/sohailc/1a5db7bb260f36b94b552e630ea844f6

Copy link
Member

Choose a reason for hiding this comment

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

"All but the first result will be discarded. "
"Discarding {}".format(result[rtloc+rtl:]))
result = result[0:rtloc]
break

return result.decode()

def close(self):
Expand Down Expand Up @@ -183,7 +226,7 @@ def ask_raw(self, cmd):
def __del__(self):
self.close()

def snapshot_base(self, update=False):
def snapshot_base(self, update=False, params_to_skip_update=None):
Copy link
Member

Choose a reason for hiding this comment

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

Please add this arg in the docstring with a description what "params_to_skip_update" means

"""
State of the instrument as a JSON-compatible dict.

Expand All @@ -194,7 +237,8 @@ def snapshot_base(self, update=False):
Returns:
dict: base snapshot
"""
snap = super().snapshot_base(update=update)
snap = super().snapshot_base(update=update,
params_to_skip_update=params_to_skip_update)

snap['port'] = self._port
snap['confirmation'] = self._confirmation
Expand Down
1 change: 1 addition & 0 deletions qcodes/instrument/ip_to_visa.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class up through the chain) and explicitly reimplementing the __init__

def __init__(self, name, address, port, visalib,
metadata=None, device_clear=False, terminator='\n',
read_terminator=None,
timeout=3, **kwargs):

# remove IPInstrument-specific kwargs
Expand Down
2 changes: 2 additions & 0 deletions qcodes/instrument_drivers/american_magnetics/AMI430.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,11 @@ class AMI430(IPInstrument):

def __init__(self, name, address=None, port=None,
reset=False, terminator='\r\n',
read_terminator='\r\n',
current_ramp_limit=None, **kwargs):

super().__init__(name, address, port, terminator=terminator,
read_terminator=read_terminator,
write_confirmation=False, **kwargs)
self._parent_instrument = None

Expand Down