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

[JoniS]

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.

[JoniS]

Mainloop feeds the parser from circular buffer which gets it's data from can rx interrupt.

believe it should warn about other if's and if else's statements aswell, if it did not figure that the data is coming from outside the mainloop.

[Tesla DeLorean]

No, look to be function calls via pointers in the structure.

Not clear to me how geyPayload() determines the object it is being associated with, and I'm assuming this doesn't have C++ code.

So my point would be that the code before the switch, and in the case cause if to look like

x = y;

..

if (x == y)

{

} else if (not happening)

{

}

[JoniS]

It's all funtion calls, no macros used.

getMappedButton returns pointer to typedef struct MappedButton_t which contains (uint8_t)action, action is the actual hid keycode to send over ble link.

[JoniS]

typedef struct MappedButton_t {
	/* Lower bytes of Can message payload */
	uint32_t PayloadLowerNibble = 0;
	/* Upper bytes of Can message payload */
	uint32_t PayloadUpperNibble = 0;
	/* Can message Identifier */
	uint16_t Identifier = 0;
	/* User defined media action*/
	HID::MediaKeys action = HID::MediaKeys::None;
	/* Does this button support Long Press */
	ButtonActionType AltAction = ButtonActionType::Normal; 
		
} MappedButton_t;

uint64_t CanMessage::getPayload() return the databytes of the can frame(CanMessage is c++ class object), getMappedButton() function checks the assosiated Payload versus array of MappedButton_t's.

MappedButton_t* getMappedButton(CanMessage *msg, bool longpress) {
	
	/* Use to index the key mapping, if user did long press, but only "short" press is mapped */
	int8_t NoLongPressMapped = -1;
	
	for (int i = 0; i < 8; i++) {
		if (ButtonMap[i].PayloadUpperNibble == msg->getPayloadUpperNipple() &&
				ButtonMap[i].PayloadLowerNibble == msg->getPayloadLowNipple()) {
			/* Check if long press action enabled incase user did longpress */
			if (longpress && ButtonMap[i].AltAction == ButtonActionType::LongPress)
				return &ButtonMap[i];
			else 
				NoLongPressMapped = i;
		}
	}
		/* Check if we were looking for long press action, but no long press action is mapped for the Steering button */
		if (NoLongPressMapped != -1) 
			return &ButtonMap[NoLongPressMapped];	
	
		/* return null if no mapped button is found */
		/* Caller is responsible for null checking */
		return NULL;			
}

[Tesla DeLorean]

Perhaps you could use standard bisection methods to pin-point the test the compiler is hung up on?

[JoniS]

x = y;
 
..
 
if (x == y)
 
{
 
} else if (not happening)
 
{
 
}

That is incorrect, you would need X, Y and Z

Somefunction(variable z)
 
....
static Y;
 
X=Z
 
If(X == Y)
 
If else(we can get here)
 
..
..
Y = X;

Or well, that's how it intended to be. LastMsg always holds the message from last iteration of the loop, ​so we have something to compare against on next iteration, which should let me identify long press if that happens.

[JoniS]

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

Now it should quarantee the order of execution that is intended.

[JoniS]

I did try to unwrap all the functions calls and the "Warning, This statement is never executed" stays in the same place, so it made no difference.

[JoniS]

Okay to make it more interesting in my opinion.

else if(IsReleaseMessage(message)) {
		>>		SendMediaEvent((getMappedButton(&lastMsg, false))->action);	<<			
				state = Release;
		}

Compiler does optimise out only one line of code which is the one its crying for

SendMediaEvent((getMappedButton(&lastMsg, false))->action);				

Only that gets optimised away, the if else statement is there and working 100% correctly with test dataset, even the "state = Release" gets executed when debugging every line one after another.

When i calculate it, there are 5 identical function calls like that in the parser different states, only one gets optimised away with no obvious reason.

[JoniS]

One bug killed

/* Use to index the key mapping, if user did long press, but only "short" press is mapped */
	uint8_t NoLongPressMapped = 100;
	
	for (uint8_t i = 0; i < 8; i++) {
		if ((ButtonMap[i].PayloadUpperNibble == msg->getPayloadUpperNipple()) && (ButtonMap[i].PayloadLowerNibble == msg->getPayloadLowNipple())) {
			/* Check if long press action enabled incase user did longpress */
			if (longpress && ButtonMap[i].AltAction == ButtonActionType::LongPress)
				return &ButtonMap[i];
			else if(ButtonMap[i].AltAction != ButtonActionType::LongPress)
				NoLongPressMapped = i;
		}
	}
		/* Check if we were looking for long press action, but no long press action is mapped for the Steering button */
		if (NoLongPressMapped != 100) 
			return &ButtonMap[NoLongPressMapped];	
	
		/* return null if no mapped button is found */
		/* Caller is responsible for null checking */
		return NULL;

i did assign new value to "NoLongPressMapped" even when the actual mapping had alternative action of LongPress, since it was never checked -.-

if (longpress && ButtonMap[i].AltAction == ButtonActionType::LongPress)
				return &ButtonMap[i];
			else 
				NoLongPressMapped = i;
 
...
...
if (NoLongPressMapped != -1) 
			return &ButtonMap[NoLongPressMapped];

yes, hitting my head to desk right now. Sadly this did not fix the more urgent issue. But now i might actually to be able to detect between long and short presses =)

(Obviously this first implementation, always returned the Mapped key which was later on the array, not one based if we seek long press mapping or not)