Skip to content

Conversation

@samsamfire
Copy link
Contributor

I am submitting a pull request to add retransmission functionnality for sdo block download.
I faced a hardware issue (silicon errata) on a DSP where in some very rare cases the CAN rx fifo would swap the reception of two or more CAN frames causing the block transfer to fail, this was a nice solution to overcome the problem.

Copy link
Member

@christiansandberg christiansandberg left a comment

Choose a reason for hiding this comment

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

Thanks for this PR!

"Will start retransmission") % (ackseq, self._blksize))
# Sub blocks betwen ackseq and end of corrupted block need to be resent
# Copy block to resend, and clear it so that multiple retransmits can be done
block = self._current_block[ackseq:].copy()
Copy link
Member

Choose a reason for hiding this comment

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

I believe just subindexing a list will return a copy already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you are absolutely right !

"block download response")
if ackseq != self._blksize:
# Sequence error, try to retransmit
self.sdo_client.abort(0x05040003)
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite remember the spec, is this abort code still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
This is taken from the spec and apparently an SDO abort is only necessary in case of a fatal error, so I will remove it.
However it still worked with this abort message, so we probably have a bug on our side.

Copy link
Member

Choose a reason for hiding this comment

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

No it's not very common for clients to send abort codes to the server, so some servers may just ignore them.

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 to know.

@christiansandberg christiansandberg merged commit e3829fc into canopen-python:master Aug 8, 2022
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