Update, document and test SerialData

peas/sensors.py

@@ -83,7 +83,7 @@ def __init__(self, auto_detect=False, *args, **kwargs):
     def _connect_serial(self, port):
         if port is not None:
             self.logger.debug('Attempting to connect to serial port: {}'.format(port))
-            serial_reader = SerialData(port=port, threaded=False)
+            serial_reader = SerialData(port=port)
             self.logger.debug(serial_reader)
 
             try:

pocs/mount/serial.py

@@ -10,7 +10,9 @@
 class AbstractSerialMount(AbstractMount):
 
     def __init__(self, *args, **kwargs):
-        """
+        """Initialize an AbstractSerialMount for the port defined in the config.
+
+        Opens a connection to the serial device, if it is valid.
         """
         super(AbstractSerialMount, self).__init__(*args, **kwargs)
 
@@ -26,6 +28,10 @@ def __init__(self, *args, **kwargs):
             self.serial = rs232.SerialData(port=self._port, baudrate=9600)
         except Exception as err:
             self.serial = None
+            # This won't be triggered because SerialData is created even if the mount port
+            # can't be opened. We can't tell the difference between not currently present
+            # and totally invalid (e.g. com1: on a Linux box, or /dev/tty7 on Windows),
+            # TODO(wtgee): What would you like to do about this?
             raise error.MountNotFound(err)
 
 
@@ -34,14 +40,15 @@ def __init__(self, *args, **kwargs):
 ##################################################################################################
 
     def connect(self):
-        """ Connects to the mount via the serial port (`self._port`)
+        """Connects to the mount via the serial port (`self._port`)
 
         Returns:
-            bool:   Returns the self.is_connected property which checks the actual serial connection.
+            Returns the self.is_connected property (bool) which checks
+            the actual serial connection.
         """
         self.logger.debug('Connecting to mount')
 
-        if self.serial.ser and self.serial.ser.isOpen() is False:
+        if self.serial and not self.serial.is_connected:
             try:
                 self._connect()
             except OSError as err:
@@ -57,7 +64,9 @@ def connect(self):
 
     def disconnect(self):
         self.logger.debug("Closing serial port for mount")
-        self._is_connected = self.serial.disconnect()
+        if self.serial:
+            self.serial.disconnect()
+        self._is_connected = self.serial.is_connected
 
     def set_tracking_rate(self, direction='ra', delta=0.0):
         """Set the tracking rate for the mount
@@ -98,19 +107,20 @@ def set_tracking_rate(self, direction='ra', delta=0.0):
     def write(self, cmd):
         """ Sends a string command to the mount via the serial port.
 
-        First 'translates' the message into the form specific mount can understand using the mount configuration yaml
-        file. This method is most often used from within `query` and may become a private method in the future.
+        First 'translates' the message into the form specific mount can understand using the
+        mount configuration yaml file. This method is most often used from within `query` and
+        may become a private method in the future.
 
         Note:
             This command currently does not support the passing of parameters. See `query` instead.
 
         Args:
-            cmd (str): A command to send to the mount. This should be one of the commands listed in the mount
-                commands yaml file.
+            cmd (str): A command to send to the mount. This should be one of the commands listed
+                in the mount commands yaml file.
         """
         assert self.is_initialized, self.logger.warning('Mount has not been initialized')
 
-        # self.serial.clear_buffer()
+        # self.serial.reset_input_buffer()
 
         # self.logger.debug("Mount Query: {}".format(cmd))
         self.serial.write(cmd)
@@ -229,7 +239,6 @@ def _get_command(self, cmd, params=None):
         return full_command
 
     def _update_status(self):
-        """ """
         self._raw_status = self.query('get_status')
 
         status = dict()

pocs/tests/serial_handlers/__init__.py

@@ -75,6 +75,14 @@ def write(self, data):
             raise serialutil.portNotOpenError
         return 0
 
+    def reset_input_buffer(self):
+        """Remove any accumulated bytes from the device."""
+        pass
+
+    def reset_output_buffer(self):
+        """Remove any accumulated bytes not yet sent to the device."""
+        pass
+
     # --------------------------------------------------------------------------
     # There are a number of methods called by SerialBase that need to be
     # implemented by sub-classes, assuming their calls haven't been blocked

pocs/tests/test_rs232.py

@@ -2,6 +2,7 @@
 import pytest
 import serial
 
+from pocs.utils import error
 from pocs.utils import rs232
 from pocs.utils.config import load_config
 
@@ -11,6 +12,23 @@
 from pocs.tests.serial_handlers import protocol_hooked
 
 
+def test_missing_port():
+    with pytest.raises(ValueError):
+        rs232.SerialData()
+
+
+def test_non_existent_device():
+    """Doesn't complain if it can't find the device."""
+    port = '/dev/tty12345698765'
+    ser = rs232.SerialData(port=port)
+    assert not ser.is_connected
+    assert port == ser.name
+    # Can't connect to that device.
+    with pytest.raises(error.BadSerialConnection):
+        ser.connect()
+    assert not ser.is_connected
+
+
 def test_detect_uninstalled_scheme():
     """If our handlers aren't installed, will detect unknown scheme."""
     with pytest.raises(ValueError):
@@ -32,9 +50,16 @@ def test_detect_bogus_scheme(handler):
         rs232.SerialData(port='bogus://')
 
 
+def test_detect_bogus_scheme(handler):
+    """When our handlers are installed, will still detect unknown scheme."""
+    with pytest.raises(ValueError):
+        rs232.SerialData(port='bogus://')
+
+
 def test_basic_no_op(handler):
     # Confirm we can create the SerialData object.
-    ser = rs232.SerialData(port='no_op://', open_delay=0, threaded=False)
+    ser = rs232.SerialData(port='no_op://', name='a name', open_delay=0)
+    assert ser.name == 'a name'
 
     # Peek inside, it should have a NoOpSerial instance as member ser.
     assert ser.ser
@@ -43,34 +68,51 @@ def test_basic_no_op(handler):
     # Open is automatically called by SerialData.
     assert ser.is_connected
 
-    # no_op handler doesn't do any reading, analogous to /dev/null, which
-    # never produces any output.
-    assert '' == ser.read(retry_delay=0.01, retry_limit=2)
-    assert 0 == ser.write('abcdef')
-
-    # Disconnect from the serial port.
+    # connect() is idempotent.
+    ser.connect()
     assert ser.is_connected
-    ser.disconnect()
-    assert not ser.is_connected
 
-    # Should no longer be able to read or write.
-    with pytest.raises(AssertionError):
-        ser.read(retry_delay=0.01, retry_limit=1)
-    with pytest.raises(AssertionError):
-        ser.write('a')
+    # Several passes of reading, writing, disconnecting and connecting.
+    for _ in range(3):
+        # no_op handler doesn't do any reading, analogous to /dev/null, which
+        # never produces any output.
+        assert '' == ser.read(retry_delay=0.01, retry_limit=2)
+        assert b'' == ser.read_bytes(size=1)
+        assert 0 == ser.write('abcdef')
+        ser.reset_input_buffer()
+
+        # Disconnect from the serial port.
+        assert ser.is_connected
+        ser.disconnect()
+        assert not ser.is_connected
+
+        # Should no longer be able to read or write.
+        with pytest.raises(AssertionError):
+            ser.read(retry_delay=0.01, retry_limit=1)
+        with pytest.raises(AssertionError):
+            ser.read_bytes(size=1)
+        with pytest.raises(AssertionError):
+            ser.write('a')
+        ser.reset_input_buffer()
+
+        # And we should be able to reconnect.
+        assert not ser.is_connected
+        ser.connect()
+        assert ser.is_connected
 
 
 def test_basic_io(handler):
     protocol_buffers.ResetBuffers(b'abc\r\n')
-    ser = rs232.SerialData(port='buffers://', open_delay=0, threaded=False)
+    ser = rs232.SerialData(port='buffers://', open_delay=0.01, retry_delay=0.01,
+                           retry_limit=2)
 
     # Peek inside, it should have a BuffersSerial instance as member ser.
     assert isinstance(ser.ser, protocol_buffers.BuffersSerial)
 
     # Can read one line, "abc\r\n", from the read buffer.
     assert 'abc\r\n' == ser.read(retry_delay=0.1, retry_limit=10)
     # Another read will fail, having exhausted the contents of the read buffer.
-    assert '' == ser.read(retry_delay=0.01, retry_limit=2)
+    assert '' == ser.read()
 
     # Can write to the "device", the handler will accumulate the results.
     assert 5 == ser.write('def\r\n')
@@ -138,7 +180,7 @@ def write(self, data):
 
 def test_hooked_io(handler):
     protocol_hooked.Serial = HookedSerialHandler
-    ser = rs232.SerialData(port='hooked://', open_delay=0, threaded=False)
+    ser = rs232.SerialData(port='hooked://', open_delay=0)
 
     # Peek inside, it should have a PySerial instance as member ser.
     assert ser.ser
@@ -157,6 +199,8 @@ def test_hooked_io(handler):
         else:
             first_line = line
             assert 'message' in line
+        reading = ser.get_reading()
+        assert reading[1] == line
 
     # Can write to the "device" many times.
     line = 'abcdefghijklmnop' * 30