7 leds blinking in loop

Hello, I have a project and a part of it is 2 functions under 2 buttons, one of the function wchich im currently working on is when you press button leds start blinking from 1st to 7th with 1 second delay, so at the start 1st one lights up for a second, then its turn off and second one lights up and so on. Heres code that I made:

//Przyciski
const int p1 = 9;
const int p2 = 10;
//Ledy
const int led[7] = {2,3,4,5,6,7,8};
//Inne
int t1 = 1;
int t2 = 1;
int i = 0;

void setup() {
  pinMode(p1, INPUT);
  pinMode(p2, INPUT);
  pinMode(led[0], OUTPUT);
  pinMode(led[1], OUTPUT);
  pinMode(led[2], OUTPUT);
  pinMode(led[3], OUTPUT);
  pinMode(led[4], OUTPUT);
  pinMode(led[5], OUTPUT);
  pinMode(led[6], OUTPUT);
}

void loop() {
  i = millis()/1000 -1;
  }
  if (digitalRead(p1) == HIGH) {
    delay(250); t1++;
  }  

  if(t1%2 == 0){ 
    digitalWrite(led[i], HIGH);
    digitalWrite(led[i--], LOW);
  }else{
    digitalWrite(led[0], LOW);
    digitalWrite(led[1], LOW);
    digitalWrite(led[2], LOW);
    digitalWrite(led[3], LOW);
    digitalWrite(led[4], LOW);
    digitalWrite(led[5], LOW);
    digitalWrite(led[6], LOW); 
   }
  
}

The problem is that program works until millis will not be greater than 7000, and i dont know how to make it work like i want to. At first i tried to make it with for loop, but I wasnt able to delay it without using delay funtion of course, beacuse it cant be here (I know its under first if but its not that bad, but i will try to make it with millis later). Can anyone help??

The code that you posted does not compile

Where should the loop() function end ?

That is because you make i an integer it needs to be an unsigned long, because an integer is not big enough to hold te number millis returns.

Lots of other stuff wrong with that code as well.

Using i like you do is fundamental wrong, and you don't use loops to remove all that turgid code.

I think i pasted my code properly, but im pasting it once again, it compile for sure

//Przyciski
const int p1 = 9;
const int p2 = 10;
//Ledy
const int led[7] = {2,3,4,5,6,7,8};
//Inne
int t1 = 1;
int t2 = 1;
int i = 0;

void setup() {
  pinMode(p1, INPUT);
  pinMode(p2, INPUT);
  pinMode(led[0], OUTPUT);
  pinMode(led[1], OUTPUT);
  pinMode(led[2], OUTPUT);
  pinMode(led[3], OUTPUT);
  pinMode(led[4], OUTPUT);
  pinMode(led[5], OUTPUT);
  pinMode(led[6], OUTPUT);
}

void loop() {
  i = millis()/1000 -1;
  if (digitalRead(p1) == HIGH) {
    delay(250); t1++;
  }  

  if(t1%2 == 0){ 
      digitalWrite(led[i], HIGH);
      digitalWrite(led[i--], LOW); 
  }else{
    digitalWrite(led[0], LOW);
    digitalWrite(led[1], LOW);
    digitalWrite(led[2], LOW);
    digitalWrite(led[3], LOW);
    digitalWrite(led[4], LOW);
    digitalWrite(led[5], LOW);
    digitalWrite(led[6], LOW); 
   }
}

I still wrong, you are miss counting the braces }
For the whole loop function there must be an equal number of { and }

However it still contains the totally incorrect use of millis, it keeps on going but to use it as an index to an array, it will go wrong once you run out of valid index values.

I dont know what youre talking about the braces, beacuse it compiles and number of { are equal to } in void loop, but thats why im asking for help, I dont know how to write a good code to make led lights switch with 1 second delay

have you studied the builtin examples blink and blink without delay?

Yes, but i dont know how to make use of it in this situation

No.
At least not in the code you posted, you missed one when you highlighted the code for copying in the IDE. To avoid this then use the "select all" option in the IDE before the copy for forum operation.

//Przyciski
const int p1 = 9;
const int p2 = 10;
//Ledy
const int led[7] = {2,3,4,5,6,7,8};
//Inne
int t1 = 1;
int t2 = 1;
long i = 0;

void setup() {
  pinMode(p1, INPUT);
  pinMode(p2, INPUT);
  pinMode(led[0], OUTPUT);
  pinMode(led[1], OUTPUT);
  pinMode(led[2], OUTPUT);
  pinMode(led[3], OUTPUT);
  pinMode(led[4], OUTPUT);
  pinMode(led[5], OUTPUT);
  pinMode(led[6], OUTPUT);
}

void loop() {
  i = millis()/1000 -1;
  if (digitalRead(p1) == HIGH) {
    delay(250); t1++;
  }  

  if(t1%2 == 0){ 
      digitalWrite(led[i], HIGH);
      digitalWrite(led[i--], LOW); 
  }else{
    digitalWrite(led[0], LOW);
    digitalWrite(led[1], LOW);
    digitalWrite(led[2], LOW);
    digitalWrite(led[3], LOW);
    digitalWrite(led[4], LOW);
    digitalWrite(led[5], LOW);
    digitalWrite(led[6], LOW); 
   }
  
}

Did you try to use an unsigned long to hold the millis() value? unsigned long i = 0;?

How does the OP know that I will not be greater than 7000? I do not see any serial prints being used for troubleshooting in the posted code?

Does the OP know about loops()?

for instance the above could be done with a for() loop.

C++ For Loop.

I dont think i was using i, but i was using long, and i was trying to do something with current millis, previous millis and interval but the code here is the closest one i was able to get

Don't whine, read, learn, try.
That's how you get a sketch.
We'll be happy to help you if there are any questions.

That code does not use millis() for timing in anyway I've ever seen.

Still missing the last closing brace in the code.

If you don't believe me then copy what you posted by using the copy icon in the top right hand corner, and then paste it into a new sketch in the IDE and then try to compile it.

If you can't handle copy and paste what hope do you have of learning to code?

Forget using millis and use instead a for loop to turn your LEDs on and off. Use a delay call to get the timing right.

I will try it later but for now i have to make blinking in loop right, maybe there's any chance to delay a for loop?

Let us know when you are interested in learning how to use millis() in a meaningful way. Otherwise, have fun with delay().

When you are ready to start using millis() in a meaningful way. Let us know a bit more about the purpose of the project.

Okay then Im going to try doing it myself, thanks for help, I will let you know if i will do something better :slight_smile:

Hi @lma519 ,

welcome to the forum.
As soon as you start asking specific questions you will get helpful answers.
Specific answers are much much easier to answer.

Here is a tutorial how to code non-blocking timing based on millis().
I am pretty sure that you will understand the everyday description of the tutorial

best regards Stefan

Is this better guys?

//Przyciski
const int p1 = 9;
const int p2 = 10;
//Ledy
const int led[7] = {2,3,4,5,6,7,8};
//Inne
int t1 = 1;
int t2 = 1;
int i = 0;
unsigned long ac = 0;
unsigned long pc = 0;
long czas = 1000;

void setup() {
  pinMode(p1, INPUT);
  pinMode(p2, INPUT);
  pinMode(led[0], OUTPUT);
  pinMode(led[1], OUTPUT);
  pinMode(led[2], OUTPUT);
  pinMode(led[3], OUTPUT);
  pinMode(led[4], OUTPUT);
  pinMode(led[5], OUTPUT);
  pinMode(led[6], OUTPUT);
  Serial.begin(19200);
}

void loop() {
  Serial.println(i);
  ac = millis();
  if (digitalRead(p1) == HIGH) {
    delay(250); t1++;
  }  

  if(ac-pc>czas){
    i++;
    pc = ac;
  }
  if(i >= 7){
    i = 0;
  }

  if(t1%2 == 0){ 
      digitalWrite(led[i], HIGH); 
      //digitalWrite(led[i-1], LOW);
  }else{
    digitalWrite(led[0], LOW);
    digitalWrite(led[1], LOW);
    digitalWrite(led[2], LOW);
    digitalWrite(led[3], LOW);
    digitalWrite(led[4], LOW);
    digitalWrite(led[5], LOW);
    digitalWrite(led[6], LOW); 
   }
  
}