From fb5ea00651ac31364b9ba7e2ccec75f35405dfd5 Mon Sep 17 00:00:00 2001 From: samsam Date: Wed, 15 Mar 2023 21:19:15 +0100 Subject: [PATCH 1/9] adding missing sdo aborts for sdo client --- canopen/sdo/client.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index fd5fd99b..5b894e2b 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -302,8 +302,10 @@ def read(self, size=-1): response = self.sdo_client.request_response(request) res_command, = struct.unpack_from("B", response) if res_command & 0xE0 != RESPONSE_SEGMENT_UPLOAD: + self.sdo_client.abort(0x05040001) raise SdoCommunicationError(f"Unexpected response 0x{res_command:02X}") if res_command & TOGGLE_BIT != self._toggle: + self.sdo_client.abort(0x05030000) raise SdoCommunicationError("Toggle bit mismatch") length = 7 - ((res_command >> 1) & 0x7) if res_command & NO_MORE_DATA: From 551e20aa2df573708fdb5d1860067a3ff0cea380 Mon Sep 17 00:00:00 2001 From: samsam Date: Wed, 15 Mar 2023 22:18:41 +0100 Subject: [PATCH 2/9] adding some missing sdo aborts (timeout, unknown command) --- canopen/sdo/client.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index 5b894e2b..e594f5b8 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -69,6 +69,7 @@ def read_response(self): response = self.responses.get( block=True, timeout=self.RESPONSE_TIMEOUT) except queue.Empty: + self.abort(0x05040000) raise SdoCommunicationError("No SDO response received") res_command, = struct.unpack_from("B", response) if res_command == RESPONSE_ABORTED: @@ -364,6 +365,7 @@ def __init__(self, sdo_client, index, subindex=0, size=None, force_segment=False response = sdo_client.request_response(request) res_command, = struct.unpack_from("B", response) if res_command != RESPONSE_DOWNLOAD: + self.sdo_client.abort(0x05040001) raise SdoCommunicationError( f"Unexpected response 0x{res_command:02X}") else: @@ -392,6 +394,7 @@ def write(self, b): response = self.sdo_client.request_response(request) res_command, = struct.unpack_from("B", response) if res_command & 0xE0 != RESPONSE_DOWNLOAD: + self.sdo_client.abort(0x05040001) raise SdoCommunicationError( f"Unexpected response 0x{res_command:02X}") bytes_sent = len(b) @@ -416,6 +419,7 @@ def write(self, b): response = self.sdo_client.request_response(request) res_command, = struct.unpack("B", response[0:1]) if res_command & 0xE0 != RESPONSE_SEGMENT_DOWNLOAD: + self.sdo_client.abort(0x05040001) raise SdoCommunicationError( f"Unexpected response 0x{res_command:02X} " f"(expected 0x{RESPONSE_SEGMENT_DOWNLOAD:02X})") From 14ecf12e8020dcbfef0a420492a36ffbfc9a18c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 22:55:10 +0200 Subject: [PATCH 3/9] Consistently format abort code literals. --- canopen/sdo/client.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index e594f5b8..bec8c684 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -69,7 +69,7 @@ def read_response(self): response = self.responses.get( block=True, timeout=self.RESPONSE_TIMEOUT) except queue.Empty: - self.abort(0x05040000) + self.abort(0x0504_0000) raise SdoCommunicationError("No SDO response received") res_command, = struct.unpack_from("B", response) if res_command == RESPONSE_ABORTED: @@ -90,7 +90,7 @@ def request_response(self, sdo_request): except SdoCommunicationError as e: retries_left -= 1 if not retries_left: - self.abort(0x5040000) + self.abort(0x0504_0000) raise logger.warning(str(e)) @@ -303,10 +303,10 @@ def read(self, size=-1): response = self.sdo_client.request_response(request) res_command, = struct.unpack_from("B", response) if res_command & 0xE0 != RESPONSE_SEGMENT_UPLOAD: - self.sdo_client.abort(0x05040001) + self.sdo_client.abort(0x0504_0001) raise SdoCommunicationError(f"Unexpected response 0x{res_command:02X}") if res_command & TOGGLE_BIT != self._toggle: - self.sdo_client.abort(0x05030000) + self.sdo_client.abort(0x0503_0000) raise SdoCommunicationError("Toggle bit mismatch") length = 7 - ((res_command >> 1) & 0x7) if res_command & NO_MORE_DATA: @@ -365,7 +365,7 @@ def __init__(self, sdo_client, index, subindex=0, size=None, force_segment=False response = sdo_client.request_response(request) res_command, = struct.unpack_from("B", response) if res_command != RESPONSE_DOWNLOAD: - self.sdo_client.abort(0x05040001) + self.sdo_client.abort(0x0504_0001) raise SdoCommunicationError( f"Unexpected response 0x{res_command:02X}") else: @@ -394,7 +394,7 @@ def write(self, b): response = self.sdo_client.request_response(request) res_command, = struct.unpack_from("B", response) if res_command & 0xE0 != RESPONSE_DOWNLOAD: - self.sdo_client.abort(0x05040001) + self.sdo_client.abort(0x0504_0001) raise SdoCommunicationError( f"Unexpected response 0x{res_command:02X}") bytes_sent = len(b) @@ -419,7 +419,7 @@ def write(self, b): response = self.sdo_client.request_response(request) res_command, = struct.unpack("B", response[0:1]) if res_command & 0xE0 != RESPONSE_SEGMENT_DOWNLOAD: - self.sdo_client.abort(0x05040001) + self.sdo_client.abort(0x0504_0001) raise SdoCommunicationError( f"Unexpected response 0x{res_command:02X} " f"(expected 0x{RESPONSE_SEGMENT_DOWNLOAD:02X})") @@ -493,7 +493,7 @@ def __init__(self, sdo_client, index, subindex=0, request_crc_support=True): res_command, res_index, res_subindex = SDO_STRUCT.unpack_from(response) if res_command & 0xE0 != RESPONSE_BLOCK_UPLOAD: self._error = True - self.sdo_client.abort(0x05040001) + self.sdo_client.abort(0x0504_0001) raise SdoCommunicationError(f"Unexpected response 0x{res_command:02X}") # Check that the message is for us if res_index != index or res_subindex != subindex: @@ -550,7 +550,7 @@ def read(self, size=-1): if self._done: if self._server_crc != self._crc.final(): self._error = True - self.sdo_client.abort(0x05040004) + self.sdo_client.abort(0x0504_0004) raise SdoCommunicationError("CRC is not OK") logger.info("CRC is OK") self.pos += len(data) @@ -570,7 +570,7 @@ def _retransmit(self): self._ackseq = seqno return response self._error = True - self.sdo_client.abort(0x05040000) + self.sdo_client.abort(0x0504_0000) raise SdoCommunicationError("Some data were lost and could not be retransmitted") def _ack_block(self): @@ -586,11 +586,11 @@ def _end_upload(self): res_command, self._server_crc = struct.unpack_from("> 2) & 0x7 @@ -660,7 +660,7 @@ def __init__(self, sdo_client, index, subindex=0, size=None, request_crc_support response = sdo_client.request_response(request) res_command, res_index, res_subindex = SDO_STRUCT.unpack_from(response) if res_command & 0xE0 != RESPONSE_BLOCK_DOWNLOAD: - self.sdo_client.abort(0x05040001) + self.sdo_client.abort(0x0504_0001) raise SdoCommunicationError( f"Unexpected response 0x{res_command:02X}") # Check that the message is for us @@ -743,11 +743,11 @@ def _block_ack(self): response = self.sdo_client.read_response() res_command, ackseq, blksize = struct.unpack_from("BBB", response) if res_command & 0xE0 != RESPONSE_BLOCK_DOWNLOAD: - self.sdo_client.abort(0x05040001) + self.sdo_client.abort(0x0504_0001) raise SdoCommunicationError( f"Unexpected response 0x{res_command:02X}") if res_command & 0x3 != BLOCK_TRANSFER_RESPONSE: - self.sdo_client.abort(0x05040001) + self.sdo_client.abort(0x0504_0001) raise SdoCommunicationError("Server did not respond with a " "block download response") if ackseq != self._blksize: From 080d8735e18795e3f51ac1991e4f168c28518ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Tue, 17 Jun 2025 22:55:44 +0200 Subject: [PATCH 4/9] Fix typo (grammatical error in exception message). --- canopen/sdo/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index bec8c684..1f8fbbdf 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -571,7 +571,7 @@ def _retransmit(self): return response self._error = True self.sdo_client.abort(0x0504_0000) - raise SdoCommunicationError("Some data were lost and could not be retransmitted") + raise SdoCommunicationError("Some data was lost and could not be retransmitted") def _ack_block(self): request = bytearray(8) From 8beeba46bd35109179c173b013bcf14503136e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Wed, 18 Jun 2025 11:15:14 +0200 Subject: [PATCH 5/9] Avoid duplicate SDO abort messages on timeout. For each call to SdoClient.read_response(), the timeout abort message can either be sent immediately before raising the exception, or it is sent later, after the exception has been handled e.g. by a retry. Add a boolean parameter to the function, which is usually False to skip the immediate transmission of an SDO abort. Only two cases actually need it, passing the option as True. --- canopen/sdo/client.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index 1f8fbbdf..0ce12bfb 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -64,12 +64,13 @@ def send_request(self, request): else: break - def read_response(self): + def read_response(self, timeout_abort: bool = False): try: response = self.responses.get( block=True, timeout=self.RESPONSE_TIMEOUT) except queue.Empty: - self.abort(0x0504_0000) + if timeout_abort: + self.abort(0x0504_0000) raise SdoCommunicationError("No SDO response received") res_command, = struct.unpack_from("B", response) if res_command == RESPONSE_ABORTED: @@ -86,7 +87,7 @@ def request_response(self, sdo_request): self.send_request(sdo_request) # Wait for node to respond try: - return self.read_response() + return self.read_response(timeout_abort=False) except SdoCommunicationError as e: retries_left -= 1 if not retries_left: @@ -526,7 +527,7 @@ def read(self, size=-1): return self.readall() try: - response = self.sdo_client.read_response() + response = self.sdo_client.read_response(timeout_abort=False) except SdoCommunicationError: response = self._retransmit() res_command, = struct.unpack_from("B", response) @@ -562,7 +563,7 @@ def _retransmit(self): end_time = time.time() + self.sdo_client.RESPONSE_TIMEOUT self._ack_block() while time.time() < end_time: - response = self.sdo_client.read_response() + response = self.sdo_client.read_response(timeout_abort=False) res_command, = struct.unpack_from("B", response) seqno = res_command & 0x7F if seqno == self._ackseq + 1: @@ -582,7 +583,7 @@ def _ack_block(self): self._ackseq = 0 def _end_upload(self): - response = self.sdo_client.read_response() + response = self.sdo_client.read_response(timeout_abort=True) res_command, self._server_crc = struct.unpack_from(" Date: Mon, 30 Jun 2025 10:02:17 +0200 Subject: [PATCH 6/9] Move SDO abort message sending to _block_ack() and _end_upload(). --- canopen/sdo/client.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index 0ce12bfb..8fd10e20 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -583,7 +583,11 @@ def _ack_block(self): self._ackseq = 0 def _end_upload(self): - response = self.sdo_client.read_response(timeout_abort=True) + try: + response = self.sdo_client.read_response(timeout_abort=False) + except SdoCommunicationError: + self.abort(0x0504_0000) + raise res_command, self._server_crc = struct.unpack_from(" Date: Mon, 30 Jun 2025 10:03:38 +0200 Subject: [PATCH 7/9] Ditch unused timeout_abort parameter again. --- canopen/sdo/client.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index 8fd10e20..1241a614 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -64,13 +64,11 @@ def send_request(self, request): else: break - def read_response(self, timeout_abort: bool = False): + def read_response(self): try: response = self.responses.get( block=True, timeout=self.RESPONSE_TIMEOUT) except queue.Empty: - if timeout_abort: - self.abort(0x0504_0000) raise SdoCommunicationError("No SDO response received") res_command, = struct.unpack_from("B", response) if res_command == RESPONSE_ABORTED: @@ -87,7 +85,7 @@ def request_response(self, sdo_request): self.send_request(sdo_request) # Wait for node to respond try: - return self.read_response(timeout_abort=False) + return self.read_response() except SdoCommunicationError as e: retries_left -= 1 if not retries_left: @@ -527,7 +525,7 @@ def read(self, size=-1): return self.readall() try: - response = self.sdo_client.read_response(timeout_abort=False) + response = self.sdo_client.read_response() except SdoCommunicationError: response = self._retransmit() res_command, = struct.unpack_from("B", response) @@ -563,7 +561,7 @@ def _retransmit(self): end_time = time.time() + self.sdo_client.RESPONSE_TIMEOUT self._ack_block() while time.time() < end_time: - response = self.sdo_client.read_response(timeout_abort=False) + response = self.sdo_client.read_response() res_command, = struct.unpack_from("B", response) seqno = res_command & 0x7F if seqno == self._ackseq + 1: @@ -584,7 +582,7 @@ def _ack_block(self): def _end_upload(self): try: - response = self.sdo_client.read_response(timeout_abort=False) + response = self.sdo_client.read_response() except SdoCommunicationError: self.abort(0x0504_0000) raise @@ -746,7 +744,7 @@ def tell(self): def _block_ack(self): logger.debug("Waiting for acknowledgement of last block...") try: - response = self.sdo_client.read_response(timeout_abort=False) + response = self.sdo_client.read_response() except SdoCommunicationError: self.sdo_client.abort(0x0504_0000) raise From 3de9bcecb7d1813a06c0f25c85691ce3d73cd47e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Mon, 30 Jun 2025 10:05:06 +0200 Subject: [PATCH 8/9] Another literal format. --- canopen/sdo/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index 1241a614..04565664 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -93,7 +93,7 @@ def request_response(self, sdo_request): raise logger.warning(str(e)) - def abort(self, abort_code=0x08000000): + def abort(self, abort_code=0x0800_0000): """Abort current transfer.""" request = bytearray(8) request[0] = REQUEST_ABORTED From d02f3a7939cdaa4107ab962156ccb68bd124de86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Sun, 13 Jul 2025 14:23:48 +0200 Subject: [PATCH 9/9] Add docstring to read_response() and detail exceptions. --- canopen/sdo/client.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index 04565664..f961fb20 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -65,6 +65,13 @@ def send_request(self, request): break def read_response(self): + """Wait for an SDO response and handle timeout or remote abort. + + :raises canopen.SdoAbortedError: + When receiving an SDO abort response from the server. + :raises canopen.SdoCommunicationError: + After timeout with no response received. + """ try: response = self.responses.get( block=True, timeout=self.RESPONSE_TIMEOUT)