Merge pull request #34 from gerph/replace-queued-data-send-with-immed…

…iate-plus-deferred-delay

Replace the queued communications with deferred delays.

library/display.py

@@ -35,13 +35,11 @@ def __init__(self):
         if CONFIG_DATA["display"]["REVISION"] == "A":
             self.lcd = LcdCommRevA(com_port=CONFIG_DATA['config']['COM_PORT'],
                                    display_width=CONFIG_DATA["display"]["DISPLAY_WIDTH"],
-                                   display_height=CONFIG_DATA["display"]["DISPLAY_HEIGHT"],
-                                   update_queue=config.update_queue)
+                                   display_height=CONFIG_DATA["display"]["DISPLAY_HEIGHT"])
         elif CONFIG_DATA["display"]["REVISION"] == "B":
             self.lcd = LcdCommRevB(com_port=CONFIG_DATA['config']['COM_PORT'],
                                    display_width=CONFIG_DATA["display"]["DISPLAY_WIDTH"],
-                                   display_height=CONFIG_DATA["display"]["DISPLAY_HEIGHT"],
-                                   update_queue=config.update_queue)
+                                   display_height=CONFIG_DATA["display"]["DISPLAY_HEIGHT"])
         else:
             logger.error("Unknown display revision '", CONFIG_DATA["display"]["REVISION"], "'")
 

library/lcd_comm.py

@@ -17,8 +17,11 @@ class Orientation(IntEnum):
 
 
 class LcdComm(ABC):
-    def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_height: int = 480,
-                 update_queue: queue.Queue = None):
+    # Amount of time that we should delay between sending bitmap data and sending the next
+    # command. If commands are issued too quickly after a bitmap, corruption is seen.
+    inter_bitmap_delay = 0.02
+
+    def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_height: int = 480):
         self.lcd_serial = None
 
         # String containing absolute path to serial port e.g. "COM3", "/dev/ttyACM1" or "AUTO" for auto-discovery
@@ -31,13 +34,11 @@ def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_hei
         # Display height in default orientation (portrait)
         self.display_height = display_height
 
-        # Queue containing the serial requests to send to the screen. An external thread should run to process requests
-        # on the queue. If you want serial requests to be done in sequence, set it to None
-        self.update_queue = update_queue
+        # Last time we sent bitmap data
+        self.last_bitmap_time = 0
 
-        # Mutex to protect the queue in case a thread want to add multiple requests (e.g. image data) that should not be
-        # mixed with other requests in-between
-        self.update_queue_mutex = threading.Lock()
+        # Mutex that must be held over any requests that must be kept together.
+        self.com_mutex = threading.Lock()
 
     def get_width(self) -> int:
         if self.orientation == Orientation.PORTRAIT or self.orientation == Orientation.REVERSE_PORTRAIT:

library/lcd_comm_rev_a.py

@@ -19,9 +19,8 @@ class Command(IntEnum):
 
 
 class LcdCommRevA(LcdComm):
-    def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_height: int = 480,
-                 update_queue: queue.Queue = None):
-        LcdComm.__init__(self, com_port, display_width, display_height, update_queue)
+    def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_height: int = 480):
+        super().__init__(com_port, display_width, display_height)
         self.openSerial()
 
     def __del__(self):
@@ -38,7 +37,13 @@ def auto_detect_com_port():
 
         return auto_com_port
 
-    def SendCommand(self, cmd: Command, x: int, y: int, ex: int, ey: int, bypass_queue: bool = False):
+    def SendCommand(self, cmd: Command, x: int, y: int, ex: int, ey: int):
+
+        # Commands must be sent at least 'inter_bitmap_delay' after the bitmap data.
+        delay = (self.last_bitmap_time + self.inter_bitmap_delay) - time.time()
+        if delay > 0:
+            time.sleep(delay)
+
         byteBuffer = bytearray(6)
         byteBuffer[0] = (x >> 2)
         byteBuffer[1] = (((x & 3) << 6) + (y >> 4))
@@ -48,35 +53,37 @@ def SendCommand(self, cmd: Command, x: int, y: int, ex: int, ey: int, bypass_que
         byteBuffer[5] = cmd
 
         # If no queue for async requests, or if asked explicitly to do the request sequentially: do request now
-        if not self.update_queue or bypass_queue:
-            self.WriteData(byteBuffer)
-        else:
-            # Lock queue mutex then queue the request
-            with self.update_queue_mutex:
-                self.update_queue.put((self.WriteData, [byteBuffer]))
+        self.WriteData(byteBuffer)
 
     def InitializeComm(self):
         # HW revision A does not need init commands
         pass
 
     def Reset(self):
         logger.info("Display reset (COM port may change)...")
-        # Reset command bypasses queue because it is run when queue threads are not yet started
-        self.SendCommand(Command.RESET, 0, 0, 0, 0, bypass_queue=True)
-        # Wait for display reset then reconnect
-        time.sleep(1)
-        self.openSerial()
+
+        with self.com_mutex:
+            self.SendCommand(Command.RESET, 0, 0, 0, 0)
+
+            # NOTE: Shouldn't we close serial before we try to open it again ?
+
+            # Wait for display reset then reconnect
+            time.sleep(1)
+            self.openSerial()
 
     def Clear(self):
         self.SetOrientation(Orientation.PORTRAIT)  # Bug: orientation needs to be PORTRAIT before clearing
-        self.SendCommand(Command.CLEAR, 0, 0, 0, 0)
+        with self.com_mutex:
+            self.SendCommand(Command.CLEAR, 0, 0, 0, 0)
         self.SetOrientation()  # Restore default orientation
 
     def ScreenOff(self):
-        self.SendCommand(Command.SCREEN_OFF, 0, 0, 0, 0)
+        with self.com_mutex:
+            self.SendCommand(Command.SCREEN_OFF, 0, 0, 0, 0)
 
     def ScreenOn(self):
-        self.SendCommand(Command.SCREEN_ON, 0, 0, 0, 0)
+        with self.com_mutex:
+            self.SendCommand(Command.SCREEN_ON, 0, 0, 0, 0)
 
     def SetBrightness(self, level: int = 25):
         assert 0 <= level <= 100, 'Brightness level must be [0-100]'
@@ -86,7 +93,8 @@ def SetBrightness(self, level: int = 25):
         level_absolute = int(255 - ((level / 100) * 255))
 
         # Level : 0 (brightest) - 255 (darkest)
-        self.SendCommand(Command.SET_BRIGHTNESS, level_absolute, 0, 0, 0)
+        with self.com_mutex:
+            self.SendCommand(Command.SET_BRIGHTNESS, level_absolute, 0, 0, 0)
 
     def SetBackplateLedColor(self, led_color: tuple[int, int, int] = (255, 255, 255)):
         logger.info("HW revision A does not support backplate LED color setting")
@@ -112,7 +120,8 @@ def SetOrientation(self, orientation: Orientation = Orientation.PORTRAIT):
         byteBuffer[8] = (width & 255)
         byteBuffer[9] = (height >> 8)
         byteBuffer[10] = (height & 255)
-        self.lcd_serial.write(bytes(byteBuffer))
+        with self.com_mutex:
+            self.lcd_serial.write(bytes(byteBuffer))
 
     def DisplayPILImage(
             self,
@@ -141,13 +150,12 @@ def DisplayPILImage(
         (x0, y0) = (x, y)
         (x1, y1) = (x + image_width - 1, y + image_height - 1)
 
-        self.SendCommand(Command.DISPLAY_BITMAP, x0, y0, x1, y1)
+        with self.com_mutex:
+            self.SendCommand(Command.DISPLAY_BITMAP, x0, y0, x1, y1)
 
-        pix = image.load()
-        line = bytes()
+            pix = image.load()
+            line = bytes()
 
-        # Lock queue mutex then queue all the requests for the image data
-        with self.update_queue_mutex:
             for h in range(image_height):
                 for w in range(image_width):
                     R = pix[w, h][0] >> 3
@@ -159,9 +167,13 @@ def DisplayPILImage(
 
                     # Send image data by multiple of DISPLAY_WIDTH bytes
                     if len(line) >= self.get_width() * 8:
-                        self.SendLine(line)
+                        self.WriteLine(line)
                         line = bytes()
 
             # Write last line if needed
             if len(line) > 0:
-                self.SendLine(line)
+                self.WriteLine(line)
+
+            # There must be a short period between the last write of the bitmap data and the next
+            # command. This seems to be around 0.02s on the flagship device.
+            self.last_bitmap_time = time.time()

library/lcd_comm_rev_b.py

@@ -1,4 +1,5 @@
 import struct
+import time
 
 from serial.tools.list_ports import comports
 
@@ -30,9 +31,8 @@ class SubRevision(IntEnum):
 
 
 class LcdCommRevB(LcdComm):
-    def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_height: int = 480,
-                 update_queue: queue.Queue = None):
-        LcdComm.__init__(self, com_port, display_width, display_height, update_queue)
+    def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_height: int = 480):
+        super().__init__(com_port, display_width, display_height)
         self.openSerial()
         self.sub_revision = SubRevision.A01  # Run a Hello command to detect correct sub-rev.
 
@@ -56,7 +56,13 @@ def auto_detect_com_port():
 
         return auto_com_port
 
-    def SendCommand(self, cmd: Command, payload=None, bypass_queue: bool = False):
+    def SendCommand(self, cmd: Command, payload=None):
+
+        # Commands must be sent at least 'inter_bitmap_delay' after the bitmap data.
+        delay = (self.last_bitmap_time + self.inter_bitmap_delay) - time.time()
+        if delay > 0:
+            time.sleep(delay)
+
         # New protocol (10 byte packets, framed with the command, 8 data bytes inside)
         if payload is None:
             payload = [0] * 8
@@ -75,20 +81,15 @@ def SendCommand(self, cmd: Command, payload=None, bypass_queue: bool = False):
         byteBuffer[8] = payload[7]
         byteBuffer[9] = cmd
 
-        # If no queue for async requests, or if asked explicitly to do the request sequentially: do request now
-        if not self.update_queue or bypass_queue:
-            self.WriteData(byteBuffer)
-        else:
-            # Lock queue mutex then queue the request
-            with self.update_queue_mutex:
-                self.update_queue.put((self.WriteData, [byteBuffer]))
+        self.WriteData(byteBuffer)
 
     def Hello(self):
         hello = [ord('H'), ord('E'), ord('L'), ord('L'), ord('O')]
 
         # This command reads LCD answer on serial link, so it bypasses the queue
-        self.SendCommand(Command.HELLO, payload=hello, bypass_queue=True)
-        response = self.lcd_serial.read(10)
+        with self.com_mutex:
+            self.SendCommand(Command.HELLO, payload=hello)
+            response = self.lcd_serial.read(10)
 
         if len(response) != 10:
             logger.warning("Device not recognised (short response to HELLO)")
@@ -138,29 +139,31 @@ def SetBrightness(self, level_user: int = 25):
 
         if self.is_brightness_range():
             # Brightness scales from 0 to 255, with 255 being the brightest and 0 being the darkest.
-            # Convert our brightness % to an absolute value.
             level = int((level_user / 100) * 255)
         else:
             # Brightness is 1 (off) or 0 (full brightness)
             logger.info("Your display does not support custom brightness level")
             level = 1 if level_user == 0 else 0
 
-        self.SendCommand(Command.SET_BRIGHTNESS, payload=[level])
+        with self.com_mutex:
+            self.SendCommand(Command.SET_BRIGHTNESS, payload=[level])
 
     def SetBackplateLedColor(self, led_color: tuple[int, int, int] = (255, 255, 255)):
         if self.is_flagship():
-            self.SendCommand(Command.SET_LIGHTING, payload=led_color)
+            with self.com_mutex:
+                self.SendCommand(Command.SET_LIGHTING, payload=led_color)
         else:
             logger.info("Only HW revision 'flagship' supports backplate LED color setting")
 
     def SetOrientation(self, orientation: Orientation = Orientation.PORTRAIT, new_width: int = 320, new_height: int = 480):
         # In revision B, basic orientations (portrait / landscape) are managed by the display
         # The reverse orientations (reverse portrait / reverse landscape) are software-managed
         self.orientation = orientation
-        if self.orientation == Orientation.PORTRAIT or self.orientation == Orientation.REVERSE_PORTRAIT:
-            self.SendCommand(Command.SET_ORIENTATION, payload=[OrientationValueRevB.ORIENTATION_PORTRAIT])
-        else:
-            self.SendCommand(Command.SET_ORIENTATION, payload=[OrientationValueRevB.ORIENTATION_LANDSCAPE])
+        with self.com_mutex:
+            if self.orientation == Orientation.PORTRAIT or self.orientation == Orientation.REVERSE_PORTRAIT:
+                self.SendCommand(Command.SET_ORIENTATION, payload=[OrientationValueRevB.ORIENTATION_PORTRAIT])
+            else:
+                self.SendCommand(Command.SET_ORIENTATION, payload=[OrientationValueRevB.ORIENTATION_LANDSCAPE])
 
     def DisplayPILImage(
             self,
@@ -193,16 +196,17 @@ def DisplayPILImage(
             (x0, y0) = (self.get_width() - x - image_width, self.get_height() - y - image_height)
             (x1, y1) = (self.get_width() - x - 1, self.get_height() - y - 1)
 
-        self.SendCommand(Command.DISPLAY_BITMAP,
-                         payload=[(x0 >> 8) & 255, x0 & 255,
-                                  (y0 >> 8) & 255, y0 & 255,
-                                  (x1 >> 8) & 255, x1 & 255,
-                                  (y1 >> 8) & 255, y1 & 255])
+        # Do the image load outside the mutex in case it takes a long time
         pix = image.load()
-        line = bytes()
 
-        # Lock queue mutex then queue all the requests for the image data
-        with self.update_queue_mutex:
+        with self.com_mutex:
+            self.SendCommand(Command.DISPLAY_BITMAP,
+                             payload=[(x0 >> 8) & 255, x0 & 255,
+                                      (y0 >> 8) & 255, y0 & 255,
+                                      (x1 >> 8) & 255, x1 & 255,
+                                      (y1 >> 8) & 255, y1 & 255])
+            line = bytes()
+
             for h in range(image_height):
                 for w in range(image_width):
                     if self.orientation == Orientation.PORTRAIT or self.orientation == Orientation.LANDSCAPE:
@@ -227,9 +231,13 @@ def DisplayPILImage(
 
                     # Send image data by multiple of DISPLAY_WIDTH bytes
                     if len(line) >= self.get_width() * 8:
-                        self.SendLine(line)
+                        self.WriteLine(line)
                         line = bytes()
 
             # Write last line if needed
             if len(line) > 0:
-                self.SendLine(line)
+                self.WriteLine(line)
+
+            # There must be a short period between the last write of the bitmap data and the next
+            # command. This seems to be around 0.02s on the flagship device.
+            self.last_bitmap_time = time.time()