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).