Switch statements not working on some boards

I've written a library for sending short messages from one device to another (with addressing and checksums)

I have tested the library on an Adafruit Feather M0, sending data between two UART instances, and it works fine.

Now I am using it on an ATTiny 3227 with the MegaTinyCore, and a SAMD21 board with the Mattairtech core and the code has stopped working. In my update function it reads the latest serial byte and then has a switch statement with the various parser states for incoming data. This switch statement never executes even when valid data arrives. If I replace the switch with a series of if/else statements the code works.

I'm at a complete loss as to why the code would fail like this on two different boards when it works fine on a third.

The update function from my class is below, it prints out incoming data to confirm that the data is arriving but the switch statement doesn't execute. A commented out if/else statement is there and when I uncomment it I get ">Start2" printed out, indicating that the first byte of incoming data is accepted by the parser.

I'm not even close to the flash or ram limits of any of the devices I am using.

This is not the first time I have had relatively simple switch statements fail on Arduino boards and I am not getting any compiler warnings (with verbose messages on)

Am I missing some subtle bug here?


bool MicroMessage::update(){
	//returns true of a new packet was accepted - including if the packet has failed the checksum
	bool returnState = false;
	int inByte;
	//Data available? If not then check the timeout
	if(port->available()){
		inByte = port->read();
	}else{
		checkTimeout();
		//timeout will reset the parser
		return returnState;
	}
	
	#ifdef MESSAGE_TIMING_DEBUG
		commRxStart = micros();
	#endif
	
	#ifdef PRINT_BYTES_IN
		//Incoming data is printed here just fine!

		Serial.print(inByte);
		Serial.print(" ");
	#endif
	/*
	//THESE IF/ELSE STATEMENTS WORK FINE 

	if(commRXState == COMM_IDLE){
		if(inByte == PACKET_START_A) commRXState = READING_START2;
	}else if(commRXState == READING_START2){
		#ifdef PARSER_DEBUG
				debug.println(">Start2");
		#endif
		if(inByte == PACKET_START_B) commRXState = READING_START3;
		else commRXState = COMM_IDLE;
	}*/
	
	//THIS SWITCH STATEMENT WILL NOT EXECUTE ON SOME BOARDS
	
	switch(commRXState){
		case COMM_IDLE:
			if(inByte == PACKET_START_A) commRXState = READING_START2;
			break;
		case READING_START2:
			#ifdef PARSER_DEBUG
				debug.println(">Start2");
			#endif
			if(inByte == PACKET_START_B) commRXState = READING_START3;
			else commRXState = COMM_IDLE;
			break;
		case READING_START3:
			#ifdef PARSER_DEBUG
				debug.println(">Start3");
			#endif
			if(inByte == PACKET_START_A) commRXState = READING_START4;
			else commRXState = COMM_IDLE;
			break;
		case READING_START4:
			#ifdef PARSER_DEBUG
				debug.println(">Start4");
			#endif
			if(inByte == PACKET_START_B) commRXState = READING_GROUP_ID;
			else commRXState = COMM_IDLE;
			break;
		case READING_GROUP_ID:
			#ifdef PARSER_DEBUG
				debug.println(">group");
			#endif
			if( checkGroup((uint8_t)inByte) ){
				crc.add( (uint8_t)inByte );
				commRXState = READING_NODE_ID;
			}
			else resetBuffer(RESET_REJECTED_GROUP);
			break;
		case READING_NODE_ID:
			#ifdef PARSER_DEBUG
				debug.println(">node");
			#endif
			if( checkNode((uint8_t)inByte) ){
				crc.add( (uint8_t)inByte );
				commRXState = READING_COMMAND;
			}
			else resetBuffer(RESET_REJECTED_NODE);
			break;
		case READING_COMMAND:
			#ifdef PARSER_DEBUG
				debug.println(">command");
			#endif
			commandIn = (uint8_t)inByte;
			crc.add(commandIn);
			commRXState = READING_PACKET_LENGTH;
			break;
		case READING_PACKET_LENGTH:
			#ifdef PARSER_DEBUG
				debug.println(">packet length");
			#endif
			packetLengthIn = (uint8_t)inByte;
			crc.add(packetLengthIn);
			if(packetLengthIn == 0) commRXState = READING_CHECKSUM;
			else commRXState = READING_DATA;
			break;
		case READING_DATA:
			#ifdef PARSER_DEBUG
				debug.println(">data");
			#endif
			if(addToBuffer((uint8_t)inByte)) commRXState = READING_CHECKSUM;
			break;
		case READING_CHECKSUM:
			#ifdef PARSER_DEBUG
				debug.println(">checksum");
			#endif
			checksumIn = (uint8_t)inByte;
			checksumOKFlag = verifyChecksum();
			resetBuffer(RESET_DATA_OK);
			#ifdef MESSAGE_TIMING_DEBUG
				commRxEnd = micros();
				
				MESSAGE_TIMING_DEBUG debug.print("RxStart ");
				MESSAGE_TIMING_DEBUG debug.print(commRxStart);
				MESSAGE_TIMING_DEBUG debug.print(" RxEnd ");
				MESSAGE_TIMING_DEBUG debug.print(commRxEnd);
				MESSAGE_TIMING_DEBUG debug.print(" elapsed ");
				MESSAGE_TIMING_DEBUG debug.println(commRxEnd - commRxStart);
			#endif
			returnState = true;
			break;
		default:
			#ifdef PARSER_DEBUG
				debug.println("UNKNOWN STATE");
			#endif
			commRXState = COMM_IDLE;
			break;
	}
  return returnState;
}

Please provide all the code, however long.

very likely your bug is not where you are looking at or showing us.

make a small compileable example which shows that error. The chances are high you will find the bug on your own. If not - post a full compileable example.

1 Like

what does MESSAGE_TIMING_DEBUG look like? curious about that part

#ifdef MESSAGE_TIMING_DEBUG
      commRxEnd = micros();

      MESSAGE_TIMING_DEBUG debug.print("RxStart ");
      MESSAGE_TIMING_DEBUG debug.print(commRxStart);
      MESSAGE_TIMING_DEBUG debug.print(" RxEnd ");
      MESSAGE_TIMING_DEBUG debug.print(commRxEnd);
      MESSAGE_TIMING_DEBUG debug.print(" elapsed ");
      MESSAGE_TIMING_DEBUG debug.println(commRxEnd - commRxStart);
#endif

(if a macro defines a local variable outside a compound statement block in a switch, then you have a issue with the switch/case and are out of spec)

2 Likes

MESSAGE_TIMING_DEBUG:
Yeah, thats a really stupid error on my part. This is what it should be like, but correcting it has no impact on the problem.

#ifdef MESSAGE_TIMING_DEBUG
      commRxEnd = micros();

      debug.print("RxStart ");
      debug.print(commRxStart);
      debug.print(" RxEnd ");
      debug.print(commRxEnd);
      debug.print(" elapsed ");
      debug.println(commRxEnd - commRxStart);
#endif

is it just a

#define MESSAGE_TIMING_DEBUG

that you comment out or is there some value attached to the #define?

a quick test to see if there is a compound statement issue would be to add pairs of {} everywhere in the cases

  switch (commRXState) {
    case COMM_IDLE: {
        if (inByte == PACKET_START_A) commRXState = READING_START2;
        break;
      }

    case READING_START2: {
#ifdef PARSER_DEBUG
        debug.println(">Start2");
#endif
        if (inByte == PACKET_START_B) commRXState = READING_START3;
        else commRXState = COMM_IDLE;
        break;
      }

    case READING_START3: {
#ifdef PARSER_DEBUG
        debug.println(">Start3");
#endif
        if (inByte == PACKET_START_A) commRXState = READING_START4;
        else commRXState = COMM_IDLE;
        break;
      }

Yes, this is what usually happens when I run out of ideas and ask for help on a forum - I suddenly find that the real problem was somewhere else!
This method checks to see if too much time has elapsed between bytes when recieving a message, and resets if the delay was too long.

There are occasional gaps between bytes of just over 10ms which I can't explain. There don't appear to be any delays when transmitting ...


void  MicroMessage::checkTimeout(commRXState_t st){
	//If there is no data
	unsigned long currentTime = micros();
	if(((currentTime - lastTime) < timeoutInterval)) return;
	
	if(st > COMM_IDLE) {
		timeoutErrors++;
		#ifdef TIMEOUT_DEBUG
			debug.print("Message Timeout:");
			//debug.print("Timeout:");
			debug.println((currentTime - lastTime));
			//debug.print((uint8_t)commRXState);
			//debug.println(" in parser");
		#endif
		#ifdef MESSAGE_TIMING_DEBUG
				commRxEnd = micros();
				debug.print("RxStart ");
				debug.print(commRxStart);
				debug.print(" RxEnd ");
				debug.print(commRxEnd);
				debug.print(" elapsed ");
				debug.println(commRxEnd - commRxStart);
			#endif
		resetBuffer(RESET_TIMEOUT);
	} else  resetBuffer(RESET_NORMAL);
	
  lastTime = micros();
}

That sounds close to Rubber duck debugging - Wikipedia

1 Like

Yup, and it seems to have worked. I've now ironed out all the bugs and the code is working really nicely.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.