latching leds with a momentary switch

I have 2 momentary switches and 2 leds. I want the led to latch with a push of the switch. I've used a loop rather than CASE so that I can extend this to cover more leds.

At the moment, there are two issues. Firstly, I tried to set the leds off at the start, but they both light to begin with.

Secondly, the leds go out only when the button is held down, they don't latch. I'm at an early stage of learning, so please bear with me. Any tips really appreciated!

// double latching LEDs

int SwitchPins[] = {11, 12};  
int states[] = {0, 0};  
int leds[] = {6, 7};  
int i;
int j;
int reading;

void setup()
  {
  for(j=-1; j<=1; j++)
    {
    pinMode(leds[j], OUTPUT); // LED
    pinMode(SwitchPins[j],INPUT_PULLUP);
    digitalWrite(leds[j], LOW);
    }
  }
     
void loop() 
{
   for(i=-1; i<=1; i++)
   {
    reading = digitalRead(SwitchPins[i]);
    if (states[i] == 0 && reading == HIGH)  // it's off
      { 
      states[i] = 1; // remember it's on
      digitalWrite(leds[i], HIGH);
      }
    if (states[i] == 1 && reading == LOW)   // it's on
      {  
      states[i] = 0; // remember it's off
      digitalWrite(leds[i], LOW);
      }
  delay(20); //  debounce
  }
}
for(j=-1; j<=1; j++)
...
for(i=-1; i<=1; i++)

-1?? Really... doesn't this feel really strange to you? Have you ever seen this done anywhere ? What do you think leds[-1] or SwitchPins[-1] are going to be ?

Describe your hardware wiring

You use INPUT_PULLUP on your button pins, so they are LOW when pressed.

In words, your if statement becomes:

first
'if led is off and button is not pressed, turn led on.'

second
'if led is on and button is pressed, turn led off.'

As you have noticed, as soon you release the button led will come back on.

There is some good built-in examples you can look at, debounce and State Change Detection comes to my mind.

And some notes

  • A delay() is the most crude way to debounce a button. If you don't want to mess with it, just grab a library like Bounce2

  • Instead of state variables you can just read the state. digitalRead() just works. That way you don't need to keep the state and the actual output in sync. I like to add this simple function when I do toggling. (Should almost be part of the IDE libraries...)

inline void digitalToggle(uint8_t pin){
  digitalWrite(pin, !digitalRead(pin));
}
  • Don't just make every variable you use global. You don't need to and it eats memory and makes it hard to reuse. For example, i and j could be replaced by just defining it in the for-loop
for(byte i = 0; i < 2; i++){