search

pyvisa / pyvisa-py

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

Work-in-progress: GPIB support #26

Closed bessman closed 9 years ago

bessman commented 9 years ago

NOT READY FOR MERGE! I'm opening a pull request because I'd appreciate some input on what I've got so far.

This implements a very basic GPIBSession via linux-gpib. It kind of works; instr.write() works, instr.read() mostly works, but sometimes takes very long to return, not sure why. instr.query_ascii_values() does not work; for example inst.query_ascii_values('*IDN?') fails with ValueError: invalid literal for float(): KEITHLEY INSTRUMENTS INC. for a Keithley 2000 digital multimeter.

See also the various TODOs strewn about the code.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-1.21%) to 23.65% when pulling 2a29e2c1857261924d64a75e4dbc9351def475ad on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

hgrecco commented 9 years ago

Nice work. I made a few comments. Can you tell me in which line you are getting ValueError when you query IDN?

bessman commented 9 years ago

Thanks for your feedback. query_ascii_values() errors out like this: 

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.6/site-packages/pyvisa/resources/messagebased.py", line 434, in query_ascii_values
    return from_ascii_block(block, converter, separator, container)
  File "/usr/lib/python2.6/site-packages/pyvisa/util.py", line 195, in from_ascii_block
    return container(converter(raw_value) for raw_value in data)
  File "/usr/lib/python2.6/site-packages/pyvisa/util.py", line 195, in <genexpr>
    return container(converter(raw_value) for raw_value in data)
ValueError: invalid literal for float(): 0KEITHLEY INSTRUMENTS INC.

Interestingly, query() does not throw an exception, but nevertheless behaves quite strange:

>>> test.query('*IDN?')
u'9KEITHLEY INSTRUMENT'
>>> test.query('*IDN?')
u'SKEITHLEY INSTRUMENTS '
>>> test.query('*IDN?')
u'IKEITHLEY INSTRUMENTS I'

It looks like the read ends prematurely, and a single unread byte of that message is then prepended to the next message.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.54%) to 24.33% when pulling 0ce8b7161039c20f0f0ad7e630e8aba5ab9281a3 on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

hgrecco commented 9 years ago

Strange. Can you try the same program with the NI backend to compare. Also, is it possible that the linux-gpib defaults for read_termination, etc are not the same that the ones for VISA?

bessman commented 9 years ago

linux-gpib terminates both reads and writes by asserting the EOI line. As of e3a9e58 I check for this event when reading, but instr.query() still stops reading prematurely. If I use a short time delay with instr.query(message, 0.1) it works as expected. I still don't understand why; instr.write(message); instr.read() does not exhibit the same behavior so it doesn't seem to be time dependent.

I'll test with NI-488.2 and NI-VISA shortly.

bessman commented 9 years ago

Huh. instr.query_ascii_values('*IDN?') actually behaves the same way with NI-VISA. Odd, I feel as though I should have noticed that before. Looking at the code, I don't see how it could possibly work with text input though, since it tries to do float() on it.

instr.query() works properly with NI-VISA though.

On a related note, NI-488.2 and linux-gpib do not play very nice together.

hgrecco commented 9 years ago

query_ascii_values is intended to be used for a numerical values (like a stream of data). So do not worry about that.

bessman commented 9 years ago

Aha! It is indeed a time related issue: Session._read() returned too early because the timout wasn't set correctly. I checked for the timeout value with the linux-gpib function ibask, which returns the value of the constant ibtmo. Turns out, ibtmo is a value between 0 and 17, which corresponds to the actual timeout via a lookup table: http://linux-gpib.sourceforge.net/doc_html/r2198.html

What happened was ibask returned 14, which is supposed to correspond to 30 seconds. However, I just passed that value directly to Session.get_attribute which assumed it was the actual timeout in milliseconds, so the timeout was set to 14 ms. Session._read then predictably timed out before the read was finished.

Now, for the actual implementation of this I used a dict for the timout getter and an enormous if switch for the setter. Not very elegant, I'm open to suggestions for improvements there.

hgrecco commented 9 years ago

A lookup table is fine (and needed). I guess you can have just a tuple (Use capital letters for constants) TIMETABLE = (0, 1e-2, 3e-2 ...)

In the getter you just do:

return TIMETABLE[self.interface.ask(3)]

in the setter

self.interface.timeout(bisect(TIMETABLE, value))

you need to add from bisect import bisect

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.96%) to 23.91% when pulling 51d710c8a706b550bbf91f81413debc94f34d77f on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.96%) to 23.91% when pulling 51d710c8a706b550bbf91f81413debc94f34d77f on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

bessman commented 9 years ago

OK, with that we have most of the basic functionality nailed down. Next on the list of things that don't work: instr.close() throws the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.6/site-packages/pyvisa/resources/resource.py", line 199, in close
    self.before_close()
  File "/usr/lib/python2.6/site-packages/pyvisa/resources/gpib.py", line 106, in before_close
    self.__switch_events_off()
  File "/usr/lib/python2.6/site-packages/pyvisa/resources/gpib.py", line 110, in __switch_events_off
    self.visalib.disable_event(self.session, constants.VI_ALL_ENABLED_EVENTS, constants.VI_ALL_MECH)
  File "/usr/lib/python2.6/site-packages/pyvisa/highlevel.py", line 448, in disable_event
    raise NotImplementedError
NotImplementedError
coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.51%) to 24.36% when pulling 3d2308acbbc301e2a26aafede26f4f165c50d390 on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

bessman commented 9 years ago

The error looks like it's coming from pyvisa itself rather than pyvisa-py, but since the NI backend works I assume it fails because it needs one of the attributes which I have yet to implement. Problem is, how do I know which one(s) is/are needed? Are the VISA attributes documented anywhere, i.e. what is supposed to happen when I call _get_attribute or _set_attribute?

hgrecco commented 9 years ago

Yes! You need to implement enable and disable event in highlevel. Right now you can just have a dummy thing like this:

https://github.com/hgrecco/pyvisa-sim/blob/master/pyvisa-sim/highlevel.py#L280

I think it will be great to commit you work, release a new version of PyVISA-py and let people play with it to start finding edge cases.

bessman commented 9 years ago

I discovered that the Gpib class from linux-gpib has no close() function, so I had to hack together one. It uses a regular expression to get the device handle which was used to initialize the Gpib object, so I had to import a function from re. Is that OK?

I also went ahead and implemented a couple of attributes, based on a National Instruments programming reference I found online here: http://www.ni.com/pdf/manuals/370132c.pdf I'm not sure if that's useful at all, but it seemed straightforward enough so I did it anyway.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.42%) to 24.45% when pulling 14de693271d941d68afea8ef44929230db2138e6 on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.42%) to 24.45% when pulling 14de693271d941d68afea8ef44929230db2138e6 on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

hgrecco commented 9 years ago

Regarding the attributes, the goal is to make pyvisa-py a replacement for the NI-VISA backend. So implementing all these attributes is a great step in the right direction. Good work!. Having said this, I would also like to say that we should NOT wait for a full implementation in order to release. I rather have something out there in the wild even if it is incomplete than nothing. This philosophy has already paid off, we decided to release pyvisa-py without GPIB support and we are about to have thanks to your interest.

Regarding the re module. I have not problem about importing it, but I do not like the idea of parsing the repr for such task. Is this not the same handle which is returned by gpib.dev in after_parsing. If that is the case, just store it as instance attribute.

bessman commented 9 years ago

Like so?

I had a thought about the current timeout implementation. Say that a user requests a timeout of 5 seconds. Right now, this will be silently rounded up to 10 seconds. Should we give the user some indication that this is happening, perhaps in the form of a warning?

hgrecco commented 9 years ago

That should work.

Regarding the timeout, it is a good question. In any case, we should think about doing this in PyVISA (not here) because NI-VISA will have the same problem.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.45%) to 24.42% when pulling a4c784c74806c52113bc3d18661c6040ae09483e on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.45%) to 24.42% when pulling a4c784c74806c52113bc3d18661c6040ae09483e on bessman:gpib into 44b92f1f8f5d03df4aabe087c39a8a652b56ef20 on hgrecco:master.

bessman commented 9 years ago

I've given it a go with some of my other instruments, and everything seems to be in working order! If you are comfortable with merging, then so am I.

hgrecco commented 9 years ago

I will!