Switch - case Doubt

Hello Guys.

I'm trying to use Switch - case structure for change parts of my code from one state to another.
I'm using UDP protocol for communication with my server. Whem my server sends me a value non zero I change the variable estado for another value, in this case showed is to 1.

But for some reason that I could understand my switch-case seams not to work propely. It doesn't get in the switch after the value modification.

The output from my Serial is showed below as it is the code. Can someone help me trying to find where I'm mystaking?

17:26:04.713 -> Entrei no case wait
17:26:05.688 -> Estado: 0
17:26:05.688 -> Entrei no case wait
17:26:06.700 -> Estado: 0
17:26:06.700 -> Entrei no case wait
17:26:07.692 -> Estado: 0
17:26:07.724 -> Entrei no case wait
17:26:08.710 -> Estado: 0
17:26:08.710 -> Entrei no case wait
17:26:09.717 -> Estado: 1
17:26:10.689 -> Estado: 1
17:26:11.698 -> Estado: 1
17:26:12.693 -> Estado: 1
17:26:13.702 -> Estado: 1
17:26:14.719 -> Estado: 1
17:26:15.696 -> Estado: 1
#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>
#include <Wire.h>

// Defindo IPs
IPAddress ip(192,168,1,92);
IPAddress serverIP(192,168,1,2);
int localPort = 5005;
byte mac[] = {0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xDD};

EthernetUDP udp; // Object used in communication protocol

// =========================================================================================
// SETUP
// =========================================================================================
void setup() {
  // Your setup code here
  Serial.begin(9600); // start serial
  Ethernet.begin(mac, ip); // start ethernet

  if (!Serial) {Serial.println("not Serial.");}
  if (udp.begin(localPort)) {Serial.println("UDP Protocol initialized!");}
}
// =========================================================================================
// Variaveis Loop
int estado;
enum estado {waitForCommand, movement, stop};
// =========================================================================================
// LOOP FUNCTION
// =========================================================================================
void loop() {
  Serial.print("Estado: ");
  Serial.println(estado);

  float defasagem;
  switch (estado) {
    case 0:
      Serial.println("Entrei no case wait");
      int packetSize =  udp.parsePacket();
      char packetBuffer[packetSize];
      newData(packetSize, packetBuffer);
      defasagem = atof(packetBuffer);
      clearBuffer(packetSize, packetBuffer);
      if (defasagem > 0.00){
        estado = 1;
      }
      else{
        estado = 0;
      }
      break;

    case 1:
      Serial.println("Entrei no case movement");
      // do something with StepperMotors
      estado = 1;
      break;
    
    case 2:
      Serial.println("Entrei no case stop");
      // wait for continue messsage
      estado = 0;
      break;
  }
  
  udp.beginPacket(serverIP, localPort);
  udp.write("COMOK");
  udp.endPacket();
  delay(1000);
}
// =========================================================================================
// FUNCTIONS:
// =========================================================================================
void newData(int packetSize, char packetBuffer[]){
  if (packetSize>0){
    udp.read(packetBuffer, packetSize);
    udp.beginPacket(serverIP, localPort);
    udp.write("OK");
    udp.endPacket();
  }
  packetBuffer[packetSize] = (char)0;
}

void clearBuffer(int &packetSize, char packetBuffer[]){
  for(int i =0; i<packetSize; i++){
    packetBuffer[i] = (char)0;
  }
  packetSize = 0;
}
1 Like

Just complementing my question, I try to declare the estado variable only as an int and I atribute the initial value as 0, and the behaviour was the same. And for finishing I used if else statement and it works, but to the code stay more beatiful and easy to ready if I use Switch-case. So It is it.

In transit looking through the tiniest of windows, this looks odd

    case 1:
      Serial.println("Entrei no case movement");
      // do something with StepperMotors
      estado = 1;
      break;

HTH

a7

I suspect that having a variable and an enum with the same name is causing a problem.

Maybe this should be

char packetBuffer[packetSize+1];

so that it has an extra index to hold the null terminator character. Without that extra index,

this line writes to an array index that is outside the array, which can have strange effects.

2 Likes

I tryied without the enum too. And the behaviour was the same. And I have another example here where my friend used enum too, and in his case works fine. I tryed to use the same structure that he used. But somehow it didn't work.

Thanks for your answer.

you can't define new variables directly in the switch case unless you make a compound statement

if you compile with warnings on, you probably saw something like

warning: jump to case label

so packetSize and packetBuffer can't be there as they are in

    case 0:
      Serial.println("Entrei no case wait");
      int packetSize =  udp.parsePacket();
      char packetBuffer[packetSize];
      newData(packetSize, packetBuffer);
      defasagem = atof(packetBuffer);
      clearBuffer(packetSize, packetBuffer);
      if (defasagem > 0.00){
        estado = 1;
      }
      else{
        estado = 0;
      }
      break;

you need to do

    case 0:
      { // <<==== START A COMPOUND STATEMENT SO THAT THE VARIABLES ARE LOCAL TO THE STATEMENT
        Serial.println("Entrei no case wait");
        int packetSize =  udp.parsePacket();
        char packetBuffer[packetSize];
        newData(packetSize, packetBuffer);
        defasagem = atof(packetBuffer);
        clearBuffer(packetSize, packetBuffer);
        if (defasagem > 0.00) {
          estado = 1;
        }
        else {
          estado = 0;
        }
      }
      break;

side note :
that's not great to have the type and variable with the same name

and you should use the enum

  switch (estado) {
    case waitForCommand:
      break;

    case movement:
      break;

    case stop:
      break;
  }
2 Likes

I didn't now that. Thank you for your answer. Now it's working perfectly.

And thanks for that too. I decide not to use enum and just an int estado variable.

My code stay like this one below:

#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>
#include <Wire.h>

// Defindo IPs
IPAddress ip(192,168,1,92);
IPAddress serverIP(192,168,1,2);
int localPort = 5005;
byte mac[] = {0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xDD};

EthernetUDP udp; // Object used in communication protocol

// =========================================================================================
// SETUP
// =========================================================================================
void setup() {
  // Your setup code here
  Serial.begin(9600); // start serial
  Ethernet.begin(mac, ip); // start ethernet

  if (!Serial) {Serial.println("not Serial.");}
  if (udp.begin(localPort)) {Serial.println("UDP Protocol initialized!");}
}
// =========================================================================================
// Variaveis Loop
int estado = 0;
// =========================================================================================
// LOOP FUNCTION
// =========================================================================================
void loop() {
  Serial.print("Estado: ");
  Serial.println(estado);

  float defasagem;
  switch (estado) {
    case 0:
      {
        Serial.println("Entrei no case wait");
        int packetSize =  udp.parsePacket();
        char packetBuffer[packetSize];
        newValueData(packetSize, packetBuffer);
        defasagem = atof(packetBuffer);
        clearBuffer(packetSize, packetBuffer);
        if (defasagem > 0.00){
          estado = 1;
        }
        else{
          estado = 0;
        }
      }
      break;

    case 1:
      {
        Serial.println("Entrei no case movement");
        // do something with StepperMotors
        sendData("MOVEOK");
        delay(100);
        estado = 2;
      }
      break;
    
    case 2:
      {
        Serial.println("Entrei no case stop");
        // wait for continue messsage
        int packetSize =  udp.parsePacket();
        char packetBuffer[packetSize];
        newValueData(packetSize, packetBuffer);
        defasagem = atof(packetBuffer);
        if (defasagem == 1000.00){
          Serial.println("Voltando o movimento.");
          estado = 1;
        }
        else {
          Serial.println("Remain stopped");
          estado = 2;
        }
        clearBuffer(packetSize, packetBuffer);
      }
      break;
  }
  sendData("COMOK");
  delay(1000);
}
// =========================================================================================
// FUNCTIONS:
// =========================================================================================
void newValueData(int packetSize, char packetBuffer[]){
  if (packetSize>0){
    udp.read(packetBuffer, packetSize);
    sendData("OK");
  }
  packetBuffer[packetSize] = (char)0;
}

void clearBuffer(int &packetSize, char packetBuffer[]){
  for(int i =0; i<packetSize; i++){
    packetBuffer[i] = (char)0;
  }
  packetSize = 0;
}

void sendData(char data[]){
  udp.beginPacket(serverIP, localPort);
  udp.write(data);
  udp.endPacket();
}

Looks like you fixed your other mistake, too.

a7

It stays like this including the error? Why did you ask for help on this forum?

Keep calm my friend. Sorry if I didn't pay attention to your answer. And thank you very much for your effort. I change my code to the correctly one, your advice apparently wasn't doing nothing wrong in my code, but as you said can happen strange effects.

The new code is below and again thank you @PaulRB .
Also @alto777 thank you.

#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>
#include <Wire.h>

// Defindo IPs
IPAddress ip(192,168,1,92);
IPAddress serverIP(192,168,1,2);
int localPort = 5005;
byte mac[] = {0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xDD};

EthernetUDP udp; // Object used in communication protocol

// =========================================================================================
// SETUP
// =========================================================================================
void setup() {
  // Your setup code here
  Serial.begin(9600); // start serial
  Ethernet.begin(mac, ip); // start ethernet

  if (!Serial) {Serial.println("not Serial.");}
  if (udp.begin(localPort)) {Serial.println("UDP Protocol initialized!");}
}
// =========================================================================================
// Variaveis Loop
int estado = 0;
// =========================================================================================
// LOOP FUNCTION
// =========================================================================================
void loop() {
  Serial.print("Estado: ");
  Serial.println(estado);

  float defasagem;
  switch (estado) {
    case 0:
      {
        Serial.println("Entrei no case wait");
        int packetSize =  udp.parsePacket();
        char packetBuffer[packetSize+1];
        newValueData(packetSize, packetBuffer);
        defasagem = atof(packetBuffer);
        clearBuffer(packetSize, packetBuffer);
        if (defasagem > 0.00){
          estado = 1;
        }
        else{
          estado = 0;
        }
      }
      break;

    case 1:
      {
        Serial.println("Entrei no case movement");
        // do something with StepperMotors
        sendData("MOVEOK");
        delay(100);
        estado = 2;
      }
      break;
    
    case 2:
      {
        Serial.println("Entrei no case stop");
        // wait for continue messsage
        int packetSize =  udp.parsePacket();
        char packetBuffer[packetSize+1];
        newValueData(packetSize, packetBuffer);
        defasagem = atof(packetBuffer);
        if (defasagem == 1000.00){
          Serial.println("Voltando o movimento.");
          estado = 1;
        }
        else {
          Serial.println("Remain stopped");
          estado = 2;
        }
        clearBuffer(packetSize, packetBuffer);
      }
      break;
  }
  sendData("COMOK");
  delay(1000);
}
// =========================================================================================
// FUNCTIONS:
// =========================================================================================
void newValueData(int packetSize, char packetBuffer[]){
  if (packetSize>0){
    udp.read(packetBuffer, packetSize);
    sendData("OK");
  }
  packetBuffer[packetSize] = (char)0;
}

void clearBuffer(int &packetSize, char packetBuffer[]){
  for(int i=0; i<=packetSize; i++){
    packetBuffer[i] = (char)0;
  }
  packetSize = 0;
}
1 Like

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