Arduino Forum upgrade scheduled for Monday, October 20th, 11am-4pm (CEST). Sorry for the inconvenience!
Pages: [1]   Go Down
Author Topic: Switch case: always default case  (Read 545 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 7
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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.
Logged

Offline Offline
Full Member
***
Karma: 5
Posts: 207
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

 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
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 7
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Ah, thanks!
Logged

Offline Offline
Edison Member
*
Karma: 51
Posts: 1734
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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
Logged

Pittsburgh, PA, USA
Offline Offline
Faraday Member
**
Karma: 100
Posts: 4869
I learn a bit every time I visit the forum.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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

Offline Offline
Edison Member
*
Karma: 51
Posts: 1734
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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
Logged

Pittsburgh, PA, USA
Offline Offline
Faraday Member
**
Karma: 100
Posts: 4869
I learn a bit every time I visit the forum.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Logged

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

Offline Offline
Newbie
*
Karma: 0
Posts: 7
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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! smiley-razz). User is not expected to hold buttons down either, so really, it does the job.
Logged

Pages: [1]   Go Up
Arduino Forum upgrade scheduled for Monday, October 20th, 11am-4pm (CEST). Sorry for the inconvenience!
Jump to: