For Commands with multiple Replies DHB::doSendRead does not read correct #405

New Issue

gaisser · 2021-04-19T18:16:53+02:00

gaisser commented

2021-04-19 18:16:53 +02:00

If a command has multiple replies with different reply ids, the code beneath will not get the right replyLen.

https://egit.irs.uni-stuttgart.de/fsfw/fsfw/src/branch/master/devicehandlers/DeviceHandlerBase.cpp#L593

	size_t requestLen = 0;
	if(cookieInfo.pendingCommand != deviceCommandMap.end()) {
		DeviceReplyIter iter = deviceReplyMap.find(
				cookieInfo.pendingCommand->first);
		if(iter != deviceReplyMap.end()) {
			requestLen = iter->second.replyLen;
		}
	}
    
	result = communicationInterface->requestReceiveMessage(comCookie, requestLen);

Possible soultions:

The most conservative Solution: Return back to the old way

   
	result = communicationInterface->requestReceiveMessage(comCookie, max_reply_length);

Add a hacky solution:

	size_t requestLen = max_reply_length;
	if(cookieInfo.pendingCommand != deviceCommandMap.end()) {
		DeviceReplyIter iter = deviceReplyMap.find(
				cookieInfo.pendingCommand->first);
		if(iter != deviceReplyMap.end()) {
			requestLen = iter->second.replyLen;
		}
	}
    
	result = communicationInterface->requestReceiveMessage(comCookie, requestLen);

This will break if a command has multiple replies and one of them has the same ID as the command!

Give a virtual protected function "virtual size_t getReplyLength(Command_id command)". The default implementation contains:

size_t getReplyLength(Command_id command){
            DeviceReplyIter iter = deviceReplyMap.find(
                        cookieInfo.pendingCommand->first);
                if(iter != deviceReplyMap.end()) {
                    return iter->second.replyLen;
                }else{
                    return 0;
               }
        }

The doSendRead function would then be:


	size_t requestLen = 0;
	if(cookieInfo.pendingCommand != deviceCommandMap.end()) {
		requestLen = getReplyLength(cookieInfo.pendingCommand->first);
	}
    
	result = communicationInterface->requestReceiveMessage(comCookie, requestLen);

User can then implement special cases for multiple answers.

My favorit is 3!

If a command has multiple replies with different reply ids, the code beneath will not get the right replyLen. https://egit.irs.uni-stuttgart.de/fsfw/fsfw/src/branch/master/devicehandlers/DeviceHandlerBase.cpp#L593 ``` c++ size_t requestLen = 0; if(cookieInfo.pendingCommand != deviceCommandMap.end()) { DeviceReplyIter iter = deviceReplyMap.find( cookieInfo.pendingCommand->first); if(iter != deviceReplyMap.end()) { requestLen = iter->second.replyLen; } } result = communicationInterface->requestReceiveMessage(comCookie, requestLen); ``` Possible soultions: 1. The most conservative Solution: *Return back to the old way* ``` c++ result = communicationInterface->requestReceiveMessage(comCookie, max_reply_length); ``` 2. Add a hacky solution: ``` c++ size_t requestLen = max_reply_length; if(cookieInfo.pendingCommand != deviceCommandMap.end()) { DeviceReplyIter iter = deviceReplyMap.find( cookieInfo.pendingCommand->first); if(iter != deviceReplyMap.end()) { requestLen = iter->second.replyLen; } } result = communicationInterface->requestReceiveMessage(comCookie, requestLen); ``` This will break if a command has multiple replies and one of them has the same ID as the command! 3. Give a virtual protected function "virtual size_t getReplyLength(Command_id command)". The default implementation contains: ``` size_t getReplyLength(Command_id command){ DeviceReplyIter iter = deviceReplyMap.find( cookieInfo.pendingCommand->first); if(iter != deviceReplyMap.end()) { return iter->second.replyLen; }else{ return 0; } } ``` The doSendRead function would then be: ``` c++ size_t requestLen = 0; if(cookieInfo.pendingCommand != deviceCommandMap.end()) { requestLen = getReplyLength(cookieInfo.pendingCommand->first); } result = communicationInterface->requestReceiveMessage(comCookie, requestLen); ``` User can then implement special cases for multiple answers. **My favorit is 3!**

gaisser added this to the ASTP 1.0.0 Local pools milestone 2021-04-19 18:16:57 +02:00

gaisser added the

bug

label 2021-04-19 18:16:59 +02:00

meierj was assigned by gaisser

2021-04-19 18:58:49 +02:00

gaisser added the

feature

label 2021-04-19 18:59:02 +02:00

muellerr commented

2021-04-20 18:11:25 +02:00

I like 3 as well.