TCPIP SOCKET improvements

[skrchnavy]

• Connect uses open_timeout parameter to be timeouted (opening TCP/IP connection to invalid IP blocks forever #88)
• Refactored read operation with improved timeout handling
• Read Uses bytearray and bytes instead of string - inspired by VXI11 protocol performance enhancement #50
• Calculation of return data in read uses indexes
• Renamed some variables for better readability.

[skrchnavy]

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

[bors]

✌️ skrchnavy can now approve this pull request

[MatthieuDartiailh]

This looks very good. I simply wish we could avoid the duplications in lines 379 and 404 but I have no good alternative to propose. I do not have an instrument handy to test so once you have thoroughly tested this on both 2 and 3, you can merge (just trigger bors, using the same command I use)

bors delegate+

[MatthieuDartiailh]

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]

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]

this could be a fix of #101

[MatthieuDartiailh]

Nice refactoring !

[skrchnavy]

@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]

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]

Build succeeded

• continuous-integration/travis-ci/push

[MatthieuDartiailh]

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]

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]

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]

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]

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

[skrchnavy]

this shall be a fix of #101 , returning only expected bytes

[MathieuLeocmach]

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]

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