Skip to content

Fix SDO abort addresses and block-upload end timeouts #686

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

Open
dhairyajangir wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix SDO abort addresses and block-upload end timeouts #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
638591b
Fix timeout at the end of SDO block uploads
Nov 10, 2026
772648a
Include object addresses in client SDO aborts
dhairyajangir Nov 11, 2026
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
169 changes: 138 additions & 31 deletions canopen/sdo/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,12 @@ def read_response(self):
raise SdoAbortedError(abort_code)
return response

def request_response(self, sdo_request):
def request_response(self, sdo_request, *, index=0, subindex=0):
"""Send a request, identifying its transfer in any timeout abort.

Stream operations supply the original object address. Low-level callers
can provide it explicitly; omitted values retain the zero-address default.
"""
retries_left = self.MAX_RETRIES
if not self.responses.empty():
# logger.warning("There were unexpected messages in the queue")
Expand All @@ -96,15 +101,25 @@ def request_response(self, sdo_request):
except SdoCommunicationError as e:
retries_left -= 1
if not retries_left:
self.abort(ABORT_TIMED_OUT)
self.abort(ABORT_TIMED_OUT, index=index, subindex=subindex)
raise
logger.warning(str(e))

def abort(self, abort_code=ABORT_GENERAL_ERROR):
"""Abort current transfer."""
def abort(self, abort_code=ABORT_GENERAL_ERROR, *, index=0, subindex=0):
"""Abort a transfer with its object dictionary address.

:param int abort_code:
Reason for aborting. Defaults to a general error.
:param int index:
Object dictionary index of the transfer, or zero if unspecified.
:param int subindex:
Object dictionary subindex of the transfer, or zero if unspecified.

Stream operations supply their original address automatically. Existing
low-level calls without an address retain their zero-address behavior.
"""
request = bytearray(8)
request[0] = REQUEST_ABORTED
# TODO: Is it necessary to include index and subindex?
SDO_STRUCT.pack_into(request, 0, REQUEST_ABORTED, index, subindex)
struct.pack_into("<L", request, 4, abort_code)
self.send_request(request)
logger.error("Transfer aborted by client with code 0x%08X", abort_code)
Expand Down Expand Up @@ -242,6 +257,8 @@ def __init__(self, sdo_client, index, subindex=0):
:param int subindex:
Object dictionary sub-index to read from.
"""
self._index = index
self._subindex = subindex
self._done = False
self.sdo_client = sdo_client
self._toggle = 0
Expand All @@ -251,7 +268,11 @@ def __init__(self, sdo_client, index, subindex=0):
sdo_client.rx_cobid - 0x600)
request = bytearray(8)
SDO_STRUCT.pack_into(request, 0, REQUEST_UPLOAD, index, subindex)
response = sdo_client.request_response(request)
response = sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
res_command, res_index, res_subindex = SDO_STRUCT.unpack_from(response)
res_data = response[4:8]

Expand Down Expand Up @@ -301,13 +322,25 @@ def read(self, size=-1):
command |= self._toggle
request = bytearray(8)
request[0] = command
response = self.sdo_client.request_response(request)
response = self.sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
res_command, = struct.unpack_from("B", response)
if res_command & 0xE0 != RESPONSE_SEGMENT_UPLOAD:
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError(f"Unexpected response 0x{res_command:02X}")
if res_command & TOGGLE_BIT != self._toggle:
self.sdo_client.abort(ABORT_TOGGLE_NOT_ALTERNATED)
self.sdo_client.abort(
ABORT_TOGGLE_NOT_ALTERNATED,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError("Toggle bit mismatch")
length = 7 - ((res_command >> 1) & 0x7)
if res_command & NO_MORE_DATA:
Expand Down Expand Up @@ -348,6 +381,8 @@ def __init__(self, sdo_client, index, subindex=0, size=None, force_segment=False
:param bool force_segment:
Force use of segmented transfer regardless of size.
"""
self._index = index
self._subindex = subindex
self.sdo_client = sdo_client
self.size = size
self.pos = 0
Expand All @@ -363,10 +398,18 @@ def __init__(self, sdo_client, index, subindex=0, size=None, force_segment=False
command |= SIZE_SPECIFIED
struct.pack_into("<L", request, 4, size)
SDO_STRUCT.pack_into(request, 0, command, index, subindex)
response = sdo_client.request_response(request)
response = sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
res_command, = struct.unpack_from("B", response)
if res_command != RESPONSE_DOWNLOAD:
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError(
f"Unexpected response 0x{res_command:02X}")
else:
Expand All @@ -392,10 +435,18 @@ def write(self, b):
raise AssertionError("More data received than expected")
data = b.tobytes() if isinstance(b, memoryview) else b
request = self._exp_header + data.ljust(4, b"\x00")
response = self.sdo_client.request_response(request)
response = self.sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
res_command, = struct.unpack_from("B", response)
if res_command & 0xE0 != RESPONSE_DOWNLOAD:
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError(
f"Unexpected response 0x{res_command:02X}")
bytes_sent = len(b)
Expand All @@ -417,10 +468,18 @@ def write(self, b):
command |= (7 - bytes_sent) << 1
request[0] = command
request[1:bytes_sent + 1] = b[0:bytes_sent]
response = self.sdo_client.request_response(request)
response = self.sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
res_command, = struct.unpack("B", response[0:1])
if res_command & 0xE0 != RESPONSE_SEGMENT_DOWNLOAD:
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError(
f"Unexpected response 0x{res_command:02X} "
f"(expected 0x{RESPONSE_SEGMENT_DOWNLOAD:02X})")
Expand All @@ -442,7 +501,11 @@ def close(self):
command |= 7 << 1
request = bytearray(8)
request[0] = command
self.sdo_client.request_response(request)
self.sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
self._done = True

def writable(self):
Expand Down Expand Up @@ -473,6 +536,8 @@ def __init__(self, sdo_client, index, subindex=0, request_crc_support=True):
:param bool request_crc_support:
If crc calculation should be requested when using block transfer
"""
self._index = index
self._subindex = subindex
self._done = False
self.sdo_client = sdo_client
self.pos = 0
Expand All @@ -490,11 +555,19 @@ def __init__(self, sdo_client, index, subindex=0, request_crc_support=True):
command |= CRC_SUPPORTED
struct.pack_into("<BHBBB", request, 0,
command, index, subindex, self.blksize, 0)
response = sdo_client.request_response(request)
response = sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
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(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError(f"Unexpected response 0x{res_command:02X}")
# Check that the message is for us
if res_index != index or res_subindex != subindex:
Expand Down Expand Up @@ -551,7 +624,11 @@ def read(self, size=-1):
if self._done:
if self._server_crc != self._crc.final():
self._error = True
self.sdo_client.abort(ABORT_CRC_ERROR)
self.sdo_client.abort(
ABORT_CRC_ERROR,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError("CRC is not OK")
logger.info("CRC is OK")
self.pos += len(data)
Expand All @@ -571,7 +648,7 @@ def _retransmit(self):
self._ackseq = seqno
return response
self._error = True
self.sdo_client.abort(ABORT_TIMED_OUT)
self.sdo_client.abort(ABORT_TIMED_OUT, index=self._index, subindex=self._subindex)
raise SdoCommunicationError("Some data was lost and could not be retransmitted")

def _ack_block(self):
Expand All @@ -586,16 +663,24 @@ def _end_upload(self):
try:
response = self.sdo_client.read_response()
except SdoCommunicationError:
self.abort(ABORT_TIMED_OUT)
self.sdo_client.abort(ABORT_TIMED_OUT, index=self._index, subindex=self._subindex)
raise
res_command, self._server_crc = struct.unpack_from("<BH", response)
if res_command & 0xE0 != RESPONSE_BLOCK_UPLOAD:
self._error = True
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError(f"Unexpected response 0x{res_command:02X}")
if res_command & 0x3 != END_BLOCK_TRANSFER:
self._error = True
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError("Server did not end transfer as expected")
# Return number of bytes not used in last message
return (res_command >> 2) & 0x7
Expand Down Expand Up @@ -641,6 +726,8 @@ def __init__(self, sdo_client, index, subindex=0, size=None, request_crc_support
:param bool request_crc_support:
If crc calculation should be requested when using block transfer
"""
self._index = index
self._subindex = subindex
self.sdo_client = sdo_client
self.size = size
self.pos = 0
Expand All @@ -662,15 +749,23 @@ def __init__(self, sdo_client, index, subindex=0, size=None, request_crc_support
else:
logger.warning("Data size has not been specified")
SDO_STRUCT.pack_into(request, 0, command, index, subindex)
response = sdo_client.request_response(request)
response = sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
res_command, res_index, res_subindex = SDO_STRUCT.unpack_from(response)
if res_command & 0xE0 != RESPONSE_BLOCK_DOWNLOAD:
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError(
f"Unexpected response 0x{res_command:02X}")
# Check that the message is for us
if res_index != index or res_subindex != subindex:
self.sdo_client.abort()
self.sdo_client.abort(index=self._index, subindex=self._subindex)
raise SdoCommunicationError(
f"Node returned a value for {pretty_index(res_index, res_subindex)} instead, "
"maybe there is another SDO client communicating "
Expand Down Expand Up @@ -750,15 +845,23 @@ def _block_ack(self):
try:
response = self.sdo_client.read_response()
except SdoCommunicationError:
self.sdo_client.abort(ABORT_TIMED_OUT)
self.sdo_client.abort(ABORT_TIMED_OUT, index=self._index, subindex=self._subindex)
raise
res_command, ackseq, blksize = struct.unpack_from("BBB", response)
if res_command & 0xE0 != RESPONSE_BLOCK_DOWNLOAD:
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError(
f"Unexpected response 0x{res_command:02X}")
if res_command & 0x3 != BLOCK_TRANSFER_RESPONSE:
self.sdo_client.abort(ABORT_INVALID_COMMAND_SPECIFIER)
self.sdo_client.abort(
ABORT_INVALID_COMMAND_SPECIFIER,
index=self._index,
subindex=self._subindex,
)
raise SdoCommunicationError("Server did not respond with a "
"block download response")
if ackseq != self._blksize:
Expand Down Expand Up @@ -813,7 +916,11 @@ def close(self):
# Add CRC
struct.pack_into("<H", request, 1, self._crc.final())
logger.debug("Ending block transfer...")
response = self.sdo_client.request_response(request)
response = self.sdo_client.request_response(
request,
index=self._index,
subindex=self._subindex,
)
res_command, = struct.unpack_from("B", response)
if not res_command & END_BLOCK_TRANSFER:
raise SdoCommunicationError("Block download unsuccessful")
Expand Down
20 changes: 20 additions & 0 deletions doc/sdo.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,26 @@ server supports it. This is done through the file object interface::
Block transfer is still in experimental stage!


Client aborts
-------------

When a stream aborts a transfer, its abort frame includes the index and subindex
used to open that stream. For a manually constructed transfer, these can also be
passed explicitly::

node.sdo.abort(0x05040000, index=0x2345, subindex=0x67)

Direct calls to ``abort()`` or ``abort(code)`` continue to send zero address
fields. The same fallback applies to ``request_response(request)`` without
explicit ``index`` and ``subindex`` keyword arguments; the client does not infer
an address from arbitrary request payloads or remember the last stream's address.

Custom ``SdoClient`` subclasses overriding ``abort`` or ``request_response`` must
accept and forward the optional ``index`` and ``subindex`` keyword arguments used
by the stream classes. Overrides accepting only the older positional arguments
need to be updated before using those streams.


API
---

Expand Down
16 changes: 16 additions & 0 deletions test/test_sdo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,22 @@ def test_block_upload(self):
data = fp.read()
self.assertEqual(data, 'Tiny Node - Mega Domains !')

def test_block_upload_timeout_waiting_for_end_response(self):
self.data = [
(TX, b'\xa4\x08\x10\x00\x7f\x00\x00\x00'),
(RX, b'\xc4\x08\x10\x00\x00\x00\x00\x00'),
(TX, b'\xa3\x00\x00\x00\x00\x00\x00\x00'),
(RX, b'\x81Tiny No'),
(TX, b'\xa2\x01\x7f\x00\x00\x00\x00\x00'),
# The server never sends the end-of-block upload response.
(TX, b'\x80\x08\x10\x00\x00\x00\x04\x05'),
]
sdo = self.network[2].sdo[0x1008]
error = self.assertRaisesRegex(canopen.SdoCommunicationError, 'No SDO response received')
with sdo.open('rb', block_transfer=True) as fp, error:
fp.read()
self.assertEqual(self.data, [])

def test_sdo_block_upload_retransmit(self):
"""Trigger a retransmit by only validating a block partially."""
self.data = [
Expand Down
Loading