From e840015cad0bbbf882e0b90cb8d722725c6e1dd2 Mon Sep 17 00:00:00 2001 From: GHGiampy <83699429+GHGiampy@users.noreply.github.com> Date: Thu, 14 Jul 2022 03:25:35 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20Abort=20firmware=20update=20on?= =?UTF-8?q?=20transfer=20error=20(#24472,=20#24499)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../share/scripts/MarlinBinaryProtocol.py | 21 ++- buildroot/share/scripts/upload.py | 132 +++++++++++++----- 2 files changed, 111 insertions(+), 42 deletions(-) diff --git a/buildroot/share/scripts/MarlinBinaryProtocol.py b/buildroot/share/scripts/MarlinBinaryProtocol.py index 4887ad9919..ecf9df35e2 100644 --- a/buildroot/share/scripts/MarlinBinaryProtocol.py +++ b/buildroot/share/scripts/MarlinBinaryProtocol.py @@ -376,11 +376,13 @@ class FileTransferProtocol(object): token, data = self.await_response(1000) if token == 'PFT:success': print("File closed") - return + return True elif token == 'PFT:ioerror': print("Client storage device IO error") + return False elif token == 'PFT:invalid': print("No open file") + return False def abort(self): self.protocol.send(FileTransferProtocol.protocol_id, FileTransferProtocol.Packet.ABORT); @@ -417,12 +419,23 @@ class FileTransferProtocol(object): self.write(data[start:end]) kibs = (( (i+1) * block_size) / 1024) / (millis() + 1 - start_time) * 1000 if (i / blocks) >= dump_pctg: - print("\r{0:2.2f}% {1:4.2f}KiB/s {2} Errors: {3}".format((i / blocks) * 100, kibs, "[{0:4.2f}KiB/s]".format(kibs * cratio) if compression_support else "", self.protocol.errors), end='') + print("\r{0:2.0f}% {1:4.2f}KiB/s {2} Errors: {3}".format((i / blocks) * 100, kibs, "[{0:4.2f}KiB/s]".format(kibs * cratio) if compression_support else "", self.protocol.errors), end='') dump_pctg += 0.1 - print("\r{0:2.2f}% {1:4.2f}KiB/s {2} Errors: {3}".format(100, kibs, "[{0:4.2f}KiB/s]".format(kibs * cratio) if compression_support else "", self.protocol.errors)) # no one likes transfers finishing at 99.8% + if self.protocol.errors > 0: + # Dump last status (errors may not be visible) + print("\r{0:2.0f}% {1:4.2f}KiB/s {2} Errors: {3} - Aborting...".format((i / blocks) * 100, kibs, "[{0:4.2f}KiB/s]".format(kibs * cratio) if compression_support else "", self.protocol.errors), end='') + print("") # New line to break the transfer speed line + self.close() + print("Transfer aborted due to protocol errors") + #raise Exception("Transfer aborted due to protocol errors") + return False; + print("\r{0:2.0f}% {1:4.2f}KiB/s {2} Errors: {3}".format(100, kibs, "[{0:4.2f}KiB/s]".format(kibs * cratio) if compression_support else "", self.protocol.errors)) # no one likes transfers finishing at 99.8% - self.close() + if not self.close(): + print("Transfer failed") + return False print("Transfer complete") + return True class EchoProtocol(object): diff --git a/buildroot/share/scripts/upload.py b/buildroot/share/scripts/upload.py index c7730d8f29..52fa1abc54 100644 --- a/buildroot/share/scripts/upload.py +++ b/buildroot/share/scripts/upload.py @@ -20,14 +20,18 @@ Import("env") import MarlinBinaryProtocol -# Internal debug flag -Debug = False - #-----------------# # Upload Callback # #-----------------# def Upload(source, target, env): + #-------# + # Debug # + #-------# + Debug = False # Set to True to enable script debug + def debugPrint(data): + if Debug: print(f"[Debug]: {data}") + #------------------# # Marlin functions # #------------------# @@ -39,19 +43,35 @@ def Upload(source, target, env): # Port functions # #----------------# def _GetUploadPort(env): - if Debug: print('Autodetecting upload port...') + debugPrint('Autodetecting upload port...') env.AutodetectUploadPort(env) - port = env.subst('$UPLOAD_PORT') - if not port: + portName = env.subst('$UPLOAD_PORT') + if not portName: raise Exception('Error detecting the upload port.') - if Debug: print('OK') - return port + debugPrint('OK') + return portName #-------------------------# # Simple serial functions # #-------------------------# + def _OpenPort(): + # Open serial port + if port.is_open: return + debugPrint('Opening upload port...') + port.open() + port.reset_input_buffer() + debugPrint('OK') + + def _ClosePort(): + # Open serial port + if port is None: return + if not port.is_open: return + debugPrint('Closing upload port...') + port.close() + debugPrint('OK') + def _Send(data): - if Debug: print(f'>> {data}') + debugPrint(f'>> {data}') strdata = bytearray(data, 'utf8') + b'\n' port.write(strdata) time.sleep(0.010) @@ -60,37 +80,37 @@ def Upload(source, target, env): clean_responses = [] responses = port.readlines() for Resp in responses: - # Test: suppress invaid chars (coming from debug info) + # Suppress invalid chars (coming from debug info) try: clean_response = Resp.decode('utf8').rstrip().lstrip() clean_responses.append(clean_response) + debugPrint(f'<< {clean_response}') except: pass - if Debug: print(f'<< {clean_response}') return clean_responses #------------------# # SDCard functions # #------------------# def _CheckSDCard(): - if Debug: print('Checking SD card...') + debugPrint('Checking SD card...') _Send('M21') Responses = _Recv() if len(Responses) < 1 or not any('SD card ok' in r for r in Responses): raise Exception('Error accessing SD card') - if Debug: print('SD Card OK') + debugPrint('SD Card OK') return True #----------------# # File functions # #----------------# def _GetFirmwareFiles(UseLongFilenames): - if Debug: print('Get firmware files...') + debugPrint('Get firmware files...') _Send(f"M20 F{'L' if UseLongFilenames else ''}") Responses = _Recv() if len(Responses) < 3 or not any('file list' in r for r in Responses): raise Exception('Error getting firmware files') - if Debug: print('OK') + debugPrint('OK') return Responses def _FilterFirmwareFiles(FirmwareList, UseLongFilenames): @@ -114,6 +134,17 @@ def Upload(source, target, env): raise Exception(f"Firmware file '{FirmwareFile}' not removed") return Removed + def _RollbackUpload(FirmwareFile): + if not rollback: return + print(f"Rollback: trying to delete firmware '{FirmwareFile}'...") + _OpenPort() + # Wait for SD card release + time.sleep(1) + # Remount SD card + _CheckSDCard() + print(' OK' if _RemoveFirmwareFile(FirmwareFile) else ' Error!') + _ClosePort() + #---------------------# # Callback Entrypoint # @@ -121,6 +152,7 @@ def Upload(source, target, env): port = None protocol = None filetransfer = None + rollback = False # Get Marlin evironment vars MarlinEnv = env['MARLIN_FEATURES'] @@ -204,9 +236,9 @@ def Upload(source, target, env): if not marlin_custom_firmware_upload: raise Exception(f"CUSTOM_FIRMWARE_UPLOAD must be enabled in 'Configuration_adv.h' for '{marlin_motherboard}'") - # Init serial port + # Init & Open serial port port = serial.Serial(upload_port, baudrate = upload_speed, write_timeout = 0, timeout = 0.1) - port.reset_input_buffer() + _OpenPort() # Check SD card status _CheckSDCard() @@ -228,24 +260,26 @@ def Upload(source, target, env): print(' OK' if _RemoveFirmwareFile(OldFirmwareFile) else ' Error!') # Close serial - port.close() + _ClosePort() # Cleanup completed - if Debug: print('Cleanup completed') + debugPrint('Cleanup completed') # WARNING! The serial port must be closed here because the serial transfer that follow needs it! # Upload firmware file - if Debug: print(f"Copy '{upload_firmware_source_name}' --> '{upload_firmware_target_name}'") + debugPrint(f"Copy '{upload_firmware_source_name}' --> '{upload_firmware_target_name}'") protocol = MarlinBinaryProtocol.Protocol(upload_port, upload_speed, upload_blocksize, float(upload_error_ratio), int(upload_timeout)) #echologger = MarlinBinaryProtocol.EchoProtocol(protocol) protocol.connect() + # Mark the rollback (delete broken transfer) from this point on + rollback = True filetransfer = MarlinBinaryProtocol.FileTransferProtocol(protocol) - filetransfer.copy(upload_firmware_source_name, upload_firmware_target_name, upload_compression, upload_test) + transferOK = filetransfer.copy(upload_firmware_source_name, upload_firmware_target_name, upload_compression, upload_test) protocol.disconnect() # Notify upload completed - protocol.send_ascii('M117 Firmware uploaded') + protocol.send_ascii('M117 Firmware uploaded' if transferOK else 'M117 Firmware upload failed') # Remount SD card print('Wait for SD card release...') @@ -253,34 +287,56 @@ def Upload(source, target, env): print('Remount SD card') protocol.send_ascii('M21') - # Trigger firmware update - if upload_reset: - print('Trigger firmware update...') - protocol.send_ascii('M997', True) + # Transfer failed? + if not transferOK: + protocol.shutdown() + _RollbackUpload(upload_firmware_target_name) + else: + # Trigger firmware update + if upload_reset: + print('Trigger firmware update...') + protocol.send_ascii('M997', True) + protocol.shutdown() - protocol.shutdown() - print('Firmware update completed') + print('Firmware update completed' if transferOK else 'Firmware update failed') + return 0 if transferOK else -1 except KeyboardInterrupt: - if port: port.close() + print('Aborted by user') if filetransfer: filetransfer.abort() - if protocol: protocol.shutdown() + if protocol: + protocol.disconnect() + protocol.shutdown() + _RollbackUpload(upload_firmware_target_name) + _ClosePort() raise except serial.SerialException as se: - if port: port.close() - print(f'Serial excepion: {se}') + # This exception is raised only for send_ascii data (not for binary transfer) + print(f'Serial excepion: {se}, transfer aborted') + if protocol: + protocol.disconnect() + protocol.shutdown() + _RollbackUpload(upload_firmware_target_name) + _ClosePort() raise Exception(se) except MarlinBinaryProtocol.FatalError: - if port: port.close() - if protocol: protocol.shutdown() - print('Too many retries, Abort') + print('Too many retries, transfer aborted') + if protocol: + protocol.disconnect() + protocol.shutdown() + _RollbackUpload(upload_firmware_target_name) + _ClosePort() raise - except: - if port: port.close() - if protocol: protocol.shutdown() + except Exception as ex: + print(f"\nException: {ex}, transfer aborted") + if protocol: + protocol.disconnect() + protocol.shutdown() + _RollbackUpload(upload_firmware_target_name) + _ClosePort() print('Firmware not updated') raise