Repository: pyvisa/pyvisa

Issue:
Reading binary data with no data crash (IEEE)
When reading binary values that return #10 which is valid IEEE header we get an error as if the lenght of data could not be determined. This should not happen.


Discussion:
MatthieuDartiailh: bors try
bors[bot]: ## try

Build failed:
  * [Check code formatting](https://github.com/pyvisa/pyvisa/runs/2465689565)
codecov-commenter: # [Codecov](https://codecov.io/gh/pyvisa/pyvisa/pull/581?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa) Report
> Merging [#581](https://codecov.io/gh/pyvisa/pyvisa/pull/581?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa) (1085f2c) into [main](https://codecov.io/gh/pyvisa/pyvisa/commit/9cab3576fb6b7c4aa18538d4337c67f8d971fcc7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa) (9cab357) will **decrease** coverage by `0.02%`.
> The diff coverage is `33.33%`.

> :exclamation: Current head 1085f2c differs from pull request most recent head 69792eb. Consider uploading reports for the commit 69792eb to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/pyvisa/pyvisa/pull/581/graphs/tree.svg?width=650&height=150&src=pr&token=ikDrmaC7B2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa)](https://codecov.io/gh/pyvisa/pyvisa/pull/581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa)

```diff
@@            Coverage Diff             @@
##             main     #581      +/-   ##
==========================================
- Coverage   78.55%   78.52%   -0.03%     
==========================================
  Files          27       27              
  Lines        5338     5337       -1     
  Branches      428      427       -1     
==========================================
- Hits         4193     4191       -2     
- Misses       1123     1124       +1     
  Partials       22       22              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `78.47% <33.33%> (-0.03%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pyvisa/pyvisa/pull/581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa) | Coverage Δ | |
|---|---|---|
| [pyvisa/resources/messagebased.py](https://codecov.io/gh/pyvisa/pyvisa/pull/581/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa#diff-cHl2aXNhL3Jlc291cmNlcy9tZXNzYWdlYmFzZWQucHk=) | `28.15% <0.00%> (-0.14%)` | :arrow_down: |
| [pyvisa/util.py](https://codecov.io/gh/pyvisa/pyvisa/pull/581/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa#diff-cHl2aXNhL3V0aWwucHk=) | `93.75% <100.00%> (-0.05%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pyvisa/pyvisa/pull/581?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pyvisa/pyvisa/pull/581?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa). Last update [9cab357...69792eb](https://codecov.io/gh/pyvisa/pyvisa/pull/581?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa).

MatthieuDartiailh: bors try
bors[bot]: ## try

Build failed:
  * [pyvisa.keysight-assisted](https://github.com/pyvisa/pyvisa/runs/2466153085)
MatthieuDartiailh: bors try
bors[bot]: ## try

Timed out.
MatthieuDartiailh: Tests are passing locally so let's try again

bors try
bors[bot]: ## try

Build failed:
  * [Check code formatting](https://github.com/pyvisa/pyvisa/runs/2503293985)

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index f4cea12..08049c7 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -65,7 +65,7 @@ jobs:
       - name: Test with pytest
         run: |
           pip install pytest-cov
-          pytest --pyargs pyvisa --cov pyvisa --cov-report xml -v
+          python -X dev -m pytest --pyargs pyvisa --cov pyvisa --cov-report xml -v
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v1
         with:
diff --git a/.gitignore b/.gitignore
index 42e53bb..0d690f7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,3 +19,4 @@ _test/
 htmlcov/
 .mypy_cache/
 pip-wheel-metadata/
+.pytest_cache
diff --git a/CHANGES b/CHANGES
index c50943c..fdf67af 100644
--- a/CHANGES
+++ b/CHANGES
@@ -5,7 +5,16 @@ PyVISA Changelog
 -------------------
 
 - fix collection of debug info on the ctypes wrapper PR #598
-
+- allow an IEEE binary block or an HP block to be empty PR #581
+  This is more correct and can affect real instruments. However this introduces
+  a minor backward incompatibility when parsing IEEE header. The data length for
+  #0 is now reported to be -1 instead of 0 since 0 corresponds to #10.
+  This changes should affect a minority of user and does not change the behavior for
+  instrument returning #0 (only the value of data_length as returned by
+  parse_ieee_block_header is affected and functions using it have been adapted).
+- allow trailing separators in ascii blocks PR #581
+  Numpy style extraction already handled this case correctly but it was not so
+  for the builtin parser.
 
 1.11.3 (07-11-2020)
 -------------------
diff --git a/docs/source/introduction/communication.rst b/docs/source/introduction/communication.rst
index f0f60d1..5c6139f 100644
--- a/docs/source/introduction/communication.rst
+++ b/docs/source/introduction/communication.rst
@@ -48,6 +48,7 @@ to filter the instruments discovered by this method. The syntax is described in
 details in |list_resources|. The default value is '?*::INSTR' which means that
 by default only instrument whose resource name ends with '::INSTR' are listed
 (in particular USB RAW resources and TCPIP SOCKET resources are not listed).
+To list all resources present, pass '?*' to ``list_resources``.
 
 In this case, there is a GPIB instrument with instrument number 14, so you ask
 the ``ResourceManager`` to open "'GPIB0::14::INSTR'" and assign the returned
diff --git a/pyvisa/highlevel.py b/pyvisa/highlevel.py
index 045387f..f57c822 100644
--- a/pyvisa/highlevel.py
+++ b/pyvisa/highlevel.py
@@ -2933,7 +2933,7 @@ def open_visa_library(specification: str = "") -> VisaLibraryBase:
 
 
 class ResourceManager(object):
-    """VISA Resource Manager. """
+    """VISA Resource Manager."""
 
     #: Maps (Interface Type, Resource Class) to Python class encapsulating that resource.
     _resource_classes: ClassVar[
diff --git a/pyvisa/resources/messagebased.py b/pyvisa/resources/messagebased.py
index 092886a..4d99913 100644
--- a/pyvisa/resources/messagebased.py
+++ b/pyvisa/resources/messagebased.py
@@ -20,7 +20,7 @@ from .resource import Resource
 
 
 class ControlRenMixin(object):
-    """Common control_ren method of some messaged based resources. """
+    """Common control_ren method of some messaged based resources."""
 
     #: Will be present when used as a mixin with Resource
     visalib: VisaLibraryBase
@@ -537,7 +537,7 @@ class MessageBasedResource(Resource):
         container: Union[Type, Callable[[Iterable], Sequence]] = list,
         header_fmt: util.BINARY_HEADERS = "ieee",
         expect_termination: bool = True,
-        data_points: int = 0,
+        data_points: int = -1,
         chunk_size: Optional[int] = None,
     ) -> Sequence[Union[int, float]]:
         """Read values from the device in binary format returning an iterable
@@ -581,7 +581,7 @@ class MessageBasedResource(Resource):
             offset, data_length = util.parse_hp_block_header(block, is_big_endian)
         elif header_fmt == "empty":
             offset = 0
-            data_length = 0
+            data_length = -1
         else:
             raise ValueError(
                 "Invalid header format. Valid options are 'ieee'," " 'empty', 'hp'"
@@ -589,7 +589,9 @@ class MessageBasedResource(Resource):
 
         # 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)
+        data_length = (
+            data_length if data_length >= 0 else data_points * struct.calcsize(datatype)
+        )
 
         expected_length = offset + data_length
 
@@ -597,10 +599,12 @@ class MessageBasedResource(Resource):
             expected_length += len(self._read_termination)
 
         # Read all the data if we know what to expect.
-        if data_length != 0:
+        if data_length > 0:
             block.extend(
                 self.read_bytes(expected_length - len(block), chunk_size=chunk_size)
             )
+        elif data_length == 0:
+            pass
         else:
             raise ValueError(
                 "The length of the data to receive could not be "
diff --git a/pyvisa/resources/usb.py b/pyvisa/resources/usb.py
index 3d538d7..814af0b 100644
--- a/pyvisa/resources/usb.py
+++ b/pyvisa/resources/usb.py
@@ -118,7 +118,7 @@ class USBInstrument(ControlRenMixin, USBCommon):
         index : int
             wIndex parameter of the setup stage of a USB control transfer.
             This is usually the index of the interface or endpoint.
-        data : str
+        data : bytes
             The data buffer that sends the data in the optional data stage of
             the control transfer.
 
diff --git a/pyvisa/shell.py b/pyvisa/shell.py
index 3f7e3c8..4712921 100644
--- a/pyvisa/shell.py
+++ b/pyvisa/shell.py
@@ -122,7 +122,7 @@ class VisaShell(cmd.Cmd):
             self.prompt = self.default_prompt
 
     def do_query(self, args):
-        """Query resource in use: query *IDN? """
+        """Query resource in use: query *IDN?"""
 
         if not self.current:
             print('There are no resources in use. Use the command "open".')
@@ -146,7 +146,7 @@ class VisaShell(cmd.Cmd):
             print(e)
 
     def do_write(self, args):
-        """Send to the resource in use: send *IDN? """
+        """Send to the resource in use: send *IDN?"""
 
         if not self.current:
             print('There are no resources in use. Use the command "open".')
diff --git a/pyvisa/thirdparty/prettytable.py b/pyvisa/thirdparty/prettytable.py
index a21beaa..214229c 100644
--- a/pyvisa/thirdparty/prettytable.py
+++ b/pyvisa/thirdparty/prettytable.py
@@ -938,7 +938,7 @@ class PrettyTable(object):
 
     @property
     def oldsortslice(self):
-        """ oldsortslice - Slice rows before sorting in the "old style" """
+        """oldsortslice - Slice rows before sorting in the "old style" """
         return self._oldsortslice
 
     @oldsortslice.setter
diff --git a/pyvisa/util.py b/pyvisa/util.py
index ab8a699..c964920 100644
--- a/pyvisa/util.py
+++ b/pyvisa/util.py
@@ -346,6 +346,8 @@ def from_ascii_block(
     data: Iterable[str]
     if isinstance(separator, str):
         data = ascii_data.split(separator)
+        if not data[-1]:
+            data = data[:-1]
     else:
         data = separator(ascii_data)
 
@@ -467,7 +469,7 @@ def parse_ieee_block_header(
     else:
         # #0DATA
         # 012
-        data_length = 0
+        data_length = -1
 
     return offset, data_length
 
@@ -577,7 +579,7 @@ def from_ieee_block(
 
     # If the data length is not reported takes all the data and do not make
     # any assumption about the termination character
-    if data_length == 0:
+    if data_length == -1:
         data_length = len(block) - offset
 
     if len(block) < offset + data_length:
@@ -627,11 +629,6 @@ def from_hp_block(
     """
     offset, data_length = parse_hp_block_header(block, is_big_endian)
 
-    # If the data length is not reported takes all the data and do not make
-    # any assumption about the termination character
-    if data_length == 0:
-        data_length = len(block) - offset
-
     if len(block) < offset + data_length:
         raise ValueError(
             "Binary data is incomplete. The header states %d data"


Test patch:
diff --git a/pyvisa/testsuite/keysight_assisted_tests/messagebased_resource_utils.py b/pyvisa/testsuite/keysight_assisted_tests/messagebased_resource_utils.py
index 53df4a2..6d6f528 100644
--- a/pyvisa/testsuite/keysight_assisted_tests/messagebased_resource_utils.py
+++ b/pyvisa/testsuite/keysight_assisted_tests/messagebased_resource_utils.py
@@ -350,11 +350,13 @@ class MessagebasedResourceTestCase(ResourceTestCase):
         with pytest.raises(ValueError):
             self.instr.write_binary_values("", values, "h", header_fmt="zxz")
 
-    def test_read_ascii_values(self):
+    # Without and with trailing comma
+    @pytest.mark.parametrize("msg", ["1,2,3,4,5", "1,2,3,4,5,"])
+    def test_read_ascii_values(self, msg):
         """Test reading ascii values."""
         # Standard separator
         self.instr.write("RECEIVE")
-        self.instr.write("1,2,3,4,5")
+        self.instr.write(msg)
         self.instr.write("SEND")
         values = self.instr.read_ascii_values()
         assert type(values[0]) is float
@@ -362,7 +364,7 @@ class MessagebasedResourceTestCase(ResourceTestCase):
 
         # Non standard separator and termination
         self.instr.write("RECEIVE")
-        self.instr.write("1;2;3;4;5")
+        self.instr.write(msg.replace(",", ";"))
         tic = time.time()
         values = self.instr.query_ascii_values(
             "SEND", converter="d", separator=";", delay=0.5
@@ -374,7 +376,7 @@ class MessagebasedResourceTestCase(ResourceTestCase):
         # Numpy container
         if np:
             self.instr.write("RECEIVE")
-            self.instr.write("1,2,3,4,5")
+            self.instr.write(msg)
             self.instr.write("SEND")
             values = self.instr.read_ascii_values(container=np.array)
             expected = np.array([1.0, 2.0, 3.0, 4.0, 5.0])
@@ -459,12 +461,10 @@ class MessagebasedResourceTestCase(ResourceTestCase):
     # Not sure how to test this
     @pytest.mark.skip
     def test_handling_malformed_binary(self):
-        """"""
+        """ """
         pass
 
-    @pytest.mark.parametrize(
-        "hfmt, header", zip(("ieee", "hp", "empty"), ("#10", "#A\x00\x00", ""))
-    )
+    @pytest.mark.parametrize("hfmt, header", zip(("ieee", "empty"), ("#0", "")))
     def test_read_binary_values_unreported_length(self, hfmt, header):
         """Test reading binary data."""
         self.instr.read_termination = "\r"
@@ -526,6 +526,38 @@ class MessagebasedResourceTestCase(ResourceTestCase):
                 chunk_size=6,
             )
 
+    def test_read_binary_values_empty(self):
+        """Test reading binary data."""
+        self.instr.write("RECEIVE")
+        self.instr.write("#10")
+        self.instr.write("SEND")
+        new = self.instr.read_binary_values(
+            datatype="h",
+            is_big_endian=False,
+            header_fmt="ieee",
+            expect_termination=True,
+            chunk_size=6,
+        )
+        assert not new
+
+        if np:
+            self.instr.write("RECEIVE")
+            self.instr.write(
+                "#10",
+                termination="\n",
+            )
+            new = self.instr.query_binary_values(
+                "SEND",
+                datatype="h",
+                header_fmt="ieee",
+                is_big_endian=True,
+                expect_termination=False,
+                chunk_size=6,
+                container=np.array if np else list,
+            )
+
+            assert not new.size if np else not new
+
     def test_delay_in_query_ascii(self):
         """Test handling of the delay argument in query_ascii_values."""
         # Test using the instrument wide delay
@@ -560,7 +592,7 @@ class MessagebasedResourceTestCase(ResourceTestCase):
 
     def test_instrument_wide_delay_in_query_binary(self):
         """Test handling delay in query_ascii_values."""
-        header = "#10"
+        header = "#0"
         data = [1, 2, 3328, 3, 4, 5]
         # Test using the instrument wide delay
         self.instr.query_delay = 1.0
@@ -585,7 +617,7 @@ class MessagebasedResourceTestCase(ResourceTestCase):
 
     def test_delay_args_in_query_binary(self):
         """Test handling of the delay argument in query_ascii_values."""
-        header = "#10"
+        header = "#0"
         data = [1, 2, 3328, 3, 4, 5]
         self.instr.query_delay = 0.0
         self.instr.write("RECEIVE")
@@ -610,7 +642,7 @@ class MessagebasedResourceTestCase(ResourceTestCase):
 
     def test_no_delay_args_in_query_binary(self):
         """Test handling of the delay argument in query_ascii_values."""
-        header = "#10"
+        header = "#0"
         data = [1, 2, 3328, 3, 4, 5]
         self.instr.query_delay = 1.0
         self.instr.write("RECEIVE")
diff --git a/pyvisa/testsuite/keysight_assisted_tests/test_resource_manager.py b/pyvisa/testsuite/keysight_assisted_tests/test_resource_manager.py
index a61175c..1193482 100644
--- a/pyvisa/testsuite/keysight_assisted_tests/test_resource_manager.py
+++ b/pyvisa/testsuite/keysight_assisted_tests/test_resource_manager.py
@@ -50,6 +50,8 @@ class TestResourceManager:
             self.rm.session
         assert self.rm.visalib.resource_manager is None
 
+    # This test is flaky
+    @pytest.mark.skip
     def test_cleanup_on_del(self, caplog):
         """Test that deleting the rm does clean the VISA session"""
         # The test seems to assert what it should even though the coverage report
diff --git a/pyvisa/testsuite/keysight_assisted_tests/test_tcpip_resources.py b/pyvisa/testsuite/keysight_assisted_tests/test_tcpip_resources.py
index ab44334..23705b6 100644
--- a/pyvisa/testsuite/keysight_assisted_tests/test_tcpip_resources.py
+++ b/pyvisa/testsuite/keysight_assisted_tests/test_tcpip_resources.py
@@ -103,6 +103,9 @@ class TestTCPIPSocket(MessagebasedResourceTestCase):
             MessagebasedResourceTestCase.test_read_binary_values_unreported_length
         )
     )
+    test_read_binary_values_empty = pytest.mark.xfail(
+        copy_func(MessagebasedResourceTestCase.test_read_binary_values_empty)
+    )
     test_delay_in_query_ascii = pytest.mark.xfail(
         copy_func(MessagebasedResourceTestCase.test_delay_in_query_ascii)
     )
diff --git a/pyvisa/testsuite/test_cmd_line_tools.py b/pyvisa/testsuite/test_cmd_line_tools.py
index a3e61f3..47b463e 100644
--- a/pyvisa/testsuite/test_cmd_line_tools.py
+++ b/pyvisa/testsuite/test_cmd_line_tools.py
@@ -56,10 +56,8 @@ class TestCmdLineTools(BaseTestCase):
         result = run("pyvisa-info", stdout=PIPE, universal_newlines=True)
         details = util.system_details_to_str(util.get_system_details())
         # Path difference can lead to subtle differences in the backends
-        # compare only the first lines.
-        assert (
-            result.stdout.strip().split("\n")[:16] == details.strip().split("\n")[:16]
-        )
+        # and Python path so compare only the first lines.
+        assert result.stdout.strip().split("\n")[:5] == details.strip().split("\n")[:5]
 
     # TODO test backend selection: this is not easy at all to assert
     @require_visa_lib
diff --git a/pyvisa/testsuite/test_util.py b/pyvisa/testsuite/test_util.py
index 8d6953b..f01d596 100644
--- a/pyvisa/testsuite/test_util.py
+++ b/pyvisa/testsuite/test_util.py
@@ -161,7 +161,7 @@ class TestConfigFile(BaseTestCase):
 class TestParser(BaseTestCase):
     def test_parse_binary(self):
         s = (
-            b"#A@\xe2\x8b<@\xe2\x8b<@\xe2\x8b<@\xe2\x8b<@\xde\x8b<@\xde\x8b<@"
+            b"#0@\xe2\x8b<@\xe2\x8b<@\xe2\x8b<@\xe2\x8b<@\xde\x8b<@\xde\x8b<@"
             b"\xde\x8b<@\xde\x8b<@\xe0\x8b<@\xe0\x8b<@\xdc\x8b<@\xde\x8b<@"
             b"\xe2\x8b<@\xe0\x8b<"
         )
@@ -192,6 +192,10 @@ class TestParser(BaseTestCase):
         for a, b in zip(p, e):
             assert a == pytest.approx(b)
 
+        # Test handling zero length block
+        p = util.from_ieee_block(b"#10" + s[2:], datatype="f", is_big_endian=False)
+        assert not p
+
         p = util.from_hp_block(
             b"#A\x0e\x00" + s[2:],
             datatype="f",
@@ -206,7 +210,8 @@ class TestParser(BaseTestCase):
         for fmt in "d":
             msg = "block=%s, fmt=%s"
             msg = msg % ("ascii", fmt)
-            tb = lambda values: util.to_ascii_block(values, fmt, ",")
+            # Test handling the case of a trailing comma
+            tb = lambda values: util.to_ascii_block(values, fmt, ",") + ","
             fb = lambda block, cont: util.from_ascii_block(block, fmt, ",", cont)
             self.round_trip_block_conversion(values, tb, fb, msg)
 
@@ -377,7 +382,7 @@ class TestParser(BaseTestCase):
                 block = block[:2] + b"0" * header_length + block[2 + header_length :]
             else:
                 block = block[:2] + b"\x00\x00\x00\x00" + block[2 + 4 :]
-            assert fb(block, "h", False, list) == values
+            assert not fb(block, "h", False, list)
 
     def test_handling_malformed_binary(self):
         containers = (list, tuple) + ((np.array, np.ndarray) if np else ())