A switch/case error. absolutely no clue

The below code is some tinkering I am doing on this thread

but I open a new topic because my simulation (work in progress) started to behave oddly.

You will see an enum, where in desperation I assigned the values that would be automatically assigned.

You will see a switch/case with a duplicate case label (2, or want1).

You will get the expected error if you verify or try to run the code in the wokwi.

You will see a /* block */ of seemingly innocuous code in one of the cases, the uncommenting of which will result in the case label (2, or want1) no longer being flagged as a duplicate.

Then you will please explain that. :expressionless:

Here it is in the wokwi, that is the quickest way to see the problem

and here is the code, a certainly hacked up hack of the original. Just in case you still don't want to click away to the wokwi. I did test this with "verify" in the IDE v. 1.8.7. Same same.

#include <Servo.h>

typedef enum  {NoNewCard = 0, NoData, Match, NoMatch} RFIDState;

RFIDState rfidCheck()
{
  unsigned char fake = random(4);

  switch (fake) {
    case 0 :
      Serial.println("RFID check <NoNewCard>");
      return NoNewCard;
      
    case 1 :
      Serial.println("RFID check <NoData>");
      return NoData;
      
    case 3 :
      Serial.println("RFID check <Match>");
      return Match;
      
    case 2 :
      Serial.println("RFID check <NoMatch>");
      return NoMatch;
  }

  return fake;
}

 Servo fingers;
 Servo thumb;
 Servo wrist;
 int angle=0;
 
 
 typedef enum
 {
  initialState = 0,
  wantCheckrfid = 1,
  want1 = 2,              //Fistclench,
  wantFistrelax = 3,
  want2 = 4,              //Thumbsupclench,
  wantThumbsuprelax = 5,
 } states;

// state machine variables
states state = initialState;
unsigned long lastStateChange = 0;
unsigned long timeInThisState = 1000;
 
 void setup() {

  Serial.begin(115200);
  Serial.println("stripped of everything! zeroing servos");

  fingers.attach(4);
  thumb.attach(3);
  wrist.attach(2);

  fingers.write(0);
  thumb.write(0);
  wrist.write(0);

  delay(777);

 //wrist2.attach(10)
 //pinktip.attach(9);
 //pinkybase.attach(8);
 //ringtip.attach(7);
 //ringbase.attach(6);
 //middletip.attach(5);
 //middlebase.attach(4);
 //indextip.attach(3);
 //indexbase.attach(2);

 }

void doStateChange ()
{
  lastStateChange = millis ();
  timeInThisState = 1000;

Serial.print("           doStateChange state ");
Serial.println(state);

  switch (state)
  {
    case 2:  // duplicate of want1
 
    case initialState:
         state = wantCheckrfid;
         break;

  case wantCheckrfid:


// if I uncomment the section below, case 2 above is no longer an error

/*    RFIDState rfidNow = rfidCheck();

//    if (rfidNow == NoNewCard) return;
 
//    if (rfidNow == NoData) return;

    if (rfidNow == Match) {
Serial.print("wantCheckrfid: fingers "); Serial.print(20);
Serial.print(" thumb "); Serial.println(30);
      fingers.write(20);
      thumb.write(30);
      delay (500);
Serial.print(" and fingers "); Serial.print(0);
Serial.print(" thumb "); Serial.println(0);
      fingers.write(0);
      thumb.write(0);
    }
 */

 // rfidNow is NoMatch or fell through from Match

    state = random(2)? want1 : want2;
    timeInThisState = random (1000)+ 2000;
 
Serial.print(" state "); Serial.println(state);
Serial.print(" for time ");Serial.println(timeInThisState);

    break;


    case want1:
Serial.print("want1: fingers "); Serial.print(120);
Serial.print(" thumb "); Serial.println(80);
    fingers.write(120);
    thumb.write(80);
    state = wantFistrelax;
    timeInThisState = 2000;
    break;

    case wantFistrelax:
Serial.print("wantFistrelax: fingers "); Serial.print(0);
Serial.print(" thumb "); Serial.println(0);
    fingers.write(0);
    thumb.write(0);
    state = wantCheckrfid;
    timeInThisState = 3000;
    break;

    case want2:
Serial.print("want2: fingers "); Serial.print(120);
Serial.print(" wrist "); Serial.println(90);
    fingers.write(120);
    wrist.write(90);
    state = wantThumbsuprelax;
    timeInThisState = 2000;
    break;

    case wantThumbsuprelax:
Serial.print("wantThumbsuprelax: fingers "); Serial.print(0);
Serial.print(" wrist "); Serial.println(0);
    fingers.write(0);
    wrist.write(0);
    state = wantCheckrfid;
    timeInThisState = 3000;
    break;
  }
 }
  
 void loop() {
  if (millis () - lastStateChange >= timeInThisState) 
    doStateChange ();
}

I do so hope that I will have wasted your time because of something dumb I am missing.

a7

The compiler is telling you exactly what's wrong.

Refer to the line numbers given in the error report;


wtgdaf.ino: In function 'void doStateChange()':
wtgdaf.ino:129:5: error: duplicate case value
     case want1:
     ^~~~
wtgdaf.ino:88:5: note: previously used here
     case 2:  // duplicate of want1
     ^~~~

Copy and run in the IDE the short sketch at post #6 in this thread - Timer counter state change toggle FSM - all at once - and examine the results.

Yes it is.

That's what I meant by the expected error. I expected that error. I got that error. I know what that error message means.

But:

Un-commenting the block of code that is /* commented out */ makes the error go away. The compiler no longer sees an error. Without the literal case 2, a switch value of 2 does not seem to run any case… which is where I started tearing out what is left of my hair.

Lines 99 - 116 in the wokwi listing.

a7

I suspect this is your problem line.

You should avoid declaring variables within a switch/case statement block, unless you enclose the case code within { }.

Either remove the declaration to before the switch/case statement, or add {} to the code the within caseWantCheckrfid:

The issue is to do with variable scope. You will see compiler warnings about this (if you have warnings turned on).

/var/folders/4z/3g9th3gs4cng3b803dg5bhw40000gn/T/arduino_modified_sketch_985433/Blink.ino:129:10: warning: jump to case label [-fpermissive]
     case want1:
          ^~~~~
/var/folders/4z/3g9th3gs4cng3b803dg5bhw40000gn/T/arduino_modified_sketch_985433/Blink.ino:99:21: note:   crosses initialization of 'RFIDState rfidNow'
           RFIDState rfidNow = rfidCheck();
                     ^~~~~~~

If you make the changes as above the original compiler error will reappear.

Well TBH I would see the compiler warnings if I read them.

Thank you, I have scheduled myself for a few slaps with a damp trout as required by forum rules. Or internet rules anyway.

Sry to any who wasted time on this. All the ways of fixing the code so it breaks mentioned by @red_car do indeed the trick.

But:

With the code uncommented and the variable declared where it was, there was no error, no warning. Just a malfunctioning switch/case statement. Which seems like a giant ditch waiting for, well, anyone.

I'll have to add to the list "don't do that thing with variables declared in switch/case statements that compile without error or warning but totally mess with the proper functioning".

Duplicate case labels or no. The only reason I started tinkering with more case labels is that my cases weren't getting executed. Too very odd.

Also I find that playing with this with the online GDB C++ compiler I can get warnings that the Arduino "all" does not emit - at least

main.cpp:18:12: warning: enumeration value ‘BAKER’ not handled in switch [-Wswitch]

I haven't the energy left just now to go further with GDB to see if I can get a similar compiles without warning program that results in mysterious behavior.

a7

Totally missed that. Sorry.

Took a long time for me to find this out ...

If you uncomment this line, the cases after this case become unreachable because they would jump over the initialization of a local variable. Since the problem case is no longer reachable, it is no longer an error.

Thanks. I can't get this to be the case in a simple example. In GDB online C, trying to minimally make a case unreachable, therefor allowing what would have been a duplicate case label, does not produce the same (mis)behavior. My small experiments there all seem to just work as if the entire case with the declaration was in braces. So far.

Imma throw in the towel on this and just keep an eye out to not do that thing. Seems to have varying effect (or not) depending on… IDK.

a7

Compiler error reporting is often less than perfect. With many compiler errors, the first error will be the only significant one, and many/most/all subsequent ones will be simply a side-effect of the compiler getting "out of sync" with the code, and throwing spurious errors. This is simply the nature of single-pass compilers. I ALWAYS fix the first error, then look at a few of the subsequent ones, and if I see no obvious problems, simply re-compile. That second compile will, more often than not, succeed. It is equally imporant to ALWAYS look at ALL warnings.

Couldn't agree better with all you point out. The problem here was code that would malfunction without a whisper of a warning or error.

Much like the compiler can't be expected to catch more mundane errors. I'm used to that, believe me.

So I was chasing not a missed warning or error, be it the first or not - it was my program was going wonky for no (at the time) good reason.

Investigating in my inimitable fashion just turned up seriously baffling problems, sometimes errors and warnings, other time just pure caprice. And it is not consistent, that is to say I have given up trying to predict anything in the case of using a local declaration in a switch case, other than don't or have it appear in a braced code block.

It has been 'splained. I will see how long it takes me to forget this gain in knowledge. :expressionless:

a7

I believe if you look, you will find you DID get a warning about the variable defined within a case, which can and will create undefined behavior. I have never seen the compiler fail to flag that issue.

"the" compiler, maybe. I don't have the years on it you do, and don't much use declarations like that. And thinking about it, placing a declaration in the code for a case is prolly something I never did before.

On my desktop, cc -Wall -Wextra -Wshadow makes no error or warning and runs correctly with a declaration like this

    case 1 :
        if (0) printf("just a statement for the label\n");
        unsigned char oops = functionq();
        

The online GDB online C++ compiler issues no error or warning if I leave off the initialiser, be it a function call or a constant. With a function call or assignment I get

main.cpp: In function ‘void theFSM()’:
main.cpp:48:10: error: jump to case label
48 | case 2 :
| ^
main.cpp:40:23: note: crosses initialization of ‘unsigned char oops’
40 | unsigned char oops = 37;

so I can certainly believe that whatever is compiling for the IDE does a third thing, that is to always throw some red ink.

And I will (have already) admit that I can blink and miss the red ink of warnings. Please don't point out the scroll bar. :expressionless:

Thank you (all) for your attention to this matter.

a7

Most switch cases have a
break;
Beween each case

Let us know if that helps!

lol :rofl: