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

Are you able to prove that the function is executed? can you put a breakpoint in it and prove that the function called from that line (54) if you can then that pretty much wraps it up for the compiler.

If I take optimisations off then it will be executed, thought for some reason debugger shows me only null data when stepping in there, and it does not actually send any reasonable data to the function call.(SendMediaEvent)

If I let the compiler ​optimize then I can see correct data with debugger which comes from my car canbus, but can't set break point on that line and it's never executed.

Sadly the arm assembler code is bit different than 8-bit avr, and i can't say that I completely understand ​it, but it seems some of the if/if else statements does get optimised out aswell, which would result in the behaviour im currently seeing with the parser.

Now with the getMappedButton function fixed, it does detect normal presses and long presses, but it likes to send every command multiple times forward, and even if it did detect long press, it will always execute command for short press before that​. I believe it's should work correctly now if the compiler was not so aggressively optimising stuff, since when it removes some of the if/else statements the parser goes to wrong states at wrong times.

I think i just have to start throwing volatile qualifier to every variable until I find the one compiler thinks can't change at all, and thus optimising too much out.​