Multiple Buttons with LEDs, Press One, Make The Others Turn Their LED Off?

[pusher]

I was given code to work with for a MIDI controller project. It has 8 buttons that select presets. The supplied code only allows for 1 LED, but I would like to alter it to have each button have an LED that lights when that button is pressed - so I know which one is active -  but turns off any other LED that was on. I'm looking for some example code (maybe with comments?) that shows the correct way to control the desired behavior and would work with the existing code. Any help would be greatly appreciated.

The original code I'm working with is below:
______________________________

#include <MIDI.h>



#define SW1 3

#define SW2 4

#define SW3 5

#define SW4 6

#define SW5 7

#define SW6 8

#define SW7 9

#define SW8 10



#define LED1 12



void setup() {

  pinMode(SW1, INPUT);

  pinMode(SW2, INPUT);

  pinMode(SW3, INPUT);

  pinMode(SW4, INPUT);

  pinMode(SW5, INPUT);

  pinMode(SW6, INPUT);

  pinMode(SW7, INPUT);

  pinMode(SW8, INPUT);

  pinMode(LED1, OUTPUT);



  digitalWrite(SW1, HIGH);

  digitalWrite(SW2, HIGH);

  digitalWrite(SW3, HIGH);

  digitalWrite(SW4, HIGH);

  digitalWrite(SW5, HIGH);

  digitalWrite(SW6, HIGH);

  digitalWrite(SW7, HIGH);

  digitalWrite(SW8, HIGH);



  //Serial.begin(9600);

  MIDI.begin(31250);



  for (int i = 0; i < 10; i++) {

    digitalWrite(LED1, HIGH);

    delay(60);

    digitalWrite(LED1, LOW);

    delay(60);

  }



  delay(100);

}



unsigned long db = 150;

unsigned long lastRead = millis();

byte chn = 1;



unsigned long ledOn = millis();

bool isLEDOn = false;



void loop() {



  // Preset G

  if (digitalRead(SW1) == LOW && millis() - lastRead > db) {

    turnLEDOn();

    MIDI.sendControlChange(16, 0, chn);

    lastRead = millis();

  }

 

  // Preset H

  if (digitalRead(SW2) == LOW && millis() - lastRead > db) {

    turnLEDOn();

    MIDI.sendControlChange(17, 0, chn);

    lastRead = millis();

  }

 

  // Preset I

  if (digitalRead(SW3) == LOW && millis() - lastRead > db) {

    turnLEDOn();

    MIDI.sendControlChange(18, 0, chn);

    lastRead = millis();

  }



  // Bank Down

  if (digitalRead(SW4) == LOW && millis() - lastRead > db) {

    turnLEDOn();

    MIDI.sendControlChange(1, 0, chn);

    lastRead = millis();

  }

 

  // Preset J

  if (digitalRead(SW5) == LOW && millis() - lastRead > db) {

    turnLEDOn();

    MIDI.sendControlChange(19, 0, chn);

    lastRead = millis();

  }

 

  // Preset K

  if (digitalRead(SW6) == LOW && millis() - lastRead > db) {

    turnLEDOn();

    MIDI.sendControlChange(20, 0, chn);

    lastRead = millis();

  }

 

  // Preset L

  if (digitalRead(SW7) == LOW && millis() - lastRead > db) {

    turnLEDOn();

    MIDI.sendControlChange(21, 0, chn);

    lastRead = millis();

  }



  // Bank Up

  if (digitalRead(SW8) == LOW && millis() - lastRead > db) {

    turnLEDOn();

    MIDI.sendControlChange(0, 0, chn);

    lastRead = millis();

  }



  // Turn led on for 500ms when any switch is pressed. Turn it off after 500ms.

  if (millis() - ledOn > 500 && isLEDOn == true) {

    isLEDOn = false;

    digitalWrite(LED1, LOW);

  }



}



void turnLEDOn() {

  isLEDOn = true;

  ledOn = millis();

  digitalWrite(LED1, HIGH);

}

[J-M-L]

Well you need more LEDs and either you remember that last one that was on and you turn it off before lighting a new one, or you turn them all off and then just turn the right one on.

How do you plan to connect all the LEDs?

[larryd]

Put your sketch between the code tags [code]Paste your sketch here[/code]

[pusher]

Thanks, J-M-L. I'm using an Arduino Micro board. I think I'll have enough pins to connect 8 switches and 8 LEDs.

I know I have to define the individual LEDs just like the switches are. What's the code look like to have a switch turn on its own LED, and turn off another LED if it was on?

Thanks,

pusher

[J-M-L]

Well I would define an array with switches, one for LEDs, (and also one for LEDs status) (or better an array of structures embedding the pair of pins and status of associated LED) because you repeat a lot of code in there

But in a crude way I would just go for a myLED array with the pins number and modify turnLEDOn() to pass an index of which one you want to turn on and that will be called in your code for example SW1 would match myLED[0], so call turnLEDOn(0); ,SW2 would match myLED[1] so call turnLEDOn(1);, ...

Code: [Select]


// in global variables
const byte myLED[8] = {8,9,10,11,12,13,A0,A1};  // whatever pins numbers used for the LEDs


//.....

Void turnLEDOn(byte whichLED)
{
   // turn all LEDs off
    for (int i =0; i<8;i++) digitalWrite(myLED[i], LOW);

   // turn the correct one on
   digitalWrite(myLED[whichLED], HIGH);

  // remember our status
   isLEDOn = true;
   ledOn = millis();
}


And of course the last part of main would need to be modified

Code: [Select]

  if (isLEDOn && millis() - ledOn > 500) {
   // turn all LEDs off
    for (int i =0; i<8;i++) digitalWrite(myLED[i], LOW);

  // remember our status
    isLEDOn = false;
  }


[pusher]

Hi,

Thanks for all the suggestions. I'm SOOOOO close to being done. I need just a little more help.

I have everything hooked up and the code modified to reflect which pins everything is attached to. I never thought I'd get this far.

The Set Up: 7 switches with 7 LEDs.

The Goal: When any switch is pressed, it turns off all other LEDs, but keeps it's own on.

The What Is: Each Switch's LED comes on when the switch is pressed. GOOD
They all turn off the LED for Switch 3 only. BAD
Switch 3 doesn't turn any LEDs off.

I keep looking at the code to find out what's different about Switch 3, but I don't know enough to spot it. Can anyone see the reason it's not working as intended? Code below. I'd post pictures if I knew how...

Code: [Select]



/*

 * Code adapted to allow for 8 LEDs

 */




#include <MIDI.h>
MIDI_CREATE_DEFAULT_INSTANCE();




#define SW1 2

#define SW2 3

#define SW3 4

#define SW4 5

#define SW5 6

#define SW6 7

#define SW7 8






#define LED1 10

#define LED2 11

#define LED3 12

#define LED4 A0

#define LED5 A1

#define LED6 A2

#define LED7 A3






#define TOTAL_LED 7




byte ledArray [] = {LED1, LED2, LED3, LED4, LED5, LED6, LED7};



void blinkAllLeds(byte numTimes, byte inDelay) {

  for (int j = 0; j < numTimes; j++) {

    for (int i = 0; i < TOTAL_LED; i++) {

      digitalWrite(ledArray[i], HIGH);

    }

    delay(inDelay);

    for (int i = 0; i < TOTAL_LED; i++) {

      digitalWrite(ledArray[i], LOW);

    }

  }

}




void setAllLEDs(int type) {

  for (int i = 0; i < TOTAL_LED; i++) {

    digitalWrite(ledArray, type);

  }

}




void setup() {




  // Set pin modes

  pinMode(SW1, INPUT);

  pinMode(SW2, INPUT);

  pinMode(SW3, INPUT);

  pinMode(SW4, INPUT);

  pinMode(SW5, INPUT);

  pinMode(SW6, INPUT);

  pinMode(SW7, INPUT);


  pinMode(LED1, OUTPUT);

  pinMode(LED2, OUTPUT);

  pinMode(LED3, OUTPUT);

  pinMode(LED4, OUTPUT);

  pinMode(LED5, OUTPUT);

  pinMode(LED6, OUTPUT);

  pinMode(LED7, OUTPUT);

 




  digitalWrite(SW1, HIGH);

  digitalWrite(SW2, HIGH);

  digitalWrite(SW3, HIGH);

  digitalWrite(SW4, HIGH);

  digitalWrite(SW5, HIGH);

  digitalWrite(SW6, HIGH);

  digitalWrite(SW7, HIGH);

 




  MIDI.begin(31250);




  // Blink LED on start

  for (int i = 0; i < 10; i++) {

    digitalWrite(LED1, HIGH);

    delay(60);

    digitalWrite(LED1, LOW);

    delay(60);

  }




  delay(100);

}




unsigned long db = 150; // For switch debounce

unsigned long lastRead = millis();

byte chn = 1; // Midi Channel to send




unsigned long LedOn = millis();

bool isLEDOn = false;




void loop() {


// Bank Up

   if (digitalRead(SW1) == LOW && millis() - lastRead > db) {

    setAllLEDs(LOW);

     digitalWrite(LED1, HIGH);

     MIDI.sendControlChange(0, 0, chn);

     lastRead = millis();

   }

   

  // Preset G

  if (digitalRead(SW2) == LOW && millis() - lastRead > db) {

    setAllLEDs(LOW);

    digitalWrite(LED2, HIGH);

    MIDI.sendControlChange(16, 0, chn);

    lastRead = millis();

  }




  // Preset H

  if (digitalRead(SW3) == LOW && millis() - lastRead > db) {

    setAllLEDs(LOW);

    digitalWrite(LED3, HIGH);

    MIDI.sendControlChange(17, 0, chn);

    lastRead = millis();

  }




  // Preset I

  if (digitalRead(SW4) == LOW && millis() - lastRead > db) {

    setAllLEDs(LOW);

    digitalWrite(LED4, HIGH);

    MIDI.sendControlChange(18, 0, chn);

    lastRead = millis();

  }




  // Preset J

  if (digitalRead(SW5) == LOW && millis() - lastRead > db) {

    setAllLEDs(LOW);

    digitalWrite(LED5, HIGH);

    MIDI.sendControlChange(19, 0, chn);

    lastRead = millis();

  }




  // Preset K

  if (digitalRead(SW6) == LOW && millis() - lastRead > db) {

    setAllLEDs(LOW);

    digitalWrite(LED6, HIGH);

    MIDI.sendControlChange(20, 0, chn);

    lastRead = millis();

  }




  // Preset L

  if (digitalRead(SW7) == LOW && millis() - lastRead > db) {

    setAllLEDs(LOW);

    digitalWrite(LED7, HIGH);

    MIDI.sendControlChange(21, 0, chn);

    lastRead = millis();

  }



}

{/code]

[larryd]

Edit your last post and put your sketch between the code tags [code]Paste your sketch here[/code]

Please show us a schematic of your circuit.
Please show us a good image of your circuit wiring.


.

[pusher]

Thanks for the suggestions. I fixed the code part.

I don't know how to upload a picture.

[larryd]

To start with:

Code: [Select]


void setAllLEDs(int type) {

  for (int i = 0; i < TOTAL_LED; i++) {

    digitalWrite(ledArray, type);

  }

Code: [Select]


void setAllLEDs(int type) {

  for (int i = 0; i < TOTAL_LED; i++)
  {

    digitalWrite(ledArray[i], type);

  }

[J-M-L]

@LarryD

I think he got bitten by copying code from a forum where the user did not use the code tags. the challenge when you don't use the [code] [/code]  tags to post code is that the [i] in the array access gets interpreted by the forum engine as a command to put text in italics and thus does not get displayed. Hence the reason one should always post code within the right tags...


Regarding the code, besides fixing the above function which should help a ton

blinkAllLeds Does not fully blink it turns everything on, wait for a delay and then off but does not wait after the off for the delay. So if you call that repetitively, you will see the lights always on as the off state will be a few hundreth of microseconds. Also passing the delay as a byte is probably a limiting factor, you can't have a pause of more that 1/4 seconds roughly (max = 255 ms)

Why don't you use an array for everything and loops? you see your code has tons of repetition of similar pattern - will make things easier.