Calling Functions acts strangely and I can't see why... [solved]

Hello dear Arduinoids,

I hope one of you will be kind enough to help me out here as I'm pulling my hair out with this!

I have written a sketch to control four RGB LEDs with a Arduino Mega via FETs.

The code basically consists of a number of different colour sequences written into Functions declared with "void".

The start of the sketch just calls the different Functions in the order they are needed.

Everything works just fine so long as my list of "calls" doesn't exceed around nine or so. If I make the list any longer, the first colour sequence will run but very, very, VERY slowly. If I "rem" out a few of the calls (doesn'y matter which) then it runs all fine again. There is nothing special going on in the functions - just setting pins high and low -no nesting or whatever. Is there a limit to how many funtions can be called?

I would really appreciate any help and I can post the whole sketch if necessary.

Thanks!!!

Here is a snippet of the code...

// Calling Functions here...
s1();s2(); // r
s3();s4(); // g
s5();s6(); // b
s7();s8(); // v
s9();s10(); //c
s11();s12(); //y
ws1();ws2(); //white

s13();s14(); // calling functions
s13();s14(); // r
s15();s16(); // g
s17();s18(); // b
s19();s20(); // v
s21();s22(); //c
s23();s24(); //y
ws3();ws4(); //white

// xxxxxxxxxxxxxxxxxxx Red Inc xxxxxxxxxxxxxxx
void s1() { //declare function
// turn the LEDs on
digitalWrite(2, HIGH);
delay(d3);
digitalWrite(5, HIGH);
delay(d3);
digitalWrite(8, HIGH);
delay(d3);
digitalWrite(11, HIGH); // leds on red
delay(d3);

// turn the LEDs off
digitalWrite(2, LOW);
delay(d3);
digitalWrite(5, LOW);
delay(d3);
digitalWrite(8, LOW);
delay(d3);
digitalWrite(11, LOW);; // turn the LEDs off
delay(d3);
}
void s2() { // Declare function
// turn the LEDs on reverse
digitalWrite(11, HIGH);
delay(d3);
digitalWrite(8, HIGH);
delay(d3);
digitalWrite(5, HIGH);
delay(d3);
digitalWrite(2, HIGH); // leds on red
delay(d3);

// turn the LEDs off reverse
digitalWrite(11, LOW);
delay(d3);
digitalWrite(8, LOW);
delay(d3);
digitalWrite(5, LOW);
delay(d3);
digitalWrite(2, LOW); // turn the LEDs off
delay(d3);
}

// xxxxxxxxxxxxxxxxxxx Green Inc xxxxxxxxxxxxxxx
void s3() { //declare function
// turn the LEDs on
digitalWrite(3, HIGH);
delay(d3);
digitalWrite(6, HIGH);
delay(d3);
digitalWrite(9, HIGH);
delay(d3);
digitalWrite(12, HIGH); // leds on red
delay(d3);

// turn the LEDs off
digitalWrite(3, LOW);
delay(d3);
digitalWrite(6, LOW);
delay(d3);
digitalWrite(9, LOW);
delay(d3);
digitalWrite(12, LOW); // turn the LEDs off
delay(d3);
}

//xxxxxxxxx Green Dec xxxxxxxxxxxxxx
void s4(){ // turn the LEDs on reverse
digitalWrite(12, HIGH);
delay(d3);
digitalWrite(9, HIGH);
delay(d3);
digitalWrite(6, HIGH);
delay(d3);
digitalWrite(3, HIGH); // leds on red
delay(d3);

// turn the LEDs off reverse
digitalWrite(12, LOW);
delay(d3);
digitalWrite(9, LOW);
delay(d3);
digitalWrite(6, LOW);
delay(d3);
digitalWrite(3, LOW); // turn the LEDs off
delay(d3);
}

// xxxxxxxxxxxxxxxxxxx Blue Inc xxxxxxxxxxxxxxx
void s5() { //declare function
// turn the LEDs on
digitalWrite(4, HIGH);
delay(d3);
digitalWrite(7, HIGH);
delay(d3);
digitalWrite(10, HIGH);
delay(d3);
digitalWrite(13, HIGH); // leds on red
delay(d3);

// turn the LEDs off
digitalWrite(4, LOW);
delay(d3);
digitalWrite(7, LOW);
delay(d3);
digitalWrite(10, LOW);
delay(d3);
digitalWrite(13, LOW);; // turn the LEDs off
delay(d3);
}

......and so on...

sequence will run but very, very, VERY slowly.

Maybe it has something to do with all those delay(d3);

Show all of your sketch using the </> icon in the top left of the posting menu.

Thanks for the reply Larry,

No, the D3 delay is set elsewhere and is only half a second or so.
i should have added that no matter what order the functions are called, it's the first function called that it hangs up (runs stupidly slow) with.
If there are less than say nine or ten calls in the first part of the sketch, it's absolutely fine. Add a few more and the problem reappears ???
I even tried it on another Mega and it does the same.

Maybe you just have too much code.

Your program is shockingly badly arranged.

The problem with only pasting in the code where you think the problem is, is that the problem is almost never where the people posing the questions think. In this case, the code you pasted is fine, which means your problem is in the code you DON'T show. So, if you don't show ALL the code, that you've actually RUN, you're wasting everyone's time.....

Regards,
Ray L.

You really should not be using the delay() function at all. Have a look at how to use millis() to manage timing in several things at a time.

Also, don't put several statements on one line like

s1();s2(); // r

and give your functions meaningful names rather than "s1"

...R

Thanks for the replies!

I couldn't post all the code due to posting limits. The missing functions follow the same format as the others - just high, low and delays.

If a few of the "rem" // are removed from the "call" list, the problem occurs. It doesn't matter which ones.

I'm sorry for the"shockingly bad structure" as I'm new to this and I have no doubt there are many better ways to do what I am attempting - control 10Amps of RGB through FETs. Any ideas would be very welcome.

Thanks in advance for any help...

int ledPin[] = {2,3,4,5,6,7,8,9,10,11,12,13,21,22,23,24,25}; // initialise output pins


int d1 = 20; // Delay time 1
int d2 = 50; // Delay time 2
int d3 = 75;
int d4 = 100; 
int d5 = 150; 
int d6 = 200;
int d7 = 250; 
int d8 = 500;
int d9 = 750;
int d10 = 1000; 

void setup() {
  
for (int i =0;i<25;i++)
   {
     pinMode(ledPin[i], OUTPUT); // set pins as outputs
   }  
}




void loop() { // This is the main loop sequencer
  
  
 // calling functions - no muckin' around! Calling too many (>9)causes VERY slow execution of 1st called function ????? 
s13();s14(); // r
//s15();s16(); // g
s17();s18(); // b
s19();s20(); // v
//s21();s22(); //c
s23();s24(); //y
ws3();ws4(); //white

s1();s2(); // r
//s3();s4(); // g
s5();s6(); // b
s7();s8(); // v
//s9();s10(); //c
s11();s12(); //y
ws1();ws2(); //white



// calling sequences with loops
//  for (int c =0;c<2;c++){  //Repeat c times
//  s13();s15();s17();s19();s21();s23();ws3();
 // }

//for (int c =0;c<2;c++){  //Repeat c times
//  s14();s16();s18();s20();s22();s24();ws4();
//  }
}


 


// ------------------------------------------------------------ INC DEC --------------------------------------------
// xxxxxxxxxxxxxxxxxxx Red Inc xxxxxxxxxxxxxxx
void s1() { //declare function     
  // turn the LEDs on
    digitalWrite(2, HIGH);
    delay(d3);
    digitalWrite(5, HIGH);
    delay(d3);
    digitalWrite(8, HIGH); 
    delay(d3);
    digitalWrite(11, HIGH); // leds on red
    delay(d3);              
                             
  // turn the LEDs off
    digitalWrite(2, LOW);
    delay(d3);
    digitalWrite(5, LOW);
    delay(d3);
    digitalWrite(8, LOW);
    delay(d3);
    digitalWrite(11, LOW);;    // turn the LEDs off
    delay(d3);             
}
 void s2() {                          
   // turn the LEDs on reverse
    digitalWrite(11, HIGH);
    delay(d3);
    digitalWrite(8, HIGH);
    delay(d3);
    digitalWrite(5, HIGH); 
    delay(d3);
    digitalWrite(2, HIGH); // leds on red
    delay(d3);              
                             
  // turn the LEDs off reverse
    digitalWrite(11, LOW);
    delay(d3);
    digitalWrite(8, LOW);
    delay(d3);
    digitalWrite(5, LOW);
    delay(d3);
    digitalWrite(2, LOW);    // turn the LEDs off
    delay(d3);             
 }
 
 // xxxxxxxxxxxxxxxxxxx Green Inc xxxxxxxxxxxxxxx
void s3() { //declare function     
  // turn the LEDs on
    digitalWrite(3, HIGH);
    delay(d3);
    digitalWrite(6, HIGH);
    delay(d3);
    digitalWrite(9, HIGH); 
    delay(d3);
    digitalWrite(12, HIGH); // leds on red
    delay(d3);              
                             
  // turn the LEDs off
    digitalWrite(3, LOW);
    delay(d3);
    digitalWrite(6, LOW);
    delay(d3);
    digitalWrite(9, LOW);
    delay(d3);
    digitalWrite(12, LOW);    // turn the LEDs off
    delay(d3);             
}                     
  
  //xxxxxxxxx Green Dec xxxxxxxxxxxxxx
  void s4(){ // turn the LEDs on reverse
    digitalWrite(12, HIGH);
    delay(d3);
    digitalWrite(9, HIGH);
    delay(d3);
    digitalWrite(6, HIGH); 
    delay(d3);
    digitalWrite(3, HIGH); // leds on red
    delay(d3);              
                             
  // turn the LEDs off reverse
    digitalWrite(12, LOW);
    delay(d3);
    digitalWrite(9, LOW);
    delay(d3);
    digitalWrite(6, LOW);
    delay(d3);
    digitalWrite(3, LOW);    // turn the LEDs off
    delay(d3);             
 }
 
// xxxxxxxxxxxxxxxxxxx Blue Inc xxxxxxxxxxxxxxx
void s5() { //declare function     
  // turn the LEDs on
    digitalWrite(4, HIGH);
    delay(d3);
    digitalWrite(7, HIGH);
    delay(d3);
    digitalWrite(10, HIGH); 
    delay(d3);
    digitalWrite(13, HIGH); // leds on red
    delay(d3);              
                             
  // turn the LEDs off
    digitalWrite(4, LOW);
    delay(d3);
    digitalWrite(7, LOW);
    delay(d3);
    digitalWrite(10, LOW);
    delay(d3);
    digitalWrite(13, LOW);;    // turn the LEDs off
    delay(d3);             
}                     
  //xxxxxxxxxxxxxxxxx Blue Dec xxxxxxxxxxxxxxx
  
  void s6(){ // turn the LEDs on reverse
    digitalWrite(13, HIGH);
    delay(d3);
    digitalWrite(10, HIGH);
    delay(d3);
    digitalWrite(7, HIGH); 
    delay(d3);
    digitalWrite(4, HIGH); // leds on red
    delay(d3);              
                             
  // turn the LEDs off reverse
    digitalWrite(13, LOW);
    delay(d4);
    digitalWrite(10, LOW);
    delay(d4);
    digitalWrite(7, LOW);
    delay(d4);
    digitalWrite(4, LOW);;    // turn the LEDs off
    delay(d4);             
 }
 
 // xxxxxxxxxxxxxxxxxxx Violet Inc xxxxxxxxxxxxxxx
void s7() { //declare function     
  // turn the LEDs on
    digitalWrite(2, HIGH);digitalWrite(4, HIGH);
    delay(d3);
    digitalWrite(5, HIGH);digitalWrite(7, HIGH);
    delay(d3);
    digitalWrite(8, HIGH);digitalWrite(10, HIGH); 
    delay(d3);
    digitalWrite(11, HIGH);digitalWrite(13, HIGH); 
    delay(d3);              
                             
  // turn the LEDs off
    digitalWrite(2, LOW);digitalWrite(4, LOW);
    delay(d4);
    digitalWrite(5, LOW);digitalWrite(7, LOW);
    delay(d4);
    digitalWrite(8, LOW);digitalWrite(10, LOW);
    delay(d4);
    digitalWrite(11, LOW);digitalWrite(13, LOW);   
    delay(d4);             
}                     
  //xxxxxxxxxxxxxxxxx V Dec xxxxxxxxxxxxxxx
 void s8() { // turn the LEDs on rev
    digitalWrite(11, HIGH);digitalWrite(13, HIGH);
    delay(d3);
    digitalWrite(8, HIGH);digitalWrite(10, HIGH);
    delay(d3);
    digitalWrite(5, HIGH);digitalWrite(7, HIGH); 
    delay(d3);
    digitalWrite(2, HIGH);digitalWrite(4, HIGH); 
    delay(d3);            
                             
  // turn the LEDs off rev
    digitalWrite(11, LOW);digitalWrite(13, LOW);
    delay(d4);
    digitalWrite(8, LOW);digitalWrite(10, LOW);
    delay(d4);
    digitalWrite(5, LOW);digitalWrite(7, LOW);
    delay(d4);
    digitalWrite(2, LOW);digitalWrite(4, LOW);   
    delay(d4);             
 

}// I had to cut the rest of the functions to less than 9000 chars. They are similar to the above.
}

Laserbee:
I couldn't post all the code due to posting limits.

• Click Reply

• Click Attachments and other options

• Click Browse

• Locate and select the file

• Click OK / Open

• Click Post

int ledPin[] = {2,3,4,5,6,7,8,9,10,11,12,13,21,22,23,24,25}; // initialise output pins

Do you plan to use pins 568, 32421, and -5678 later on?

PaulS:
Do you plan to use pins 568, 32421, and -5678 later on?

Sorry, I don't understand...

PaulS suggests you should use "byte" rather than "int"

These lines are going to cause a problem:

for (int i =0;i<25;i++)
   {
     pinMode(ledPin[i], OUTPUT); // set pins as outputs
   }  
}

The array ledPin does not have this many elements.

Laserbee:
I'm sorry for the"shockingly bad structure"

See planning and implementing a program

...R

You need to learn about data driven programming - put all the information
about LED sequences in data (specifically PROGMEM arrays) and have a short,
simple piece of code interpret the sequences.

PROGMEM means the data is stored in flash, not RAM, which has much more
room but is read-only. Search for examples of this.

Without your entire sketch there's not much point speculating what the problem
is other than d3 has the wrong value in it for some reason.

Laserbee:
I couldn't post all the code due to posting limits.

You can post all code as an .ino file, attached as a "file attachment" to your posting.
Activate "Preview" and then "Attachments and other options" to include a file with your posting.

Laserbee:
I'm sorry for the"shockingly bad structure" as I'm new to this and I have no doubt there are many better ways to do what I am attempting - control 10Amps of RGB through FETs. Any ideas would be very welcome.

Actually your code looks as if it contains much too many lines of redundant code.
This makes your program longer than actually needed and it makes the code error-prone.

You'd better write useful "functions" of your own for similar tasks than always have to be done the same way, providing just some parameter as modifiers when calling the function to make it run a bit different. So perhaps instead of all your switching functions without any parameter like s1(), s2(), s3(), s(4) and so on you possibly would have just ONE function which takes the pin numbers and delay duration as a parameter. Or possibly complete different programming logic to switch "patterns" by using "algorithms" for doing so.

At the moment its not clear to me what you really have as a hardware and how you want to control it.

When reading "RGB" i first thought, you'd use PWM to control the color mixing of RGB LEDs.

But your code looks as if you just control either the R, G and B channels of several LEDs ON or OFF, or if you just switch colored LEDs ON or OFF.

Thankyou again for the helpful replies!

I have tidied up the sketch following the advice posted here and for some reason (I'm not sure which!) the code runs great no matter how long my "call" list is.

It is very kind of you knowledgable guys people to take time out to help us noobs. Very much appreciated indeed.

Thankyou, thankyou, thankyou x