Merge #435

435: Add `chunk_size` and `data_length` to `read/query_binary_values` r=MatthieuDartiailh a=MatthieuDartiailh

This address a couple of issues and is based on #430 :
- `chunk_size` appears to be able to fix the issue seen in #431, even if at the time being the underlying issue is not clear but is likely outside of PyVISA (whether it is a VISA implementation issue or an instrument specific is unclear)
- `data_length` fills a gap by providing a way to get in a controlled manner binary data from instrument that use a protocol that do not report how many bytes will be transferred (such as the Keithley 2000 which is commonly used with PyVISA). **I still have some work to do on the utility functions to handle this properly**

The documentation has been updated to reflect those changes.  The changelog must be updated before merging. @bjaraujo if you can test and review, I would greatly appreciate it. 

Co-authored-by: MatthieuDartiailh <marul@laposte.net>
Co-authored-by: Matthieu Dartiailh <marul@laposte.net>

CHANGES

@@ -4,6 +4,11 @@ PyVISA Changelog
 1.10 (unreleased)
 -----------------
 
+- Change the returned data_length for IEEE block of undefined size to 0 PR #435
+- Add chunk_size and data_length keyword argument to read/query_binary_values PR #435
+- Make the ordering of the visa library deterministic PR #399
+- Properly close pipes when looking for a shared libary path on linux #380
+- Fixing missing argument for USBInstrument.usb_control_out PR #353
 - avoid attempting to close already closed resources in del PR #424
 - add a list_opened_resources method to the ResourceManager PR #415
 - use privately stored resource name in Resource class rather than relying on
@@ -14,7 +19,7 @@ PyVISA Changelog
 - properly close pipes when looking for a shared libary path on linux #380
 - fixing missing argument for USBInstrument.usb_control_out PR #353
 - usb_control_out -> control_out. warnings for deprecated usb_control_out PR #353
-- added new function log_to_stream() PR #363
+- Added new function log_to_stream() PR #363
 - Made all enumerations of the `constants` module unique.
   Fixed duplicate enums in StatusCode PR #371
 - Use ni backend when specifying a file in open_visa_library PR #373

docs/source/introduction/rvalues.rst

@@ -102,6 +102,22 @@ you get from your instrument (plus the header). If it is so, then you can
 safely pass ``expect_termination=False``, and PyVISA will not look for a
 termination character at the end of the message.
 
+If you can read without any problem from your instrument, but cannot retrieve
+the full message when using this method (VI_ERROR_CONN_LOST,
+VI_ERROR_INV_SETUP, or Python simply crashes), try passing different values for
+``chunk_size``(the default is 20*1024). The underlying mechanism for this issue
+is not clear but changing ``chunk_size`` has been used to work around it. Note
+that using  larger chunk sizes for large transfer may result in a speed up of
+the transfer.
+
+In some cases, the instrument may use a protocol that does not indicate how
+many bytes will be transferred. The Keithley 2000 for example always return the
+full buffer whose size is reported by the 'trace:points?' command. Since a
+binary block may contain the termination character, PyVISA need to know how
+many bytes to expect. For those case, you can pass the expected number of
+points using the ``data_points`` keyword argument. The number of bytes will be
+inferred from the datatype of the block.
+
 
 Writing ASCII values
 --------------------

pyvisa/resources/messagebased.py

@@ -15,6 +15,7 @@
                         absolute_import)
 
 import contextlib
+import struct
 import time
 import warnings
 
@@ -450,7 +451,8 @@ def read_ascii_values(self, converter='f', separator=',', container=list,
 
     def read_binary_values(self, datatype='f', is_big_endian=False,
                            container=list, header_fmt='ieee',
-                           expect_termination=True):
+                           expect_termination=True, data_points=0,
+                           chunk_size=None):
         """Read values from the device in binary format returning an iterable
         of values.
 
@@ -465,39 +467,68 @@ def read_binary_values(self, datatype='f', is_big_endian=False,
                                    the binary values block does not account
                                    for the final termination character (the
                                    read termination)
+        :param data_points: Number of points expected in the block. This is
+                            used only if the instrument does not report it
+                            itself. This will be converted in a number of bytes
+                            based on the datatype.
+        :param chunk_size: Size of the chunks to read from the device. Using
+                           larger chunks may be faster for large amount of
+                           data.
         :returns: the answer from the device.
         :rtype: type(container)
 
         """
-        block = self._read_raw()
-        expected_length = 0
+        block = self._read_raw(chunk_size)
 
         if header_fmt == 'ieee':
             offset, data_length = util.parse_ieee_block_header(block)
-            expected_length = offset + data_length
+
+            # Allow to support instrument such as the Keithley 2000 that do not
+            # report the length of the block
+            data_length = data_length or data_points*struct.calcsize(datatype)
+
         elif header_fmt == 'hp':
             offset, data_length = util.parse_hp_block_header(block,
                                                              is_big_endian)
-            expected_length = offset + data_length
+        elif header == 'empty':
+            offset = 0
+        else:
+            raise ValueError("Invalid header format. Valid options are 'ieee',"
+                             " 'empty', 'hp'")
+
+        expected_length = offset + data_length
 
         if expect_termination and self._read_termination is not None:
             expected_length += len(self._read_termination)
 
-        while len(block) < expected_length:
-            block.extend(self._read_raw())
+        # Read all the data if we know what to expect.
+        if data_length != 0:
+            block.extend(self.read_bytes(expected_length - len(block),
+                                         chunk_size=chunk_size))
+        else:
+            # Backward compatibility. This is dangerous and should probably be
+            # removed
+            msg = ('Reading binary data without a known length is error prone,'
+                   ' and should be avoided. This capability will will be'
+                   ' removed in future versions.\n'
+                   'If the instrument does not report the length of the block '
+                   'as part of the transfer, it may be because the size is '
+                   'fixed or can be accessed from the instrumentin using a '
+                   'specific command. You should find the expected number of '
+                   'bytes and pass it using the `data_length` keyword.')
+            warnings.warn(msg, FutureWarning)
+            # Do not keep reading since we may have already read everything
+
+            # Set the datalength to None to infer it from the block length
+            data_length = None
+
 
         try:
-            if header_fmt == 'ieee':
-                return util.from_ieee_block(block, datatype, is_big_endian,
-                                            container)
-            elif header_fmt == 'hp':
-                return util.from_hp_block(block, datatype, is_big_endian,
+            # Do not reparse the headers since it was already done and since
+            # this allows for custom data length
+            return util.from_binary_block(block, offset, data_length,
+                                          datatype, is_big_endian,
                                           container)
-            elif header_fmt == 'empty':
-                return util.from_binary_block(block, 0, None, datatype,
-                                              is_big_endian, container)
-            else:
-                raise ValueError('Unsupported binary block format.')
         except ValueError as e:
             raise errors.InvalidBinaryFormat(e.args)
 
@@ -630,7 +661,8 @@ def query_ascii_values(self, message, converter='f', separator=',',
 
     def query_binary_values(self, message, datatype='f', is_big_endian=False,
                             container=list, delay=None, header_fmt='ieee',
-                            expect_termination=True):
+                            expect_termination=True, data_points=0,
+                            chunk_size=None):
         """Query the device for values in binary format returning an iterable
         of values.
 
@@ -646,6 +678,13 @@ def query_binary_values(self, message, datatype='f', is_big_endian=False,
                                    the binary values block does not account
                                    for the final termination character (the
                                    read termination)
+        :param data_points: Number of points expected in the block. This is
+                            used only if the instrument does not report it
+                            itself. This will be converted in a number of bytes
+                            based on the datatype.
+        :param chunk_size: Size of the chunks to read from the device. Using
+                           larger chunks may be faster for large amount of
+                           data.
         :returns: the answer from the device.
         :rtype: list
         """
@@ -660,7 +699,8 @@ def query_binary_values(self, message, datatype='f', is_big_endian=False,
             time.sleep(delay)
 
         return self.read_binary_values(datatype, is_big_endian, container,
-                                       header_fmt, expect_termination)
+                                       header_fmt, expect_termination,
+                                       data_points, chunk_size)
 
     def ask_for_values(self, message, fmt=None, delay=None):
         """A combination of write(message) and read_values()
@@ -697,7 +737,6 @@ def stb(self):
 
     def read_stb(self):
         """Service request status register.
-
         """
         value, retcode = self.visalib.read_stb(self.session)
         return value

pyvisa/util.py

@@ -318,6 +318,9 @@ def parse_ieee_block_header(block):
     Indefinite Length Arbitrary Block:
     #0<data>
 
+    In this case the data length returned will be 0. The actual length can be
+    deduced from the block and the offset.
+
     :param block: IEEE block.
     :type block: bytes | bytearray
     :return: (offset, data_length)
@@ -342,7 +345,7 @@ def parse_ieee_block_header(block):
     else:
         # #0DATA
         # 012
-        data_length = len(block) - offset - 1
+        data_length = 0
 
     return offset, data_length
 
@@ -388,7 +391,7 @@ def from_ieee_block(block, datatype='f', is_big_endian=False, container=list):
     Indefinite Length Arbitrary Block:
     #0<data>
 
-    :param block: HP block.
+    :param block: IEEE block.
     :type block: bytes | bytearray
     :param datatype: the format string for a single element. See struct module.
     :param is_big_endian: boolean indicating endianess.
@@ -398,6 +401,9 @@ def from_ieee_block(block, datatype='f', is_big_endian=False, container=list):
     """
     offset, data_length = parse_ieee_block_header(block)
 
+    if data_length == 0:
+        data_length = len(block) - offset - 1
+
     if len(block) < offset + data_length:
         raise ValueError("Binary data is incomplete. The header states %d data"
                          " bytes, but %d where received." %
@@ -416,7 +422,7 @@ def from_hp_block(block, datatype='f', is_big_endian=False, container=list):
     The header ia always 4 bytes long.
     The data_length field specifies the size of the data.
 
-    :param block: IEEE block.
+    :param block: HP block.
     :type block: bytes | bytearray
     :param datatype: the format string for a single element. See struct module.
     :param is_big_endian: boolean indicating endianess.