Variable Block Download block sizes #126
Comments
|
Thank you for your contribution! I will check your changes and add any change if we improve the conformity to the standard. I guess the calculation of the block size of last block seems to be reasonable. One question (just for my info): Is your host program setting the c field in the last message? |
|
Thank you. I tried to conform to your coding style and to the CiA standard. Feel free to change it as you wish to keep all files consistent. I am glad I could help in some way. Sorry that my original post did not parse the code snippet properly. I will try to fix that if I have any other contributions. Yes, my host does set the c field when it is the last message. However, it takes the block size as an absolute must. So it will send as many messages as indicated (with 0 data bytes if needed) then set the c field on the final message. The Stack would send an abort code if it received a message with 0 data bytes. This caused all downloads to be aborted. |
|
Ok, thanks for your explanation. I think we are improving the conformity with your changes. When I have some time, I will write an integration test for this behavior and fix it with your collaboration. Thanks again. Btw: to simplify code changes, you can fork the repository and create a pull-request. This way we can chat on diffs and have some support by the system ;-) |
Hello Michael,
I have been porting your canopen-stack to a project I am working on. Thank you for creating this project and making it open for others to use.
One aspect that I have been working with recently is the Block download process. My project is replacing a product that is already operating in the field and is required to be a "Drop In replacement". So I am testing it with a host system that cannot change.
The Code follows the protocol in CiA 301 Version 4.2.0 CANopen Application layer and communication profile paragraph 7.2.4.3.9 and 7.2.4.3.10. However the block size (blksize) does not change when the next block does not require the maximum 127 messages. In this scenario the client is the host interacting with this stack. This stack is the server side and is receiving large blocks of data.
Lets say the host is sending 1024 bytes of data. With each message capable of sending 7 bytes of data, this transfer will require 147 messages. 146 messages using all 7 bytes of data and the last using only 2 bytes. The block download is initiated according to 7.2.4.3.9 and the block size returned is 0x7F (127). Once the block of data is complete the response is sent according to 7.2.4.3.10 but the block size returned is 0x7F (127). I believe it should be 0x14 (147 - 127 = 20).
This may be a spec interpretation mistake or oversight on the part of my existing host but like I mentioned, this product must be compatible with the host as is. 7.2.4.3.10 does specify the c field which can indicate that the message is the last one. I believe that if this is set your stack will correctly finish the download. My host seems to take the block size must be the size dictated by the server and continued sending messages without any data (to send the complete 127 messages). The stack will abort the transfer if it receives a messages without any data therefore I could not get a successful block download completed.
I was able to fix this issue by making slight changes to co_ssdo.c. I am currently still testing with version 4.3.1.
the changes I made are outlined below:
Line 646 in co_ssdo.c
*/ *** New Function to calculate the block size ***
only used locally to calculate the number of blocks to request
if More bytes than can fit in the max block size are remaining
return the max block size
else calculate the number of remaining messages required
return as last block size
*/
static uint32_t COSdoBlockSizeRequest(uint32_t SizeRemaining, uint32_t MaxSize)
{
uint32_t PartialBlockSize;
if (SizeRemaining >= (MaxSize * 7))
return MaxSize;
PartialBlockSize = SizeRemaining / 7;
if (SizeRemaining % 7)
PartialBlockSize++;
return PartialBlockSize;
}
/* ** Minor changes **
see function definition
*/
CO_ERR COSdoInitDownloadBlock(CO_SDO srv)
{
CO_ERR result = CO_ERR_SDO_ABORT;
uint32_t width = 0;
uint32_t size;
uint32_t SegmentCount; / New line */
uint8_t cmd;
cmd = CO_GET_BYTE(srv->Frm, 0);
if ((cmd & 0x02) != 0) {
width = CO_GET_LONG(srv->Frm, 4);
}
size = COSdoGetSize(srv, width, false);
if (size> 0) {
SegmentCount = COSdoBlockSizeRequest(size, CO_SDO_BUF_SEG); /* New line /
srv->Blk.State = BLK_DOWNLOAD;
srv->Blk.SegNum = SegmentCount; / Changed line */
srv->Blk.SegCnt = 0;
srv->Blk.SegOk = 0;
srv->Blk.Size = size;
srv->Blk.Len = size;
srv->Buf.Cur = srv->Buf.Start;
srv->Buf.Num = 0;
}
return (result);
}
/* ** No change **
see function definition
*/
CO_ERR COSdoEndDownloadBlock(CO_SDO *srv)
{
CO_ERR result = CO_ERR_SDO_ABORT;
uint32_t len;
uint8_t cmd;
uint8_t n;
cmd = CO_GET_BYTE(srv->Frm, 0);
if ((cmd & 0x01) != 0) {
n = (cmd & 0x1C) >> 2;
len = ((uint32_t)srv->Buf.Num - n);
if (srv->Obj->Type != 0) {
/* write at current offset for typed object entry /
result = COObjWrBufCont(srv->Obj, srv->Node, srv->Buf.Start, len);
} else if (len <= 4) {
/ write value for basic type entry */
result = COObjWrValue(srv->Obj, srv->Node, srv->Buf.Start, len, srv->Node->NodeId);
}
if (result != CO_ERR_NONE) {
srv->Node->Error = CO_ERR_SDO_WRITE;
COSdoAbort(srv, CO_SDO_ERR_TOS);
result = CO_ERR_SDO_ABORT;
}
CO_SET_BYTE(srv->Frm, 0xA1, 0);
CO_SET_WORD(srv->Frm, 0, 1);
CO_SET_BYTE(srv->Frm, 0, 3);
CO_SET_LONG(srv->Frm, 0, 4);
}
return (result);
}
/* ** Minor Changes **
see function definition
*/
CO_ERR COSdoDownloadBlock(CO_SDO srv)
{
CO_ERR result = CO_ERR_SDO_SILENT;
CO_ERR err;
uint32_t len;
uint32_t SegmentCount; / New line */
uint8_t cmd;
uint8_t i;
cmd = CO_GET_BYTE(srv->Frm, 0);
if ((cmd & 0x7F) == (srv->Blk.SegCnt + 1)) {
/* check, that we need at least 1 byte out of the payload */
if (srv->Blk.Len > 0) {
for (i = 0; i < 7; i++) {
(srv->Buf.Cur) = CO_GET_BYTE(srv->Frm, 1 + i);
srv->Buf.Cur++;
srv->Buf.Num++;
if (srv->Blk.Len > 0) {
srv->Blk.Len--;
}
}
} else {
srv->Blk.State = BLK_IDLE;
srv->Buf.Cur = srv->Buf.Start;
srv->Buf.Num = 0;
srv->Obj = 0;
COSdoAbort(srv, CO_SDO_ERR_LEN_HIGH);
result = CO_ERR_SDO_ABORT;
return (result);
}
srv->Blk.SegCnt++;
if ((srv->Blk.SegCnt == srv->Blk.SegNum) || / Changed line */
((cmd & 0x80) != 0 )) {
} else {
if ((srv->Blk.SegCnt & 0x80) == 0) {
srv->Blk.SegCnt |= 0x80;
}
}
return (result);
}
The choice is yours if you would like to merge these changes into your main project. I am putting them out there incase anyone else runs into a similar issue. I noted the changes above with comments for reference above. I have also attached the modified file.
Thank you.
Bob Haefner
co_ssdo.txt
The text was updated successfully, but these errors were encountered: