I have a problem in the performance of my program

Projects Programming
9

Some comments on the program of KawasakiZx10r. He is indeed showing the extreme receptivity in the program. But:

#define button 1

The compiler supports C++, use const byte instead. Way saver and gives way more useful errors when you mess up :slight_smile:

While you're add it, give it a more useful name like 'ButtonPin' and 'LedPins' (the plural already indicates array).

I would switch to using the internal pulls ups. Saves the hassle of connecting a resistor to the button.

The sorting is just plain stupid ::slight_smile: Sorry, that way you make the pins hard coded again. Why not simply reverse the order of the looping?

Also, the program isn't a clone of OP's because you can now change direction while the sequence is going on, the code of OP finishes a sequence first.

And you're only looping 4 of the 5 leds and never turn them all off...

So my take, also made it non-blocking

const byte ButtonPin = 1;
const byte LedPins[] = {13, 11, 9, 7, 5};
const byte NrLeds = sizeof(LedPins);
//Yes, this is my exception to plurals if I prefix it with Nr (NumbeR, because I think No is confusing
const unsigned int BlinkInterval = 200;

bool direction = false;
bool currentLed = 0; //0 = no led on

unsigned long blinkMillis = -BlinkInterval; //so it will start right away

void setup(){
  for(byte i = 0; i < NrLeds; i++){
    pinMode(LedPins[i], OUTPUT);
  }
  
  pinMode(ButtonPin, INPUT_PULLUP);
}

void loop(){
  if(currentLed == 0){
    checkButton();
  }
  
  updateLeds();
}

void checkButton(){
  direction = digitalRead(ButtonPin);
}

void updateLeds(){
  if(millis() - blinkMillis >= BlinkInterval){
    blinkMillis = millis();
    
    //if a led is on, turn it off
    if(currentLed){
      digitalWrite(LedPins[currentLed - 1, LOW);
    }
    
    //turn on next led
    updateNextLed();
    if(currentLed){
      digitalWrite(LedPins[currentLed - 1], HIGH);
    }
  }
}

void updateNextLed(){
  //goto next led
  if(direction){
    if(currentLed == NrLeds){
      currentLed = 0;
    }
    else{
      currentLed++;
    }
  }
  else{
    if(currentLed == 0){
      currentLed = NrLeds;
    }
    else{
      currentLed--;
    }
  }
}