Switch structure using a volatile variable erroneously ignores a case

[rififi2063]

Hi,
I have a switch structure which runs different actions based on a volatile variable, which is updated by an interruption when a button (pin 3) is pressed.

The problem is that the action linked to the value 3 (WIPER_WASH, so the last before the "default:" in the code) actually never runs even if wiperState is 3 : the debug code before the switch statement correctly prints out "three" (so wiperState is actually 3), but the case linked to this value is never run. Putting a Serial.println("test"); inside the not working case shows it never runs since "test" is never printed out.

// Define constants
// Pins
#define BTN_PIN 3
#define SERVO_PIN 11
#define POT_PIN A0
#define LED_PIN 9
// Values
#define WIPER_OFF 0
#define WIPER_ON 1
#define WIPER_INTERMITTENT 2
#define WIPER_WASH 3
// Debug
#define debug 1
// Include libs
#include <Servo.h> // include the Servo lib

// Instanciate objects
Servo wiper;
// Declare global variables
// Volatile because used in an interruption
volatile int wiperState = WIPER_OFF;
int delayTime = 0;

void switchWiperState() {
  // Switches between states. OFF -> ON -> INTERMITTENT -> WASH -> OFF (reset
  // inside the loop function)
  // Autre option : array contenant les états, ou bien un if.
  wiperState = (wiperState+1);
}

void wiperCycle(int downDelay = 1000) {
  wiper.write(179);
  delay(1000);
  wiper.write(0);
  delay(downDelay);
}

void setup() {
  // Debug code
  #if debug
  Serial.begin(9600);
  #endif

  // Declare/attach pins
  pinMode(BTN_PIN, INPUT);
  pinMode(LED_PIN, OUTPUT);
  wiper.attach(SERVO_PIN);

  // Initialisation
  wiper.write(0);

  // Interruptions
  attachInterrupt(digitalPinToInterrupt(3), switchWiperState, RISING);
}

void loop() {
  if(wiperState > 3) {
    wiperState = 0;
  }

  #if debug
  if(wiperState == 0) {
    Serial.println("zero");
  }
  if(wiperState == 1) {
    Serial.println("one");
  }
  if(wiperState == 2) {
    Serial.println("two");
  }
  if(wiperState == 3) {
    Serial.println("three");
  }
  #endif
  switch(wiperState) {
    // when the wiper is off, move it down ant let it still
    case WIPER_OFF:
      wiper.write(0);
    break;

    // when the wiper is on, move it up and down successively
    case WIPER_ON:  
      wiperCycle();
    break;

    // when the wiper is in intermittent mode, move it up and down successively but
    // while waiting a delay time choosen based on the potentiometer value
    case WIPER_INTERMITTENT:
      int delayTime = map(analogRead(POT_PIN), 0, 1023, 1000, 3000);

      wiperCycle(delayTime);
    break;
 
     // when the wiper is in wash mode, we wash the windshield and then go back to off mode
    case WIPER_WASH:
      // turn on the wiper fluid
      digitalWrite(LED_PIN, HIGH);

      // wipe 5 times
      for(int i=0; i < 3; i++) {
        wiperCycle();
      }

      // turn off wiper fluid
      digitalWrite(LED_PIN, LOW);

      wiperState = 0;
      delay(0);
    break;


    default:
      wiperState = WIPER_OFF;
  }
}

And strangely, switching the order of the last case and the before-last case (so defining case WIPER_STATE before case WIPER_INTERMITTENT) makes it work perfectly, all the cases are correctly executed according to what the user asked.

I'm using an Arduino UNO R3, the one given in the student kit.

Thanks by advance for any clue,

[anon73444976]

@rififi2063

case WIPER_INTERMITTENT:
      int delayTime = map(analogRead(POT_PIN)

Didn't the compiler give you a warning here?

[madmark2150]

You can't declare a variable inside a SWITCH statement. Move all variable declarations out of the SWITCH/CASE and it will work.

int delayTime = map(analogRead(POT_PIN), 0, 1023, 1000, 3000); 
...
for(int i=0; i < 3; i++) { 

Are both causing issues

Define both i and delayTime outside the SWITCH and the problem will clear.

Apparently declaring a variable conditionally in a SWITCH confuses the runtime.

[rififi2063]

Warnings were disabled (yes, bad practice). After enabling them, I saw the cross initialisation, fixed it, and it worked.
Thanks

[anon73444976]

@madmark2150

You can't declare a variable inside a SWITCH statement.

You can, if you limit the scope to a single case.

[rififi2063]

As pointed by TheMemberFormerlyKnownAsAWOL the delaytime wasn't supposed to be declared there at all… I moved int i outside the switch too. Thanks to both of you.

[anon73444976]

@rififi2063

I moved int i outside the switch too

That was unnecessary - the scope of the i is limited.

[rififi2063]

It worked either way with the i outside or inside, so if there is no problem will put it back inside.

[madmark2150]

Whats the point of a single scope case? Just inline it as an IF.

[anon73444976]

@madmark2150

Whats the point of a single scope case?

I'm not sure what you're asking, but the fix is simple

  case WIPER_INTERMITTENT:
  {
      int delayTime = map(analogRead(POT_PIN), 0, 1023, 1000, 3000);

      wiperCycle(delayTime);
  }
  break;