Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force restart device on serial error. #274

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Force restart device on serial error. #274

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3fd76d8
Force restart device on serial error.
alexwbaule Jun 30, 2023
3af3dca
Check if is None (device does not exist)
alexwbaule Jun 30, 2023
2f8553f
Update detection Pid/Vid.
alexwbaule Jul 5, 2023
a244504
Update README.md
mathoudebine Jul 4, 2023
5a21541
default config
alexwbaule Jul 5, 2023
7110730
If has idProduct and idVendor
alexwbaule Jul 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 31 additions & 19 deletions library/lcd/lcd_comm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import os
import queue
import signal
import sys
import threading
from abc import ABC, abstractmethod
Expand All @@ -28,6 +29,8 @@
from PIL import Image, ImageDraw, ImageFont

from library.log import logger
from usb.core import find as finddev
from usb.core import USBError


class Orientation(IntEnum):
Expand All @@ -45,6 +48,10 @@ def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_hei
# String containing absolute path to serial port e.g. "COM3", "/dev/ttyACM1" or "AUTO" for auto-discovery
self.com_port = com_port

# This is used to reset the device if the computer sleeps and freeze the display.
self.idVendor = None
self.idProduct = None

# Display always start in portrait orientation by default
self.orientation = Orientation.PORTRAIT
# Display width in default orientation (portrait)
Expand Down Expand Up @@ -88,7 +95,7 @@ def openSerial(self):
logger.debug(f"Static COM port: {self.com_port}")

try:
self.lcd_serial = serial.Serial(self.com_port, 115200, timeout=1, rtscts=1)
self.lcd_serial = serial.Serial(self.com_port, 115200, timeout=5, rtscts=1, write_timeout=5)
except Exception as e:
logger.error(f"Cannot open COM port {self.com_port}: {e}")
try:
Expand Down Expand Up @@ -116,32 +123,18 @@ def SendLine(self, line: bytes):
def WriteLine(self, line: bytes):
try:
self.lcd_serial.write(line)
except serial.serialutil.SerialTimeoutException:
# We timed-out trying to write to our device, slow things down.
logger.warning("(Write line) Too fast! Slow down!")
except serial.serialutil.SerialException:
# Error writing data to device: close and reopen serial port, try to write again
logger.error(
"SerialException: Failed to send serial data to device. Closing and reopening COM port before retrying once.")
self.closeSerial()
self.openSerial()
self.lcd_serial.write(line)
logger.warning("(WriteLine) error, reseting device.")
self.reset_by_usb()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a shame to always reset USB and stop the program, when in some cases just the closeSerial/openSerial/write is enough to keep the program running (people have confirmed this in #269)
Maybe the closeSerial/openSerial/write can be kept, and then only on exception the USB is reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good question... In case of desktop suspend/resume, only close and reopen the serial, don't work in 5 pol version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, the official app on windows, send reset USB in case of errors in serial. It's try to open serial 2 times, if don't work, it's send the reset. I checked this behavior with Wireshark in USB.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 5 pol version its very strict to send images, updates, if the order is not following, it's doesn't work.
For me, the correct is to stop all threads, reset USB and start again, but it's a massive change to the code, stop the program has less work to do, because the systemd cares about restart it.


def ReadData(self, readSize: int):
try:
response = self.lcd_serial.read(readSize)
# logger.debug("Received: [{}]".format(str(response, 'utf-8')))
return response
except serial.serialutil.SerialTimeoutException:
# We timed-out trying to read from our device, slow things down.
logger.warning("(Read data) Too fast! Slow down!")
except serial.serialutil.SerialException:
# Error writing data to device: close and reopen serial port, try to read again
logger.error(
"SerialException: Failed to read serial data from device. Closing and reopening COM port before retrying once.")
self.closeSerial()
self.openSerial()
return self.lcd_serial.read(readSize)
logger.warning("(ReadData) error, reseting device.")
self.reset_by_usb()

@staticmethod
@abstractmethod
Expand All @@ -156,6 +149,25 @@ def InitializeComm(self):
def Reset(self):
pass

def reset_by_usb(self):
logger.info(f"Reseting device via USB...cleaning all {self.update_queue.qsize()} queue entries")
with self.update_queue_mutex:
while not self.update_queue.empty():
try:
self.update_queue.get()
except queue.Empty:
continue
self.update_queue.task_done()
logger.info(f"Reseting device via USB queue cleaned: {self.update_queue.empty()}")
try:
dev = finddev(idVendor=self.idVendor, idProduct=self.idProduct)
if dev is not None:
dev.reset()
os.kill(os.getpid(), signal.SIGTERM)
except USBError or OSError:
logger.info("Error reseting device via USB...")
pass

@abstractmethod
def Clear(self):
pass
Expand Down
2 changes: 2 additions & 0 deletions library/lcd/lcd_comm_rev_a.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_hei
update_queue: queue.Queue = None):
LcdComm.__init__(self, com_port, display_width, display_height, update_queue)
self.openSerial()
self.idVendor = 0x1a86
self.idProduct = 0x5722
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idVendor and idProduct may vary, I have seen at least 2 different values for Turing smart screens. It would be better to read them directly from the device after opening serial (can be done in openSerial() for all devices)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, ! Good point, there is a variable holding this values in serial ? In don't know.

Copy link
Owner

@mathoudebine mathoudebine Jun 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not in Serial, but it can be retrieved from COM ports list:

from serial.tools.list_ports import comports

[...]

        for com_port in comports():
            if com_port.device == self.com_port:
                self.idVendor = com_port.vid
                self.idProduct = com_port.pid
                break

It can be added in lcd_comm.openSerial() after COM port discovery
https://pyserial.readthedocs.io/en/latest/tools.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great ! I will replace that part.


def __del__(self):
self.closeSerial()
Expand Down
2 changes: 2 additions & 0 deletions library/lcd/lcd_comm_rev_b.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ def __init__(self, com_port: str = "AUTO", display_width: int = 320, display_hei
update_queue: queue.Queue = None):
LcdComm.__init__(self, com_port, display_width, display_height, update_queue)
self.openSerial()
self.idVendor = 0x1a86
self.idProduct = 0x5722
self.sub_revision = SubRevision.A01 # Run a Hello command to detect correct sub-rev.

def __del__(self):
Expand Down
2 changes: 2 additions & 0 deletions library/lcd/lcd_comm_rev_c.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ def __init__(self, com_port: str = "AUTO", display_width: int = 480, display_hei
update_queue: queue.Queue = None):
LcdComm.__init__(self, com_port, display_width, display_height, update_queue)
self.openSerial()
self.idVendor = 0x1d6b
self.idProduct = 0x0106

def __del__(self):
self.closeSerial()
Expand Down