Skip to content

fixed timeout issue for big sdo block transfers#36

Merged
wilkinsw merged 2 commits intoDaxbot:mainfrom
binarybox:main
Aug 9, 2022
Merged

fixed timeout issue for big sdo block transfers#36
wilkinsw merged 2 commits intoDaxbot:mainfrom
binarybox:main

Conversation

@binarybox
Copy link

Hello,
first of all, thank you for the code. I use it in a project, where we send a lot of small data.
At one point, we want to send big data (1MB) over the can. It works great, as long as there is nothing else on the network.
If there are other packages, the sdo block transfer will just stop and don't tell anything.

I figured out a way around this issue. The SDO block transfer is just trying to send as much as possible, even though the can connection is to full. But luckly the socketcan package helps us out of this.

Thats why we want the return value of the send method, it tells us if the message got transmitted succesfully.

On the other hand, I update the interval timeout, to give the can some time to recover after it filled up. For now this code seems to work fine, but It could run into issues if you have multiple sdo block transfers. What could happen then, is, it will give the Can channel more time to recover, because two messages try to send at the same time.
I was not sure what a good way is to track the download, so I did it in one variable. This can result in slower download when running multiple block transfers.

Let me know what you think about this and Cheers

@wilkinsw wilkinsw merged commit 7cbd243 into Daxbot:main Aug 9, 2022
@wilkinsw
Copy link
Member

wilkinsw commented Aug 9, 2022

Looks good, thanks for adding that!

@wilkinsw
Copy link
Member

wilkinsw commented Aug 9, 2022

Included in version 5.0.7

@wilkinsw wilkinsw added the enhancement New feature or request label Aug 9, 2022
@wilkinsw
Copy link
Member

wilkinsw commented Aug 9, 2022

I opened a new issue #37 to go back and take a look at multiple block transfers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

2 participants