pyvisa / pyvisa-py

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

TCPIP SOCKET improvements #115

Closed skrchnavy closed 6 years ago

skrchnavy commented 6 years ago
skrchnavy commented 6 years ago

@MatthieuDartiailh please review (I can't change reviewers).

bors[bot] commented 6 years ago

:v: skrchnavy can now approve this pull request

MatthieuDartiailh commented 6 years ago

By the way mentioning me in the PR also makes it easy to find, so you can keep doing this while waiting for hgrecco to add you.

skrchnavy commented 6 years ago

The part at line 379 is something hat shall be considered. This is good for handling chunks or binary data when buffer not big enough. When termchar presented, in normal situation I don't expect further data shall be processed (using standard query). Special case could be in case of using send and read instead of query, when called 2 times send and then 2 times query.

skrchnavy commented 6 years ago

this could be a fix of #101

MatthieuDartiailh commented 6 years ago

Nice refactoring !

skrchnavy commented 6 years ago

@MatthieuDartiailh I have added handling of timeout in connect (#88). Have to test with py2.7. Also open_timeout=0 have to be handled better way (to analyse ni-visa way)

skrchnavy commented 6 years ago

There are few improvements possible in processing return codes from connect_ex but it is difficult to identify errno codes for all systems, so for now I see implementation sufficient. Select waits 100ms min and it is usually enough to connect to a resource on LAN.

Merging bors r+

bors[bot] commented 6 years ago

Build succeeded

MatthieuDartiailh commented 6 years ago

Just one thing I missed and that need to be fixed: the _pending_buffer defined at https://github.com/pyvisa/pyvisa-py/blob/master/pyvisa-py/tcpip.py#L327 should not be a class variable since otherwise it will be shared by all instances, which not what we want at all ! this attribute should be created in __init__

skrchnavy commented 6 years ago

I was thinking about it, and not only pending_buffer but also other variables. Fortunately not used in code. By mu opinion, intention was not to implement __init__ in all sub-classes and call super and have variables.

MatthieuDartiailh commented 6 years ago

There is a number of class variables that are indeed perfectly fine because they are simply used to alter some behaviors easily on subclasses. However pending_buffer HAS to be an instance variable as otherwise it would be shared by alll instances of TCPIPSocket which is not desirable ! So either we initialize it in init or on first use (but i do not favor this solution).

skrchnavy commented 6 years ago

I agree, I am cleaning such variables in other features when I am able to test them. For pending buffer I have to find such device (function) that is able to produce more data then are consumed in read or simulate such device (pending buffer is in action).

skrchnavy commented 6 years ago

@MatthieuDartiailh please check #116 for implementation of pending buffer per session.

MathieuLeocmach commented 6 years ago

Thanks a lot for these feature improvements.

With the version one can get with pip (0.2) I was experiencing infinite waiting when reading from a TCPIP socket that transmit binary data without termination character. I had identified a timeout problem in pyvisa-py.tcpip.TCPIPSocketSession.read, but I was unable to go further. Taking the bleeding edge from github solved the problem.

MatthieuDartiailh commented 6 years ago

Happy to know this works for you. Hopefully we will be able to make a new release soon.