cancel
Showing results for 
Search instead for 
Did you mean: 

"Warning, this statement is never executed", need help finding the reason for it.

JoniS
Senior

Hi, Since i could not find "general coding" subforum i will post here, even when i dont use TrueStudio. but visualGDB and arm gcc toolchain.

so the proplem is here, i'm trying to parse Steering wheel button Can codes from my cars canbus, to detect if it was long pressed or just tapped, to get some more functionality from the couple buttons i have. I have tested this prototype many months IRL with my daily car and all the other aspects are working great.

void __attribute__((optimize("O0"))) AdvancedMappedMessageParser(CanMessage *message, uint32_t time) {
		
	uint32_t timeStamp = time;
	uint64_t currentPayload;
	static uint32_t lastValidMsg = 0;
	static CanParserState state = Idle;
	static CanMessage lastMsg;
	static volatile uint32_t processingStarted = 0;
	
	/* Recover from possible deadlock */
	if (message == NULL && state != Idle && time - processingStarted > 1000) {
		state = Idle;
		lastValidMsg = 0;
		processingStarted = 0;
		return;
	} 
	if (message == NULL)
		return;
		
	
	currentPayload = message->getPayload();
	
	switch (state) {
		case Idle:
		if (IsPayloadMapped(currentPayload)) {
			/* save timestamp from the receive of the message */
			processingStarted = message->getTimeStamp();	
			/* Check if long press is mapped for the payload, so we know if further processing is needed */
			if (IsLongPressMapped(currentPayload))
			{			
				state = ProcessKeyPress;
				lastMsg = *message;	
			}
			else
			{
				/* No long press mapped for payload, execute now and move to wait for release or timeout */
				SendMediaEvent((getMappedButton(message, false))->action);	
				state = Release;
			}					
		}	
		break;
		
		case ProcessKeyPress:
		/* Check if we got same message, it indicates we might have long press */
		if (currentPayload == lastMsg.getPayload()) {
			if ((timeStamp - processingStarted) > HardwareConfig.ButtonSettings.LongPressDelay) {
				SendMediaEvent((getMappedButton(message, true))->action);
				/* Action cant repeat & long press, so we just go to release state now*/
				state = Release;
			}				
		}
		/* check if button was released, if it was send event and go to release state */
		else if(IsReleaseMessage(message)) {
				SendMediaEvent((getMappedButton(&lastMsg, false))->action);	<< this line of code			
				state = Release;
		}
		/* Check the payload against other button mappings */
		else if(IsPayloadMapped(currentPayload)) {		
			SendMediaEvent((getMappedButton(&lastMsg, false))->action);								
			lastMsg = *message;
			processingStarted = message->getTimeStamp();
			if (!IsLongPressMapped(currentPayload)) {
				SendMediaEvent((getMappedButton(&lastMsg, false))->action);	
				state = Release;
			}
		}	
		
		break;
		case Release:
		if (IsReleaseMessage(message) && timeStamp - processingStarted > HardwareConfig.ButtonSettings.ReleaseDelay) {
			state = Idle;
		}
			
		if (lastMsg.getPayload() == currentPayload) {
			if (getMappedButton(message, false)->AltAction == ButtonActionType::Repeat) {
				if (timeStamp - processingStarted > HardwareConfig.ButtonSettings.RepeatActionDelay) {
					SendMediaEvent((getMappedButton(message, false))->action);	
					processingStarted = timeStamp;
				}
					
			}		
		}
		else if(IsPayloadMapped(currentPayload)) {									
			lastMsg = *message;
			processingStarted = message->getTimeStamp();
			if (!IsLongPressMapped(currentPayload)) {
				SendMediaEvent((getMappedButton(&lastMsg, false))->action);	
				state = Release;
			}
		}	
				
		break;
						
		default:
		break;
	}	
	
	lastMsg = *message;
	lastValidMsg = timeStamp;
	return;
}

I have looked at the code for hours and at this point i'm too frustrated to find out what's causing that if else statement to be something that cant be reached, maybe someone else could spot the proplem with fresh eyes.

i have had countless iterations of the parser, but just cant get long presses and normal presses to work as expected, only long presses cool it works, only short presses and it works again. but for reason or another when i try get both at the same time, i either make some silly mistake or the compiler thinks it knows better and optimise's something out.

22 REPLIES 22
Korporal
Senior

You haven't told us which line is generating the compiler warning.

/* check if button was released, if it was send event and go to release state */
		else if(IsReleaseMessage(message)) {
				SendMediaEvent((getMappedButton(&lastMsg, false))->action);	<< this line of code			
				state = Release;
		}

It's the first line inside that if else, so I believe for some reason compiler thinks the if else can never be true, or reached.​

Korporal
Senior
	case ProcessKeyPress:
		/* Check if we got same message, it indicates we might have long press */
		if (currentPayload == lastMsg.getPayload()) 
		{
			if ((timeStamp - processingStarted) > HardwareConfig.ButtonSettings.LongPressDelay) 
			{
				mp = getMappedButton(message, true);
				SendMediaEvent(mp->action);
				/* Action cant repeat & long press, so we just go to release state now*/
				state = Release;
			}
		}

break out the nested function call like this, then lets see if it whines about line 7 or 8 (using above numbering).

Korporal
Senior

Sorry I copied the wrong part, but basically extract the nested function call getMappedButton and make it a separate function call as I did above, do that in all places for consistency and lets see how the compiler then reacts.

Currently I do have hardcoded "button map" which for sure contains the release code, and the function(IsReleaseMessage) is working as expected if I took it separate to test. ​

Korporal
Senior

Are any of those apparent function calls actually macros?

How does getPayload() change in function for lastMsg vs currentPayload?

Same function, same data, compiler might assume the answer is the same.

Tips, Buy me a coffee, or three.. PayPal Venmo
Up vote any posts that you find helpful, it shows what's working..
Korporal
Senior

Hmmm

S.Ma
Principal

Sometime the error is earlier than detected. You can anyway check the generated assembly and try to put breakpoint to make sure there is code in the clicked code path.

Still I feel unconfortable with this:

if (message == NULL && state != Idle && time - processingStarted > 1000) {

overwrapping won't hurt.