pyvisa / pyvisa-py

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

TCPIP INSTR Session / RPC timeout handling improvements #120

Closed skrchnavy closed 6 years ago

skrchnavy commented 6 years ago

Related to #88

MatthieuDartiailh commented 6 years ago

Is this ready for review ?

skrchnavy commented 6 years ago

Ready for review. But I would prefer to have someone to test it with real device. I had access in the time of development to one device where I tested also fragmentation but only for text data.

Btw. rpc part requires some improvement in the future (rpc.py is with small modifications used accross projects I didn;t find any standalone module that could be reused).

MatthieuDartiailh commented 6 years ago

I will try to review this this week and perhaps run some tests.

MatthieuDartiailh commented 6 years ago

I just did some basic testing and it looks like there is an issue with termination character.

(pyvisa-test) D:\Users\hqc\Documents\Madar\pyvisa>python
Python 3.6.2 |Continuum Analytics, Inc.| (default, Jul 20 2017, 12:30:02) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import visa
>>> rm = visa.ResourceManager('@py')
>>> psg = rm.open_resource('TCPIP::192.168.0.7::INSTR')
>>> psg.query('*IDN?')
'Agilent Technologies, E8257D, MY51111912, C.06.18\n'
>>> psg.read_termination = '\n'
>>> psg.query('*IDN?')
D:\Users\hqc\Documents\Madar\pyvisa\pyvisa\resources\messagebased.py:521: UserWarning: read string doesn't end with termination characters
  return self.read()
'Agilent Technologies, E8257D, MY5'
>>> psg.query('*IDN?')
''
>>> psg.query('*IDN?')
'Agilent Technologies, E8257D, MY5'
>>> psg.close()
>>> rm2 = visa.ResourceManager()
>>> psg = rm2.open_resource('TCPIP::192.168.0.7::INSTR')
>>> psg.query('*IDN?')
'Agilent Technologies, E8257D, MY51111912, C.06.18\n'
>>> psg.read_termination = '\n'
>>> psg.query('*IDN?')
'Agilent Technologies, E8257D, MY51111912, C.06.18'

With the NI backend when specifying the read_termination I get the expected message no warning and I can read twice in a row which is not the case with the py backend. I am not sure this is related to your changes or not, but it is worth fixing. I will keep testing.

skrchnavy commented 6 years ago

@MatthieuDartiailh I don't have access to device with TCPIP_INSTR protocol so I checked only the code. I don't see any suspicious change that could trim response. I checked https://github.com/python-ivi/python-vxi11/tree/master/vxi11 (files rpc and vxi11) and there are some differences, so there could be real fixes of the issue you have reported.

Are you able to check same using code in master branch to be sure mine changes affects or not the behaviour?

MatthieuDartiailh commented 6 years ago

I will try. Not sure I will have time before Christmas howevee.

MatthieuDartiailh commented 6 years ago

I checked on master and indeed the bug is there too.

skrchnavy commented 6 years ago

I analysed the code and there is Packer nad Unpacker in vxi11.py and termchar is packed (pack_device_read_parms) but in response it is not read (unpack_device_read_resp). in https://github.com/python-ivi/python-vxi11/blob/master/vxi11/vxi11.py#L696 there is slightly different implementation of read. @alexforencich is your implementation of python-ivi vxi11 protocol able to handle termchar correctly?

MatthieuDartiailh commented 6 years ago

I had no time yet to look into this in more details. And I don't know when I will, but in the meantime I got the rights to invite you @skrchnavy to join the organization if you are still interested.

MatthieuDartiailh commented 6 years ago

I rebased on #126 and did some additional clean up. Could you add support for proper timeout in instr session the way you did for SOCKET ? I will take care of testing.

MatthieuDartiailh commented 6 years ago

Implemented the last missing parts and tested locally. This is working great !

bors r+

bors[bot] commented 6 years ago

Build succeeded