Switch statement error

[codium]

Posted on May 15, 2014 at 18:09

Hi

I just can't understand this... So, I'm communicating with PC's terminal program via USART2 on my STM32F10. Communication is running smoothly both ways, but I have a problem when I want my embedded system to respond appropriately to PC's commands. Basically, when ''I'' receive 9 characters, I read the first character and optionally what's behind, but mostly just 1st character matters. So, if I send for example 9 x m (RxBuffer = ''mmmmmmmmm'') or any combination of 'm' and ''whatever follows'', it seems to work fine, and the LED goes off. Similarly with 'c' (LED goes ON).  But, if I send i.e. ''t01100100'' or ''ttttttttt'', switch interprets it as 'm'. o_O Well, screenshot says it all. Thanks for taking time :)

[Tesla DeLorean]

Posted on May 15, 2014 at 18:18

The break is on the for, not the case

You will need a break on the case to stop it falling through to the next one, which is default switch/case behaviour defined by the language.

[codium]

Posted on May 15, 2014 at 18:25

Thanks for noticing, howerer it doesn't solve it. The behaviour is completely the same.

[Tesla DeLorean]

Posted on May 15, 2014 at 18:43

Post the code, I can't edit text in graphical format....

[codium]

Posted on May 16, 2014 at 09:59

void react_on_cmd(void)       //reaction to receiving a command through USART

{  uint8_t i;    switch(RxBuffer[0]) {   case 'c': {     mode = calibration;    break;   }   case 't': {       for (i = 1; i == sizeof(active_sensor_table); i++){     active_sensor_table[i] = RxBuffer[i];    }   break;  }   case 'm': {    mode = measurement;    break;   }      //case 's': start measurement...   //RxCounter = 0;   default: {    strcpy(TxBuffer, ''\nCommunication error'');      //send RST indicator to PC    TxCounter = 0;    USART_ITConfig(USART2 , USART_IT_TXE , ENABLE);   }  }     if(mode) STM32vldiscovery_LEDOn(LED3);  else STM32vldiscovery_LEDOff(LED3); } Is this sufficient or you need other parts of program?

[Kraal]

Posted on May 16, 2014 at 10:37

Hi !

One thing first, don't put braces around case statements, i.e.switch(expression){ case 1: ... break; case 2: ... break; default: ... break;}Second thing, in case't', the for loop will never execute, unless active_sensor_table has only one element. It should be defined as(for i = 1; i < sizeof(active_sensor_table); i++){...}Then if your table contains words, since sizeof(active_sensor_table) will give you the size in bytes, you'll have a runaway pointer for sure...Have a nice day

[codium]

Posted on May 16, 2014 at 11:59

Wau, thanks, i think those brackets were the issue (gotta relearn the syntax i guess :))! I also repaired the sizeof() - countof = (sizeof (a)/sizeof(*a)).

Take care

void react_on_cmd(void)       //reaction to receiving a command through USART

{  uint8_t i;    switch(RxBuffer[0]) {   case 'c':      mode = calibration;    break;      case 't':        for (i = 0; i <= (countof(active_sensor_table)-1); i++){     active_sensor_table[i] = RxBuffer[i+1];    }    break;     case 'm':    mode = measurement;    break;         //case 's': start measurement...   //RxCounter = 0;   default:    strcpy(TxBuffer, ''\nCommunication error'');      //send RST indicator to PC    TxCounter = 0;    USART_ITConfig(USART2 , USART_IT_TXE , ENABLE);   }     if(mode) STM32vldiscovery_LEDOn(LED3);  else STM32vldiscovery_LEDOff(LED3); }

[frankmeyer9]

Posted on May 16, 2014 at 12:31

Wau, thanks, i think those brackets were the issue (gotta relearn the syntax i guess :))!

Actually, the practice of putting case branches into brackets, is perfectly legal and often seen (albeit I strongly dislike it, too). It usually makes no sense - except to create a new context for declaring local variables.

[Kraal]

Posted on May 16, 2014 at 14:18

Ah, since I have never seen this before, I assumed it was not correct. As you (fm) said, it is only meaningful for handling very local variable (and of course I also (wrongly) assumed that each case had its own context).