msftrncs / canopen_sdosvr_st Goto Github PK

The assignment of Request_BUFFEROVERRUN (line 120) should be moved before line 117, and then its result utilized in the IF's condition.

The expression i := i + 1 can be moved before the indexed array operation to reduce the expression of the index to just i.

While I have not experienced an issue with this in my applications, it was noticed that, CANSDO_IsEnum is not cleared if the application directly utilizes the action TX_ACK_EXP_DN to acknowledge the expedited download, instead of using action DispatchResponse. CANSDO_IsEnum may then remain set when the next request is processed, and in most cases this only results in muting of errors if the next request results in such an error.

The code is being reviewed further to see if there are further situations where this may occur.

The expression for Request_BUFFEROVERRUN may never be set because i will not exceed Response_BUFFER_LEN when the WHILE loop exits upon detecting that the supplied buffer has been filled to capacity.

The correction requires rearranging lines (382-384):

		(* if request length was known, then its a problem if there was more data than buffer, but not if length was not known *)
		Request_BUFFEROVERRUN := SEL( Request_LEN_KNOWN, FALSE, Request_LEN > Response_BUFFER_LEN );
		IF Request_BUFFEROVERRUN THEN
			Request_LEN := i; (* length of data downloaded *)
		END_IF

Line 151 seems redundant.

If an object post processor does not otherwise provide a response to a request, the the SDO SVR Protocol Processor will automatically generate an ERR_INV_OBJ (invalid object ID) response, and this response cannot be prevented.

This is a problem when operating in a mixed environment where another SDO server may be responsible for providing some objects. While this is NOT an ideal situation, because the two SDO servers will not be aware of each other, it should still be possible for them to coexist mostly peacefully.

I am considering to add an action that will cause a non-response to a request.

I am also considering changing the default behavior so that the application must explicitly request the ERR_INV_OBJ response, instead of it being automatic, and then a non-response action will not be necessary. (The {PRIVATE} attribute will need to be removed from TX_ERR_INV_OBJ action then.

Can be simplified:

CANSDO_DATA[0] := SEL( Response_BUFFER_LEN = 0, 16#43 + SHL( 4 - DWORD_TO_BYTE( MIN( Response_BUFFER_LEN, DWORD#4 ) ), 2 ), 16#42 );