pyvisa / pyvisa-py

A pure python PyVISA backend
https://pyvisa-py.readthedocs.io
MIT License
288 stars 124 forks source link

fix gpib #132

Closed greyltc closed 6 years ago

greyltc commented 6 years ago

These changes fix gpib and implement the highlevel gpib_send_ifc and clear functions.

I've tested this with the latest linux-gpib from svn (the only way currently to get it working with Linux version 4.15.15) on a Keithley 2400 sourcemeter via a GPIB-USB-HS adapter.

I also tested it with the serial interface to make sure I didn't break anything there.

MatthieuDartiailh commented 6 years ago

Could you post your full traceback ? There has already been several iterations on this (see #119 and related PRs and issues) and others (see #105) have not complained. As I cannot test this myself (I do not have access to the proper hardware) so I prefer to be careful. Can you confirm the bug exists on both Python 2 and Python 3 ? @tivek can you confirm this issue with latest master ? Looking at @tivek gpib_ctypes, I believe there may be an issue. I am also worried that linux-gpib signature may have changed over version. (Which may explain why we have had so many back and forth iterations) Could you check this ?

greyltc commented 6 years ago

About the gpib bugs I found in master: 1) So far, I've only tested in python 3 I'll check on python2.7 and get back to you. 1) I found that read operations are broken This line breaks reads because .encode() can't be called on a bytes type object (this might not be seen in python2, I'll have a look at adding an if statement to catch this). 2) I also found that write operations are broken On a successful write operation, the write function in gpib.py only returns one value, but then the write function in highlevel.py tries to index that returned value (which is obviously impossible). This error is not possible to see unless one is testing with working hardware because the error cases are handled correctly. This must be broken for everyone, irrespective of python version.

greyltc commented 6 years ago

Anyway, here's the full traceback for the bug I fixed in GPIB writes:

# ipython
Python 3.6.4 (default, Jan  5 2018, 02:35:40) 
Type 'copyright', 'credits' or 'license' for more information
IPython 6.2.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import visa

In [2]: rm = visa.ResourceManager('@py')

In [3]: sm = rm.open_resource('GPIB0::24::INSTR')

In [4]: sm.write('*RST')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-94a8ad38a2ef> in <module>()
----> 1 sm.write('*RST')

/usr/lib/python3.6/site-packages/pyvisa/resources/messagebased.py in write(self, message, termination, encoding)
    221             message += term
    222 
--> 223         count = self.write_raw(message.encode(enco))
    224 
    225         return count

/usr/lib/python3.6/site-packages/pyvisa/resources/messagebased.py in write_raw(self, message)
    199         :rtype: int
    200         """
--> 201         return self.visalib.write(self.session, message)
    202 
    203     def write(self, message, termination=None, encoding=None):

/usr/lib/python3.6/site-packages/pyvisa-py/highlevel.py in write(self, session, data)
    278             return 0, StatusCode.error_invalid_object
    279 
--> 280         if ret[1] < 0:
    281             raise errors.VisaIOError(ret[1])
    282 

TypeError: 'StatusCode' object does not support indexing
greyltc commented 6 years ago

and here's the full traceback for the bug in gpib reads:

In [5]: sm.read()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-9b8c3635d853> in <module>()
----> 1 sm.read()

/usr/lib/python3.6/site-packages/pyvisa/resources/messagebased.py in read(self, termination, encoding)
    411         if termination is None:
    412             termination = self._read_termination
--> 413             message = self._read_raw().decode(enco)
    414         else:
    415             with self.read_termination_context(termination):

/usr/lib/python3.6/site-packages/pyvisa/resources/messagebased.py in _read_raw(self, size)
    384                     logger.debug('%s - reading %d bytes (last status %r)',
    385                                  self._resource_name, size, status)
--> 386                     chunk, status = self.visalib.read(self.session, size)
    387                     ret.extend(chunk)
    388             except errors.VisaIOError as e:

/usr/lib/python3.6/site-packages/pyvisa-py/highlevel.py in read(self, session, count)
    251         # from the session handle, dispatch to the read method of the session object.
    252         try:
--> 253             ret = self.sessions[session].read(count)
    254         except KeyError:
    255             return 0, StatusCode.error_invalid_object

/usr/lib/python3.6/site-packages/pyvisa-py/gpib.py in read(self, count)
    139         reader = lambda: self.interface.read(count).encode('ascii')
    140 
--> 141         return self._read(reader, count, checker, False, None, False, gpib.GpibError)
    142 
    143     def write(self, data):

/usr/lib/python3.6/site-packages/pyvisa-py/sessions.py in _read(self, reader, count, end_indicator_checker, suppress_end_en, termination_char, termination_char_en, timeout_exception)
    386         while True:
    387             try:
--> 388                 current = reader()
    389             except timeout_exception:
    390                 return out, StatusCode.error_timeout

/usr/lib/python3.6/site-packages/pyvisa-py/gpib.py in <lambda>()
    137         checker = lambda current: self.interface.ibsta() & 8192
    138 
--> 139         reader = lambda: self.interface.read(count).encode('ascii')
    140 
    141         return self._read(reader, count, checker, False, None, False, gpib.GpibError)

AttributeError: 'bytes' object has no attribute 'encode'
greyltc commented 6 years ago

Traceback for broken gpib writes in python2:

$ ipython2
Python 2.7.14 (default, Jan  5 2018, 10:41:29) 
Type "copyright", "credits" or "license" for more information.

IPython 5.5.0 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: import visa

In [2]: rm = visa.ResourceManager('@py')

In [3]: sm = rm.open_resource('GPIB0::24::INSTR')

In [4]: sm.write('*RST')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-94a8ad38a2ef> in <module>()
----> 1 sm.write('*RST')

/usr/lib/python2.7/site-packages/pyvisa/resources/messagebased.pyc in write(self, message, termination, encoding)
    205             message += term
    206 
--> 207         count = self.write_raw(message.encode(enco))
    208 
    209         return count

/usr/lib/python2.7/site-packages/pyvisa/resources/messagebased.pyc in write_raw(self, message)
    183         :rtype: int
    184         """
--> 185         return self.visalib.write(self.session, message)
    186 
    187     def write(self, message, termination=None, encoding=None):

/usr/lib/python2.7/site-packages/pyvisa-py/highlevel.pyc in write(self, session, data)
    278             return 0, StatusCode.error_invalid_object
    279 
--> 280         if ret[1] < 0:
    281             raise errors.VisaIOError(ret[1])
    282 

TypeError: 'StatusCode' object does not support indexing

In [5]:
greyltc commented 6 years ago

Reads do work though in python2:

In [5]: sm.read()
Out[5]: u'KEITHLEY INSTRUMENTS INC.,MODEL 2400,[REDACTED],C34 Sep 21 2016 15:30:00/A02  /K/J\n'

I'll add an if to my PR so that this continues to work.

MatthieuDartiailh commented 6 years ago

Does your fix break Python 2 without the version guard ? I find surprising that we get unicode on Python 2 and bytes on Python 3 from linux-gpib, this looks so wrong.

greyltc commented 6 years ago

My python2 fix in 51d9f65 was bad. Reverted in 918ebc5 Looking into that now.

greyltc commented 6 years ago

Okay. In python2, the read function in linux-gpib returns a str type. In python3 it returns a bytes type. That doesn't seem too out of the ordinary to me. However, .encode('ascii') should not be used in either case. So the PR as it stands right now tests fine in both cases, there's no need for an if statement for python2.

All of the user controlled encoding seems to be taken care of properly in the read wrapper function in pyvisa/messagebased.py

greyltc commented 6 years ago

So is there anything now that's preventing a merge here?

tivek commented 6 years ago

I will be able to test the latest master with linux-gpib and gpib-ctypes under Linux and on Windows (NI GPIB) some time tomorrow when I get access to the equipment. Will get back with what I find.

MatthieuDartiailh commented 6 years ago

Thanks @tivek. If you have time enough, please check this branch too (I suspect master will fail for you). @greyltc once @tivek can confirm your patch work I will merge.

greyltc commented 6 years ago

Hey @tivek, will you be able to test with python2 and 3? What version of linux-gpib will you use?

tivek commented 6 years ago

Sourceforge seems to be down at the moment so I can't pull linux-gpib yet :/

@greyltc, I'm planning to use the latest svn commit. AFAICT 4.15 changed the timer interface so the latest linux-gpib release does not work on my kernel 4.15.12. I should be able to test both python 2 and 3.

greyltc commented 6 years ago

What Linux are you on? I'm using Arch and I have a linux-gpib-svn package in the AUR if you happen to be on Arch too.

I bet SF isn't down. They (linux-gpib devs) changed the folder structure in the past few days. Try pulling r1730. That's before the big folder structure change.

tivek commented 6 years ago

@greyltc, thanks for your help. Also on Arch.

SF was actually down for a day or so, ironically it came back now - right after I already found your linux-gpib-svn PKGBUILD, pointed it to your github fork and built linux-gpib tip :)

greyltc commented 6 years ago

FYI @tivek , I just fixed my linux-gpib-svn AUR package to build without modification. It points to r1730 for now until I can get the latest svn to build

You should notice that that PKGBUILD has a variable for you to edit to build for python2

tivek commented 6 years ago

Took me a while, but here goes:

Linux kernel 4.15.15-1-ARCH linux-gpib-svn r1730-1 built from AUR Python 3.6.4 and 2.7.14 PyVISA 1.9.0 from PyPI

With pyvisa-py master, the above GPIB read and write bugs are consistently reproduced in both Python 2 and Python 3.

I can also confirm that #132 fixes reads and writes which now work as expected both for Python 2 and Python 3.

MatthieuDartiailh commented 6 years ago

Thanks to both of you for your work on this ! Merging ! By the way @tivek, did you manage to release gpib-ctypes ?

bors r+

bors[bot] commented 6 years ago

Build succeeded

tivek commented 6 years ago

@MatthieuDartiailh, yes, you can install it using pip install gpib_ctypes. Then please try it with pyvisa-py.

MatthieuDartiailh commented 6 years ago

Thanks for the tip @tivek. I still hope to test this on windows but I did not manage to do it yet.