fix terminator when using pyvisa, add / fix tests (#362)

src/instruments/abstract_instruments/comm/visa_communicator.py

@@ -28,13 +28,15 @@ class VisaCommunicator(io.IOBase, AbstractCommunicator):
     def __init__(self, conn):
         super().__init__(self)
 
+        self._terminator = None
+
         version = int(pyvisa.__version__.replace(".", "").ljust(3, "0"))
         # pylint: disable=no-member
         if (version < 160 and isinstance(conn, pyvisa.Instrument)) or (
             version >= 160 and isinstance(conn, pyvisa.Resource)
         ):
             self._conn = conn
-            self._terminator = "\n"
+            self.terminator = "\n"
         else:
             raise TypeError("VisaCommunicator must wrap a VISA Instrument.")
 
@@ -59,6 +61,19 @@ def address(self, newval):
             "Changing addresses of a VISA Instrument " "is not supported."
         )
 
+    @property
+    def read_termination(self):
+        """Get / Set the read termination that is defined in pyvisa."""
+        return self._conn.read_termination
+
+    @read_termination.setter
+    def read_termination(self, newval):
+        if not isinstance(newval, str):
+            raise TypeError(
+                "Read terminator for VisaCommunicator must be specified as a string."
+            )
+        self._conn.read_termination = newval
+
     @property
     def terminator(self):
         """
@@ -72,14 +87,11 @@ def terminator(self):
     def terminator(self, newval):
         if not isinstance(newval, str):
             raise TypeError(
-                "Terminator for VisaCommunicator must be specified "
-                "as a single character string."
-            )
-        if len(newval) > 1:
-            raise ValueError(
-                "Terminator for VisaCommunicator must only be 1 " "character long."
+                "Terminator for VisaCommunicator must be specified as a string."
             )
         self._terminator = newval
+        self.read_termination = newval
+        self.write_termination = newval
 
     @property
     def timeout(self):
@@ -90,6 +102,19 @@ def timeout(self, newval):
         newval = assume_units(newval, u.second).to(u.second).magnitude
         self._conn.timeout = newval
 
+    @property
+    def write_termination(self):
+        """Get / Set the write termination that is defined in pyvisa."""
+        return self._conn.write_termination
+
+    @write_termination.setter
+    def write_termination(self, newval):
+        if not isinstance(newval, str):
+            raise TypeError(
+                "Write terminator for VisaCommunicator must be specified as a string."
+            )
+        self._conn.write_termination = newval
+
     # FILE-LIKE METHODS #
 
     def close(self):
@@ -158,10 +183,10 @@ def _sendcmd(self, msg):
         VISA connections. This function is in turn wrapped by the concrete
         method `AbstractCommunicator.sendcmd` to provide consistent logging
         functionality across all communication layers.
+        Termination characters are automatically added by pyvisa.
 
         :param str msg: The command message to send to the instrument
         """
-        msg += self._terminator
         self.write(msg)
 
     def _query(self, msg, size=-1):
@@ -170,12 +195,12 @@ def _query(self, msg, size=-1):
         VISA connections. This function is in turn wrapped by the concrete
         method `AbstractCommunicator.query` to provide consistent logging
         functionality across all communication layers.
+        Termination characters are automatically added by pyvisa.
 
         :param str msg: The query message to send to the instrument
         :param int size: The number of bytes to read back from the instrument
             response.
         :return: The instrument response to the query
         :rtype: `str`
         """
-        msg += self._terminator
         return self._conn.query(msg)

tests/test_comm/test_visa_communicator.py

@@ -51,32 +51,30 @@ def test_visacomm_address(visa_inst):
     assert err_msg == ("Changing addresses of a VISA Instrument is not supported.")
 
 
+def test_visacomm_read_termination_not_string(visa_inst):
+    """Raise TypeError if read termination is set with non-string character."""
+    comm = VisaCommunicator(visa_inst)
+    with pytest.raises(TypeError):
+        comm.read_termination = 42
+
+
 def test_visacomm_terminator(visa_inst):
-    """Get / Set terminator."""
+    """Get / Set terminator and ensure pyvisa takes the right communicator."""
     comm = VisaCommunicator(visa_inst)
     comm.terminator = "\r"
     assert comm.terminator == "\r"
 
+    assert comm.read_termination == "\r"
+    assert comm.write_termination == "\r"
+
 
 def test_visacomm_terminator_not_string(visa_inst):
     """Raise TypeError if terminator is set with non-string character."""
     comm = VisaCommunicator(visa_inst)
     with pytest.raises(TypeError) as err:
         comm.terminator = 42
     err_msg = err.value.args[0]
-    assert err_msg == (
-        "Terminator for VisaCommunicator must be specified as a single "
-        "character string."
-    )
-
-
-def test_visacomm_terminator_not_single_char(visa_inst):
-    """Raise ValueError if terminator longer than one character."""
-    comm = VisaCommunicator(visa_inst)
-    with pytest.raises(ValueError) as err:
-        comm.terminator = "\r\n"
-    err_msg = err.value.args[0]
-    assert err_msg == ("Terminator for VisaCommunicator must only be 1 character long.")
+    assert err_msg == ("Terminator for VisaCommunicator must be specified as a string.")
 
 
 def test_visacomm_timeout(visa_inst):
@@ -88,6 +86,13 @@ def test_visacomm_timeout(visa_inst):
     assert comm.timeout == u.Quantity(40, u.s)
 
 
+def test_visacomm_write_termination_not_string(visa_inst):
+    """Raise TypeError if write termination is set with non-string character."""
+    comm = VisaCommunicator(visa_inst)
+    with pytest.raises(TypeError):
+        comm.write_termination = 42
+
+
 def test_visacomm_close(visa_inst, mocker):
     """Raise an IOError if comms cannot be closed."""
     io_error_mock = mocker.Mock()
@@ -158,7 +163,7 @@ def test_visacomm_sendcmd(visa_inst, mocker):
     comm = VisaCommunicator(visa_inst)
     msg = "asdf"
     comm._sendcmd(msg)
-    mock_write.assert_called_with(msg + comm.terminator)
+    mock_write.assert_called_with(msg)
 
 
 def test_visacomm_query(visa_inst, mocker):
@@ -167,4 +172,4 @@ def test_visacomm_query(visa_inst, mocker):
     comm = VisaCommunicator(visa_inst)
     msg = "asdf"
     comm._query(msg)
-    mock_query.assert_called_with(msg + comm.terminator)
+    mock_query.assert_called_with(msg)