Go Down

Topic: Switch case: always default case (Read 547 times) previous topic - next topic

engachou

Hi,

I'm trying to program some buttons and LEDs so that when a button is pressed, only the corresponding LED is turned on. But when I run this code, whenever any button is pressed, only the default case executes.

Code: [Select]

int buttonLED1 = 5;  //pin that LED 1 is connected to, etc
int buttonLED2 = 6;
int buttonLED3 = 7;
int buttonLED4 = 8;
int button1 = 1;  //pin that button 1 is connected to, etc
int button2 = 2;
int button3 = 3;
int button4 = 4;

int LEDcase;

void setup() {
  pinMode(buttonLED1, OUTPUT);
  pinMode(buttonLED2, OUTPUT);
  pinMode(buttonLED3, OUTPUT);
  pinMode(buttonLED4, OUTPUT);

  pinMode(button1, INPUT);
  digitalWrite(button1, HIGH);
  pinMode(button2, INPUT);
  digitalWrite(button2, HIGH);
  pinMode(button3, INPUT);
  digitalWrite(button3, HIGH);
  pinMode(button4, INPUT);
  digitalWrite(button4, HIGH);
}

void loop() {

   if (digitalRead(button2)==LOW) {
    LEDcase==2;
      whichLED(LEDcase);
  }

  if (digitalRead(button1)==LOW) {
    LEDcase==1;
      whichLED(LEDcase);
  }
 
  if (digitalRead(button3)==LOW) {
    LEDcase==3;
      whichLED(LEDcase);
  }
 
  if (digitalRead(button4)==LOW) {
    LEDcase==4;
      whichLED(LEDcase);
  }
 
}

void useLED(int l1, int l2, int l3, int l4) {
  if (l1 == 0) {
    digitalWrite(buttonLED1, LOW);
    }
    else {
      digitalWrite(buttonLED1, HIGH);
    }
  if (l2 == 0) {
    digitalWrite(buttonLED2, LOW);
    }
    else {
      digitalWrite(buttonLED2, HIGH);
    }
  if (l3 == 0) {
    digitalWrite(buttonLED3, LOW);
    }
    else {
      digitalWrite(buttonLED3, HIGH);
    }
  if (l4 == 0) {
    digitalWrite(buttonLED4, LOW);
    }
    else {
      digitalWrite(buttonLED4, HIGH);
    }
}

void whichLED(int LEDcase) {
  switch (LEDcase) {
    case 1: useLED(1,0,0,0);
      break;
    case 2: useLED(0,1,0,0);
      break;
    case 3: useLED(0,0,1,0);
      break;
    case 4: useLED(0,0,0,1);
      break;
    default: useLED(1,1,1,1);
  }
}


Any help is appreciated, thanks.

bobthebanana

if (digitalRead(button2)==LOW) {
   LEDcase==2;
     whichLED(LEDcase);
 }

should be

if (digitalRead(button2)==LOW) {
   LEDcase=2;
     whichLED(LEDcase);
 }

== checks for a value

= assigns a value


el_supremo

Apart from the problem with "==" and "=", your code is unnecessarily complicated. You can remove the whichLED and useLED functions and change the loop() function to this:
Code: [Select]
void loop() {

 if (digitalRead(button1)==LOW)digitalWrite(buttonLED1, LOW);
 else digitalWrite(buttonLED1, HIGH);
 if (digitalRead(button2)==LOW)digitalWrite(buttonLED2, LOW);
 else digitalWrite(buttonLED2, HIGH);
 if (digitalRead(button3)==LOW)digitalWrite(buttonLED3, LOW);
 else digitalWrite(buttonLED3, HIGH);
 if (digitalRead(button4)==LOW)digitalWrite(buttonLED4, LOW);
 else digitalWrite(buttonLED4, HIGH);
 
}

Actually, you can tighten it up even more but


Pete

GoForSmoke

User can press more than one button at once but the output will show it poorly.
I find it harder to express logic in English than in Code.
Sometimes an example says more than many times as many words.

el_supremo

Oooops. I have the operations the wrong way round. If the button goes LOW the output to the LED should be HIGH (assuming you've wired them so that pressing the button grounds the input and a high on the output turns on the LED).
Code: [Select]
void loop() {

  if (digitalRead(button1)==LOW)digitalWrite(buttonLED1, HIGH);
  else digitalWrite(buttonLED1, LOW);
  if (digitalRead(button2)==LOW)digitalWrite(buttonLED2, HIGH);
  else digitalWrite(buttonLED2, LOW);
  if (digitalRead(button3)==LOW)digitalWrite(buttonLED3, HIGH);
  else digitalWrite(buttonLED3, LOW);
  if (digitalRead(button4)==LOW)digitalWrite(buttonLED4, HIGH);
  else digitalWrite(buttonLED4, LOW);
 
}


Quote
User can press more than one button at once but the output will show it poorly.

Why?

Pete

GoForSmoke

Not your code Pete, the OP's.

I could see wiring the buttons to 4 open pins on PORTB (8-11) and the leds to 4 open pins on PORTD (4-7 to avoid the hardware serial pins).

http://www.arduino.cc/en/Reference/PortManipulation

It would mean reading PORTB into a byte, masking out the non-button pins, shifting the result to match the proper bits for PORTD then using | (or) to set the PORTD bits that run the leds. Likely the DDR bits for both would need to be set before the PORTB read and PORTD write, perhaps once in setup() would do.

Of course this is a BAD IDEA because it won't easily travel to every chip made....

I find it harder to express logic in English than in Code.
Sometimes an example says more than many times as many words.

engachou

Thank you, Pete; initially I tried to think of a way of writing it with if statements but I was thinking too much about how to explicitly turn all the other lights off when one was on. I tried your code out, and yes it works, but I'd like to have the lights stay on without holding the button down. My code may be roundabout, but if it does what I want, that's good enough for me!


User can press more than one button at once but the output will show it poorly.



What do you mean by 'show it poorly'? On testing, it does what I expect: no two lights can be on at once so whichever button press was detected last, that corresponding light will turn on. It's difficult to press these buttons (lilypad) exactly simultaneously, as well (it's pretty fiddly to press one down on its own! :P). User is not expected to hold buttons down either, so really, it does the job.

Go Up