A Loop is making my brain hurt

I dont know why but im having a brain deficiency.

I have 40 leds in a line with shift registers, im trying to shrink my code down into loops

So a loop where i want 4 to light, skip 4 then light the next four, so on until 40. How do you increment that loop?

I know index++ does 1, and index+=4 would be 4, how do i get 1 through 4, 9 through 13, etc..

you have 5 shift registers…

are you using shiftOut?

for (int i = 0; i < 5; i++)
{
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
}

so i would need 10 for loops then ? I was trying to get all in one

so at one time pins, 1 thru 4, 9 thru 13, 17 thru 21, 26 thru 30, 34 thru 36 are all on at the same time.

not sure what you mean...

are your shift registers all attached to each other?

this will push 5 bytes (B11110000) through the shift registers 5 x 8 = 40 LEDs

do you have some code you are working with?

here is my register writing function i left out the usual used in every program garbage unless you need it

#define number_of_74hc595s 5 
#define numOfRegisterPins number_of_74hc595s * 8

void writeRegisters(){

  digitalWrite(latchpin, LOW);

  for(int i = numOfRegisterPins - 1; i >=  0; i--){
    digitalWrite(clockpin, LOW);

    int val = registers[i];

    digitalWrite(datapin, val);
    digitalWrite(clockpin, HIGH);

  }
  digitalWrite(latchpin, HIGH);

}

//set an individual pin HIGH or LOW
void setRegisterPin(int index, int value){
  registers[index] = value;
}

and here is what im trying to condense

void redLEDs() {
  int index = 0;
  
  setRegisterPin(index, HIGH);
  setRegisterPin(index+1, HIGH);
  setRegisterPin(index+2, HIGH);
  setRegisterPin(index+3, HIGH);
  setRegisterPin(index+8, HIGH);
  setRegisterPin(index+9, HIGH);
  setRegisterPin(index+10, HIGH);
  setRegisterPin(index+11, HIGH);
  setRegisterPin(index+16, HIGH);
  setRegisterPin(index+17, HIGH);
  setRegisterPin(index+18, HIGH);
  setRegisterPin(index+19, HIGH);
  setRegisterPin(index+24, HIGH);
  setRegisterPin(index+25, HIGH);
  setRegisterPin(index+26, HIGH);
  setRegisterPin(index+27, HIGH);
  setRegisterPin(index+33, HIGH);
  setRegisterPin(index+33, HIGH);
  setRegisterPin(index+34, HIGH);
  setRegisterPin(index+35, HIGH);
  writeRegisters();
}

it would help to see the rest of your code, actually

Nhoc6131: ...i left out the usual used in every program garbage unless you need it

int datapin = 13; 
int latchpin = 7;  
int clockpin = 12; 

//How many of the shift registers 
#define number_of_74hc595s 5 
#define numOfRegisterPins number_of_74hc595s * 8

boolean registers[numOfRegisterPins];

void setup(){
  pinMode(datapin, OUTPUT);
  pinMode(latchpin, OUTPUT);
  pinMode(clockpin, OUTPUT);

  //reset all register pins
  clearRegisters();
  writeRegisters();
}               


//set all register pins to LOW
void clearRegisters(){
  for(int i = numOfRegisterPins - 1; i >=  0; i--){
     registers[i] = LOW;
  }
} 

void writeRegisters(){

  digitalWrite(latchpin, LOW);

  for(int i = numOfRegisterPins - 1; i >=  0; i--){
    digitalWrite(clockpin, LOW);

    int val = registers[i];

    digitalWrite(datapin, val);
    digitalWrite(clockpin, HIGH);
  }
  digitalWrite(latchpin, HIGH);

}

//set an individual pin HIGH or LOW
void setRegisterPin(int index, int value){
  registers[index] = value;
}


void loop(){
 wigWag();  
}

void redLEDs() {
  int index = 0;
  
  setRegisterPin(index, HIGH);
  setRegisterPin(index+1, HIGH);
  setRegisterPin(index+2, HIGH);
  setRegisterPin(index+3, HIGH);
  setRegisterPin(index+8, HIGH);
  setRegisterPin(index+9, HIGH);
  setRegisterPin(index+10, HIGH);
  setRegisterPin(index+11, HIGH);
  setRegisterPin(index+16, HIGH);
  setRegisterPin(index+17, HIGH);
  setRegisterPin(index+18, HIGH);
  setRegisterPin(index+19, HIGH);
  setRegisterPin(index+24, HIGH);
  setRegisterPin(index+25, HIGH);
  setRegisterPin(index+26, HIGH);
  setRegisterPin(index+27, HIGH);
  setRegisterPin(index+32, HIGH);
  setRegisterPin(index+33, HIGH);
  setRegisterPin(index+34, HIGH);
  setRegisterPin(index+35, HIGH);
  writeRegisters();
  switchOff();
}

void BlueLEDs() {
  
  index = 0;  
  setBrightness(255);
  setRegisterPin(index+4, HIGH);
  setRegisterPin(index+5, HIGH);
  setRegisterPin(index+6, HIGH);
  setRegisterPin(index+7, HIGH);
  setRegisterPin(index+12, HIGH);
  setRegisterPin(index+13, HIGH);
  setRegisterPin(index+14, HIGH);
  setRegisterPin(index+15, HIGH);
  setRegisterPin(index+20, HIGH);
  setRegisterPin(index+21, HIGH);
  setRegisterPin(index+22, HIGH);
  setRegisterPin(index+23, HIGH);
  setRegisterPin(index+28, HIGH);
  setRegisterPin(index+29, HIGH);
  setRegisterPin(index+30, HIGH);
  setRegisterPin(index+31, HIGH);
   setRegisterPin(index+36, HIGH);
  setRegisterPin(index+37, HIGH);
  setRegisterPin(index+38, HIGH);
  setRegisterPin(index+39, HIGH);
     
  
  writeRegisters();
  switchOff();
}

void wigWag();
{
  BlueLEDs();
  delay(150);
  redLEDs();
}

voidSwitchOff();
{
   int index;
   
  for(index = 0; index <= 39; index ++)
    {
      setRegisterPin(index, LOW);
      writeRegisters();
    }
}

Based on what you wrote, I can assume this is what you were going for, but I like Bulldogs’s method better.

for(byte i = 0; i < 50; i++)
{
setRegisterPin(i, HIGH); //RED
if( (i % 4) == 3) i+=4;
}

void setup()
{
  Serial.begin(115200);

  Serial.print("Red: ");
  for(byte i = 0; i < 50; i++) // red starts at 0
  {
    Serial.print(i);
    Serial.print(", ");
    //setRegisterPin(i, HIGH); //RED
    if( (i % 4) == 3) i+=4;
  }
  Serial.println();
  
  Serial.print("Blue: ");
  for(byte i = 4; i < 50; i++)// blue starts at 4
  {
    Serial.print(i);
    Serial.print(", ");
    //setRegisterPin(i-1, HIGH); //BLUE
    if( (i % 4) == 3) i+=4;
  } 
}

void loop() {

}

Result:

Red: 0, 1, 2, 3, 8, 9, 10, 11, 16, 17, 18, 19, 24, 25, 26, 27, 32, 33, 34, 35, 40, 41, 42, 43, 48, 49,
Blue: 4, 5, 6, 7, 12, 13, 14, 15, 20, 21, 22, 23, 28, 29, 30, 31, 36, 37, 38, 39, 44, 45, 46, 47,

plus… be careful with functions calling functions… it is really difficult to keep all that organized.

mess with this for a while and learn how your shift register, functions, arrays and for() loops work.

int datapin = 13; 
int latchpin = 7;  
int clockpin = 12; 
byte trace[8] = {B00000001, B00000010, B00000100, B00001000, B00010000, B00100000, B01000000, B10000000};


void setup()
{
  pinMode(datapin, OUTPUT);
  pinMode(latchpin, OUTPUT);
  pinMode(clockpin, OUTPUT);
  writeRegisters(B00000000);  // all LEDs off !!!!
  writeRegisters(B11111111);  // all LEDs ON for one second...
  delay(1000);
  writeRegisters(B00000000);// all LEDs off!!!
}               

void loop()
{
  
  for (int i = 0; i < 8; i++) // eight elements in the array called trace[] above
  {
    writeRegisters(trace[i]);  //take the i(th) element of the array and run the writeRegisters() function with it
    delay(50);
  }
  
}
void writeRegisters(byte myByte)  // take the byte sent to the function and call it myByte inside this function .....
{
  digitalWrite(latchpin, LOW);
  for(int i = 0; i < 5 ; i++) // 5 registers with one byte each
  {
    shiftOut(datapin, clockpin, MSBFIRST, myByte);
  }
  digitalWrite(latchpin, HIGH);
}

great thanks a ton for the help, i couldnt wrap my head around the logic.

the reason for the functions in the functions is eventually i wanted to do other flash patterns and those functions will be in the code but i didnt want to have to call a ton of functions in the main loop, only the few i wanted and some a variation of others. confusing I know.

I'll play with what you gave me and see what it does. I was trying to stay away from the actual bits as i thought thatd make the program more complicated then just using numbers.

It is easy enough to customize what lights are on when? I basically am trying to build a shell of a program that is easily modifiable for flash patters, number of leds, and colors

bytes with shift registers are MUCH more easily understood and managed.

just looking at the byte (e.g. B00000000 and B11111111) you can see the expected output (all LEDs off and on respectively in that example) for the shift register.

You can do your flashing patterns MUCH easier without calling functions from functions. it just becomes important to decide how you want to change the flashing pattern (e.g. from a pushbutton, from the serial monitor, from another computer, form the internet, on a Timer, etc.).

I see,

I currently have a push button switching between some of the patterns. I omitted it from the above code since its not what I was having a problem with.

I have just started changing alot of them from lineeeeessss of code to loops and also using the millis() checking so I can do more at once

I was able to follow your code chunk in my head so I feel like im getting better with this!

Nhoc6131: I was able to follow your code chunk in my head so I feel like im getting better with this!

So... let's get those LEDs flashing....

let's create a state machine that changes the pattern!!!

the state machine is starting to churn! I have good back forth going right now. This is also why i did a function inside a function is I thought this is the only way to get the state machine part to work

void redLEDs() {
  int index = 0;
   
   setBrightness(255); 
   
 if(ledState1 == HIGH) {
     for(index = 0; index <= 39; index++)
        {
         setRegisterPin(index, LOW); //RED
         if( (index % 4) == 3) index+=4;
        }
  
  ledState1 = LOW;
}
else {
  for(index = 0; index <= 39; index++)
        {
         setRegisterPin(index, HIGH); //RED
         if( (index % 4) == 3) index+=4;
        }
    ledState1 = HIGH;
  }
    
  writeRegisters();
  flash1 = millis() + nextflash1;
}


void wigWag()
{  
 if(millis() >= flash1) redLEDs();
 if(millis() >= flash2) blueLEDs(); 
 
}

I didnt post the blue Leds part yet because im still switching it over to the for loop

Also so in plain english help me understand( (i % 4) == 3) index +=4;

So, thats returning a true false right?

I'm thinking thats not what I want. I dont think Id want to have the nextflash1 or 2 variable be global because if I have say, 10 different patterns Id want to change the interval inside that specific function/pattern to my tastes, rather than have a ton of gobal intervals, right?

strike that last question, I just tried it. I did:

void wigWag()
{  
  int index = 0;
  int nextflash1 = 250;
  int nextflash2 = 250;
  setBrightness(255); 
  if(millis() >= flash1){ 
    if(ledState1 == HIGH) {
      for(index = 0; index <= 39; index++)
      {
        setRegisterPin(index, LOW); //RED
        if( (index % 4) == 3) index+=4;
      }

      ledState1 = LOW;
    }
    else {
      for(index = 0; index <= 39; index++)
      {
        setRegisterPin(index, HIGH); //RED
        if( (index % 4) == 3) index+=4;
      }
      ledState1 = HIGH;
    }

    writeRegisters();
    flash1 = millis() + nextflash1;
  }

  if(millis() >= flash2) {
    if(ledState2 == HIGH) {
      for(index = 4; index <= 39; index++)// blue starts at 4
      {

        setRegisterPin(index, LOW); //BLUE
        if( (index % 4) == 3) index+=4;
      }      

      ledState2 = LOW;
    }
    else  {

      for(index = 4; index <= 39; index++)// blue starts at 4
      {

        setRegisterPin(index, HIGH); //BLUE
        if( (index % 4) == 3) index+=4;
      }
      ledState2 = HIGH;
    }
    writeRegisters(); 
    flash2 = millis() + nextflash2;
  }
}

worked like a charm. one pattern down

think about it like this:

#define NUMBER_OF_STATES 4

int datapin = 13; 
int latchpin = 7;  
int clockpin = 12; 
byte trace[8] = {B00000001, B00000010, B00000100, B00001000, B00010000, B00100000, B01000000, B10000000};
int state;
int lastPressed;
int buttonPin = 3;

void setup()
{
  pinMode(datapin, OUTPUT);
  pinMode(latchpin, OUTPUT);
  pinMode(clockpin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  writeRegisters(B00000000);
  writeRegisters(B11111111);
  delay(1000);
  writeRegisters(B00000000);
}               

void loop()
{
  int pressed = digitalRead(buttonPin); 
  if (pressed == LOW)
  {
    if (pressed != lastPressed)
    {
      state++;
      if (state > NUMBER_OF_STATES) state = 0;
    }
  }
  lastPressed = pressed;
  if (state == 0) chaseLeds();
  else if (state == 1) singleLed();
  else if (state == 2) flashAll();
  else if (state == 3) writeRegisters(B11111111);
  else if (state == 4) writeRegisters(B00000000);
}
//
void singleLed()
{
  // put a function here
}
//
void flashAll()
{
  // put a function here 
}
//
void chaseLeds()
{
  for (int i = 0; i < 8; i++)
  {
    writeRegisters(trace[i]);
    delay(50);
  }
}
//
void writeRegisters(byte myByte)
{
  digitalWrite(latchpin, LOW);
  for(int i = 0; i < 5 ; i++)
  {
    shiftOut(datapin, clockpin, MSBFIRST, myByte);
  }
  digitalWrite(latchpin, HIGH);
}

So I have my lights functioning great as a state machine.

Now I’m trying to incorporate a siren sound. I did a forum search and found this from a few years back and I was trying to get that US police siren wail transitioning from low tone to high tones. The length of this loop of course effects my lights ability to flash. So I know I need to do a state machine on this too

this is the siren tone im trying to incorporate

 while(frequency < 1000)
  {
    tone(3,frequency,250);
    frequency = (frequency) + (5);
    Serial.println(frequency);
    delay(10);
    
  }
  Serial.println();

  while(frequency > 400)
  {
    tone(3,frequency,250);
    frequency = (frequency) - (5);
    Serial.println(frequency);
    delay(10);

Can I get a shove in the right direction. I understand it with LEDs since your checking the States of the LED but what about something constant like sound?

this is the siren tone im trying to incorporate

You haven't learned anything, have you? For loops, while loops, do/while loops and state machines and blink without delay philosophy code do NOT belong in the same sketch.

Your while loop is BLOCKING, just like delay(), which you STILL use.

I know. Thats why i posted what i currently have and said my desire to modify my code to work with state machine. I thought id at least post my starting point