Skip to content

Commit

Permalink
fix SDO block download process continuity after sequence breaks (CANo…
Browse files Browse the repository at this point in the history
…penNode#171)

Now additional state CO_SDO_ST_DOWNLOAD_BL_SUB_RESP_2 is used to send response without resetting SDO sequence.
Refactoring timeout halding logic in sub-block transfer.
Also add missed unsigned indicators for several constants.

issue CANopenNode#170
  • Loading branch information
lukegluke authored Mar 13, 2020
1 parent 03613cc commit b92b4bb
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 32 deletions.
48 changes: 29 additions & 19 deletions stack/CO_SDO.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ static void CO_SDO_receive(void *object, const CO_CANrxMsg_t *msg){
SDO->CANrxData[0] = msg->data[0];
seqno = SDO->CANrxData[0] & 0x7fU;
SDO->timeoutTimer = 0;
/* clear timeout in sub-block transfer indication if set before */
if (SDO->timeoutSubblockDownolad)
SDO->timeoutSubblockDownolad = false;

/* check correct sequence number. */
if(seqno == (SDO->sequence + 1U)) {
Expand All @@ -220,7 +223,7 @@ static void CO_SDO_receive(void *object, const CO_CANrxMsg_t *msg){
}
}

/* break reception if last segment or block sequence is too large */
/* break reception if last segment, block ends or block sequence is too large */
if(((SDO->CANrxData[0] & 0x80U) == 0x80U) || (SDO->sequence >= SDO->blksize)) {
SDO->state = CO_SDO_ST_DOWNLOAD_BL_SUB_RESP;
SET_CANrxNew(SDO->CANrxNew);
Expand All @@ -230,8 +233,8 @@ static void CO_SDO_receive(void *object, const CO_CANrxMsg_t *msg){
/* Ignore message, if it is duplicate or if sequence didn't started yet. */
}
else {
/* seqno is totally wrong, break reception. */
SDO->state = CO_SDO_ST_DOWNLOAD_BL_SUB_RESP;
/* seqno is wrong, send response without resetting sequence */
SDO->state = CO_SDO_ST_DOWNLOAD_BL_SUB_RESP_2;
SET_CANrxNew(SDO->CANrxNew);
}
}
Expand Down Expand Up @@ -771,7 +774,6 @@ int8_t CO_SDO_process(
uint16_t *timerNext_ms)
{
CO_SDO_state_t state = CO_SDO_ST_IDLE;
bool_t timeoutSubblockDownolad = false;
bool_t sendResponse = false;

/* return if idle */
Expand Down Expand Up @@ -869,9 +871,12 @@ int8_t CO_SDO_process(
SDO->timeoutTimer += timeDifference_ms;
}
if(SDO->timeoutTimer >= SDOtimeoutTime){
if((SDO->state == CO_SDO_ST_DOWNLOAD_BL_SUBBLOCK) && (SDO->sequence != 0) && (!SDO->CANtxBuff->bufferFull)){
timeoutSubblockDownolad = true;
state = CO_SDO_ST_DOWNLOAD_BL_SUB_RESP;
if((SDO->state == CO_SDO_ST_DOWNLOAD_BL_SUBBLOCK) && (!SDO->timeoutSubblockDownolad) && (!SDO->CANtxBuff->bufferFull)){
/* set indication timeout in sub-block transfer and reset timeout */
SDO->timeoutSubblockDownolad = true;
SDO->timeoutTimer = 0;
/* send response without resetting sequence */
state = CO_SDO_ST_DOWNLOAD_BL_SUB_RESP_2;
}
else{
CO_SDO_abort(SDO, CO_SDO_AB_TIMEOUT); /* SDO protocol timed out */
Expand Down Expand Up @@ -939,8 +944,8 @@ int8_t CO_SDO_process(
return -1;
}
}
SDO->bufferOffset = 0;
SDO->sequence = 0;
SDO->bufferOffset = 0U;
SDO->sequence = 0U;
SDO->state = CO_SDO_ST_DOWNLOAD_SEGMENTED;
sendResponse = true;
}
Expand Down Expand Up @@ -980,7 +985,7 @@ int8_t CO_SDO_process(
}

SDO->ODF_arg.dataLength = CO_SDO_BUFFER_SIZE;
SDO->bufferOffset = 0;
SDO->bufferOffset = 0U;
}
}

Expand Down Expand Up @@ -1042,8 +1047,9 @@ int8_t CO_SDO_process(
}
}

SDO->bufferOffset = 0;
SDO->sequence = 0;
SDO->bufferOffset = 0U;
SDO->sequence = 0U;
SDO->timeoutSubblockDownolad = false;
SDO->state = CO_SDO_ST_DOWNLOAD_BL_SUBBLOCK;

/* send response */
Expand All @@ -1056,15 +1062,19 @@ int8_t CO_SDO_process(
break;
}

case CO_SDO_ST_DOWNLOAD_BL_SUB_RESP:{
/* no new message received, SDO timeout occured, try to response */
lastSegmentInSubblock = (!timeoutSubblockDownolad &&
case CO_SDO_ST_DOWNLOAD_BL_SUB_RESP:
case CO_SDO_ST_DOWNLOAD_BL_SUB_RESP_2:{
/* check if last segment received */
lastSegmentInSubblock = (!SDO->timeoutSubblockDownolad &&
((SDO->CANrxData[0] & 0x80U) == 0x80U)) ? true : false;

/* prepare response */
SDO->CANtxBuff->data[0] = 0xA2;
SDO->CANtxBuff->data[1] = SDO->sequence;
SDO->sequence = 0;

/* reset sequence on reception break */
if (state == CO_SDO_ST_DOWNLOAD_BL_SUB_RESP)
SDO->sequence = 0U;

/* empty buffer in domain data type if not last segment */
if((SDO->ODF_arg.ODdataStorage == 0) && (SDO->bufferOffset != 0) && !lastSegmentInSubblock){
Expand All @@ -1082,7 +1092,7 @@ int8_t CO_SDO_process(
}

SDO->ODF_arg.dataLength = CO_SDO_BUFFER_SIZE;
SDO->bufferOffset = 0;
SDO->bufferOffset = 0U;
}

/* blksize */
Expand Down Expand Up @@ -1306,8 +1316,8 @@ int8_t CO_SDO_process(
return -1;
}

SDO->bufferOffset = 0;
SDO->sequence = 0;
SDO->bufferOffset = 0U;
SDO->sequence = 0U;
SDO->endOfTransfer = false;
CLEAR_CANrxNew(SDO->CANrxNew);
SDO->state = CO_SDO_ST_UPLOAD_BL_SUBBLOCK;
Expand Down
29 changes: 16 additions & 13 deletions stack/CO_SDO.h
Original file line number Diff line number Diff line change
Expand Up @@ -452,19 +452,20 @@ typedef enum{
* Internal states of the SDO server state machine
*/
typedef enum {
CO_SDO_ST_IDLE = 0x00U,
CO_SDO_ST_DOWNLOAD_INITIATE = 0x11U,
CO_SDO_ST_DOWNLOAD_SEGMENTED = 0x12U,
CO_SDO_ST_DOWNLOAD_BL_INITIATE = 0x14U,
CO_SDO_ST_DOWNLOAD_BL_SUBBLOCK = 0x15U,
CO_SDO_ST_DOWNLOAD_BL_SUB_RESP = 0x16U,
CO_SDO_ST_DOWNLOAD_BL_END = 0x17U,
CO_SDO_ST_UPLOAD_INITIATE = 0x21U,
CO_SDO_ST_UPLOAD_SEGMENTED = 0x22U,
CO_SDO_ST_UPLOAD_BL_INITIATE = 0x24U,
CO_SDO_ST_UPLOAD_BL_INITIATE_2 = 0x25U,
CO_SDO_ST_UPLOAD_BL_SUBBLOCK = 0x26U,
CO_SDO_ST_UPLOAD_BL_END = 0x27U
CO_SDO_ST_IDLE = 0x00U,
CO_SDO_ST_DOWNLOAD_INITIATE = 0x11U,
CO_SDO_ST_DOWNLOAD_SEGMENTED = 0x12U,
CO_SDO_ST_DOWNLOAD_BL_INITIATE = 0x14U,
CO_SDO_ST_DOWNLOAD_BL_SUBBLOCK = 0x15U,
CO_SDO_ST_DOWNLOAD_BL_SUB_RESP = 0x16U,
CO_SDO_ST_DOWNLOAD_BL_SUB_RESP_2 = 0x17U,
CO_SDO_ST_DOWNLOAD_BL_END = 0x18U,
CO_SDO_ST_UPLOAD_INITIATE = 0x21U,
CO_SDO_ST_UPLOAD_SEGMENTED = 0x22U,
CO_SDO_ST_UPLOAD_BL_INITIATE = 0x24U,
CO_SDO_ST_UPLOAD_BL_INITIATE_2 = 0x25U,
CO_SDO_ST_UPLOAD_BL_SUBBLOCK = 0x26U,
CO_SDO_ST_UPLOAD_BL_END = 0x27U
} CO_SDO_state_t;


Expand Down Expand Up @@ -623,6 +624,8 @@ typedef struct{
uint16_t crc;
/** Length of data in the last segment in block upload */
uint8_t lastLen;
/** Indication timeout in sub-block transfer */
bool_t timeoutSubblockDownolad;
/** Indication end of block transfer */
bool_t endOfTransfer;
/** Variable indicates, if new SDO message received from CAN bus */
Expand Down

0 comments on commit b92b4bb

Please sign in to comment.