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.

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

Conversation

alexwbaule
Copy link
Contributor

With this commit, when the devices stops read/write will clean the queue, reset the device with USB Command, and send "SIGTERM" signal to himself, to stop the daemon.
In linux, with "Restart=always" in service file, will restart the service, fixing some read/write problem.

Please, someone can check if in windows its work ? (i think its work !)

With this commit, when the devices stops read/write will clean
the queue, reset the device with USB Command, and send "SIGTERM" signal
to himself, to stop the daemon.
In linux, with "Restart=always" in service file, will restart the
service, fixing some read/write problem.
@alexwbaule
Copy link
Contributor Author

Hi @mathoudebine !

I replace your code in Read/Write, if some exceptions occours, calls "usb_reset" from usb.core, reset the device and stop the service.
My Desktop sometimes has Suspended by time, and when its returns from Suspend, the service stop to update the display, with this, the display is "reseted" and "restarted".

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.

@@ -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

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.

@alexwbaule
Copy link
Contributor Author

Hi @mathoudebine ,

i Find some solution about suspend/resume, building a "sleep" entry service to systemd.

About the "force reset", maybe we can change it to do after 2 or 3 times that the reconnect fails.

Ah, i think its good to , add a "write_timeout" to Serial.

What you think ?

alexwbaule and others added 4 commits July 4, 2023 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants