*Session attributes handling enhancement and TcpSocketSession cleanup by skrchnavy · Pull Request #116 · pyvisa/pyvisa-py

skrchnavy · 2017-11-30T16:44:52Z

Session.attrs table enhanced to support getter and setter methods, not only values
VI_ATTR_TMO_VALUE attribute handling unified to setup also self.timeout as value holder (can be used in sessions and is set to 'python' timeout - seconds as float or None for infinite)
- _get_timeout/_set_timeout methods added and updated for places where external lib timeout is setup.
- GPIBInstrSessiontimeout handling corrected (tested without HW if correct value is calculated in different conditions)
TCPIP Socket Session supports additional attributes (were not implemented)
- Session.attrs table used for implementation with getters and setters
- code cleanup to remove class attributes and use instance attributes
- _pending_buffer is main buffer for handling received data, removed copying from temp buffer (out) and pending buffer

…etSession

skrchnavy · 2017-11-30T19:05:00Z

@MatthieuDartiailh code tested and ready for final review

MatthieuDartiailh · 2017-11-30T21:45:29Z

I may need some time to review this. Hopefully I will merge sometimes next week.

MatthieuDartiailh

Overall this looks really good. I left some minor comments. I will try to make a more thorough review later.

MatthieuDartiailh · 2017-11-30T21:50:11Z

@@ -181,7 +184,8 @@ def __init__(self, resource_manager_session, resource_name, parsed=None, open_ti
        self.attrs = {constants.VI_ATTR_RM_SESSION: resource_manager_session,


Could you expand the above description to detail the possible values in the dictionary ?

Comment added into after_parsing method of Session

MatthieuDartiailh · 2017-11-30T21:51:44Z

        # therefore we must handle the case that the termination character is in the middle of the  block
        # or that the maximum number of bytes is exceeded
-        timeout = self.get_attribute(constants.VI_ATTR_TMO_VALUE)[0] / 1000.
+        timeout, _ = self.get_attribute(constants.VI_ATTR_TMO_VALUE)


Why split this on two lines ?

Unified form of reading timeout in all *Session object

But now I seethis shall be changed to use `self.timeout, it is now properly filled containing python way of timeout (seconds + None) so can be used directly without double conversion.

MatthieuDartiailh · 2017-11-30T21:53:26Z


        ret_status = self._connect()
-        if ret_status != constants.StatusCode.success:
+        if ret_status != SUCCESS:


Why this change ?

This is unification of return codes in this file with previous implementation, when SUCCESS was used, one way shall be followed, so which of these 3 options (see line 25 and 26 of this file):

SUCCES

StatusCode.success

constants.StatusCode.success

it is related other return codes in the file, when second form is used.

…oud double conversion

MatthieuDartiailh · 2017-12-01T08:57:41Z

One stupid question: shouldn't timeout on the TCPIP session be a private attribute rather than a public one ? From my understanding it should only be set through the visa attribute and not directly.

…ethods

skrchnavy · 2017-12-01T10:31:18Z

Further changes / fixes in timeout handling added, some comments applied).

Regarding Session.timeout attribute: it is protected but also 'Session.attrs, Session.interfaceorSession.parsed` are protected. Candidate for writing new issue for this kind of refactoring.

How to cope with the SUCCESS naming convention? spread across Sessions. Which is prefered? (I like StatusCode.success way as it is used for reast of status codes and short enough.)

skrchnavy · 2017-12-01T13:16:47Z

@MatthieuDartiailh #118 have to be resolved (merged or closed) before this issue (in conflict)

skrchnavy · 2017-12-01T14:20:54Z

-                return bytes(out[:count]), constants.StatusCode.success_max_count_read
+            if term_char_en and term_byte in self._pending_buffer:
+                term_byte_index = self._pending_buffer.index(term_byte) + 1
+                if term_byte_index > count:


this check improves handling of case when requested less data then recevied and also terchar received. (#101)

# Conflicts resolved manualy: # pyvisa-py/sessions.py # pyvisa-py/tcpip.py

MatthieuDartiailh

I believe that one comment is wrong but apart from this this is good to go for me.

MatthieuDartiailh · 2017-12-03T17:18:38Z

-TIMETABLE = (0, 1e-2, 3e-2, 1e-1, 3e-1, 1e0, 3e0, 1e1, 3e1, 1e2, 3e2, 1e3, 3e3,
-             1e4, 3e4, 1e5, 3e5, 1e6)
-
+# linux-gpib timeout constants, in milliseconds. See GPIBSession._set_timeout.


Those numbers are in second now no ?

yes, i forgot to change comment properly

MatthieuDartiailh · 2017-12-05T21:29:48Z

Merging. Thanks again @skrchnavy

bors r+

116: *Session attributes handling enhancement and TcpSocketSession cleanup r=MatthieuDartiailh a=skrchnavy * `Session.attrs` table enhanced to support getter and setter methods, not only values * `VI_ATTR_TMO_VALUE` attribute handling unified to setup also self.timeout as value holder (can be used in sessions and is set to 'python' timeout - seconds as float or None for infinite) * `_get_timeout`/`_set_timeout` methods added and updated for places where external lib timeout is setup. * GPIBInstrSessiontimeout handling corrected (tested without HW if correct value is calculated in different conditions) * TCPIP Socket Session supports additional attributes (were not implemented) * `Session.attrs` table used for implementation with getters and setters * code cleanup to remove class attributes and use instance attributes * `_pending_buffer` is main buffer for handling received data, removed copying from temp buffer (`out`) and pending buffer

bors · 2017-12-05T21:34:59Z

Build succeeded

continuous-integration/travis-ci/push

Sveto Krchnavy added 11 commits November 30, 2017 12:06

support for getters and setters (as tupple) into Session.attrs

452a683

updated handling of timeout in *Session objects

b998891

fixed set_attribute for attributes withtou setter

01d7807

forgotten import in gpib

7968301

attributes implemented using defined getters and setters in TCPIPSock…

82a90c7

…etSession

code cleanup to unify status codes and remove unused class variables

ed5379a

fixed write timeout in serial

6bd719d

import in serial.py simplified

48cf334

class variables becomes instance variables

ecacc0f

pending buffer is used as main buffer for session

2a552c0

processing of case, when expected less data then position of termchar

4123d1e

skrchnavy changed the title ~~Session attributes handling enhancement~~ Session attributes handling enhancement and TcpSocketSession cleanup Nov 30, 2017

skrchnavy mentioned this pull request Nov 30, 2017

TCPIP SOCKET improvements #115

Merged

MatthieuDartiailh reviewed Nov 30, 2017

View reviewed changes

Sveto Krchnavy added 2 commits December 1, 2017 09:53

self.timeout used in sessions instead of reading visa attribute to av…

8c916c5

…oud double conversion

simplified usege of attributes constants

18971d4

Sveto Krchnavy added 3 commits December 1, 2017 10:57

TODO unification and code cleanup

e193148

fixed set/get timeout handling in serial and usb and unification of m…

ad70cad

…ethods

using self.timeout in GPIBSession in get and set timeout methods

3593f9d

cosmetic changes in code to unify sources

1f9092e

skrchnavy changed the title ~~Session attributes handling enhancement and TcpSocketSession cleanup~~ *Session attributes handling enhancement and TcpSocketSession cleanup Dec 1, 2017

Sveto Krchnavy added 2 commits December 1, 2017 12:28

added documentation haw to register attribute handler

5c7185b

improved comment for registration of attributes

8ecfbb9

skrchnavy mentioned this pull request Dec 1, 2017

Unified StatusCode usage #118

Merged

skrchnavy commented Dec 1, 2017

View reviewed changes

skrchnavy mentioned this pull request Dec 1, 2017

Problem with TCPIP read() when more than "chunk_length" is returned #101

Open

Merge branch 'master' into session_attributes_handling

a70dfd4

# Conflicts resolved manualy: # pyvisa-py/sessions.py # pyvisa-py/tcpip.py

MatthieuDartiailh requested changes Dec 3, 2017

View reviewed changes

comment to timetable corrected

1e05128

MatthieuDartiailh approved these changes Dec 5, 2017

View reviewed changes

bors Bot merged commit 1e05128 into pyvisa:master Dec 5, 2017

skrchnavy deleted the session_attributes_handling branch December 6, 2017 08:47

skrchnavy mentioned this pull request Dec 7, 2017

query_binary_values on TCPIP-Socket stops after encountering termination character #102

Open

		@@ -181,7 +184,8 @@ def __init__(self, resource_manager_session, resource_name, parsed=None, open_ti
		self.attrs = {constants.VI_ATTR_RM_SESSION: resource_manager_session,

Uh oh!

Conversation

skrchnavy commented Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skrchnavy commented Nov 30, 2017

Uh oh!

MatthieuDartiailh commented Nov 30, 2017

Uh oh!

MatthieuDartiailh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skrchnavy Dec 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthieuDartiailh commented Dec 1, 2017

Uh oh!

skrchnavy commented Dec 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skrchnavy commented Dec 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthieuDartiailh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthieuDartiailh commented Dec 5, 2017

Uh oh!

bors Bot commented Dec 5, 2017

Build succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

skrchnavy commented Nov 30, 2017 •

edited

Loading

skrchnavy Dec 1, 2017 •

edited

Loading

skrchnavy commented Dec 1, 2017 •

edited

Loading