Rules on subroutines / if statements?

I'm rewriting some code that I have been working on which interfaces an Arduino Uno and a TLC5940 chip. The program has 4 subroutines and then the main part of the program checks a combination of 4 separate digital inputs (all connected to switches that send either a high or low signal- none floating) to determine which subroutine to execute. The subroutines all execute fine individually, and I can call 3 of the 4 without any problem but when the 4th subroutine is added to the program the chasing lights dim and almost act as if there is a floating input. I combined two of the subroutines together and now the circuit looks like it is working normally. Is there a rule to how many subroutines / if statements can be executed in the main part of the program? I can provide code if needed but wondered if anyone had any experience with this kind of problem in general.

Is there a rule to how many subroutines / if statements can be executed in the main part of the program?

No.

I can provide code if needed but wondered if anyone had any experience with this kind of problem in general

You should probably post your code.

Failing code:

#include "Tlc5940.h"

const int spdPin = 2;     // the number of the pin connected to speed trigger
const int rdyPin = 4;     // signal ready
const int tcPin = 5;      // signal circuit is on
const int pwrPin = 6;     // signal sufficient power 
int spd = 0;
int bright = 0;
int count = 0;
int randon = 0;
int randnum = 0;
int rdy = 0;
int tc = 0;
int pwr = 0;

void chase()
{
  for (int channel = 1; channel < 7; channel ++) 
  {
    /* Tlc.clear() sets all the grayscale values to zero, but does not send
       them to the TLCs.  To actually send the data, call Tlc.update() */
    Tlc.clear();
    Tlc.set(channel, 1000);
    Tlc.update();
    delay(21);
  }  
}

void charge()
{
  for (int bright = 0; bright < 3000;) 
  {
    for (int channel = 1; channel < 7; channel ++)
    {
      spd = digitalRead(spdPin);
      Tlc.clear();
      Tlc.set(7, bright);
      Tlc.set(channel, 1000);
      Tlc.update();
      delay(21);
    }
    if (spd == LOW)
    {
      bright = 3000;
    }
    else
    {
      bright = bright + 300;
    }
  }   
}

void activate()
{
  count = 0;
  Tlc.set(1, 4000);
  Tlc.set(2, 4000);
  Tlc.set(3, 4000);
  Tlc.set(4, 4000);
  Tlc.set(5, 4000);
  Tlc.set(6, 4000);
  Tlc.set(7, 4000);
  Tlc.update();
  do
  {
    spd = digitalRead(spdPin);
    delay(1);
    count++;
  } while (spd == HIGH && count < 6000);
}
  
void fail()
{
  for (int channel = 1; channel < 7; channel ++)
  {
    randnum = random(0, 6);
    randon = random(10,900);
    Tlc.clear();
    if (randnum % channel == 0)
    {
      Tlc.set(7, randon);
    }
    Tlc.set(channel, 1000);
    Tlc.update();
    delay(21);    
  }
}

void setup()
{
  /* Call Tlc.init() to setup the tlc.
     You can optionally pass an initial PWM value (0 - 4095) for all channels.*/
  Tlc.init();
  pinMode(spdPin, INPUT);
  pinMode(pwrPin, INPUT);
  pinMode(tcPin, INPUT);
  pinMode(rdyPin, INPUT);
}

/*  NUM_TLCS is defined in "tlc_config.h" in the
   library folder.  After editing tlc_config.h for your setup, delete the
   Tlc5940.o file to save the changes. */

void loop()
{
  spd = digitalRead(spdPin);
  pwr = digitalRead(pwrPin);
  tc = digitalRead(tcPin);
  rdy = digitalRead(rdyPin);
  
  chase();
  
  if (tc == HIGH && rdy == HIGH)
  {
    if(spd == HIGH)
    {
      if(pwr == HIGH)
      {
        charge();
        activate();
       }
     }
     else
     {
       fail();
     }
   }
 
}

Working code (charge and activate subroutines combined):

#include "Tlc5940.h"

const int spdPin = 2;     // the number of the pin connected to speed trigger
const int rdyPin = 4;     // signal ready
const int tcPin = 5;      // signal circuit is on
const int pwrPin = 6;     // signal sufficient power 
int spd = 0;
int bright = 0;
int count = 0;
int randon = 0;
int randnum = 0;
int rdy = 0;
int tc = 0;
int pwr = 0;

void chase()
{
  for (int channel = 1; channel < 7; channel ++) 
  {
    /* Tlc.clear() sets all the grayscale values to zero, but does not send
       them to the TLCs.  To actually send the data, call Tlc.update() */
    Tlc.clear();
    Tlc.set(channel, 1000);
    Tlc.update();
    delay(21);
  }  
}

void charge()
{
  for (int bright = 0; bright < 3000;) 
  {
    for (int channel = 1; channel < 7; channel ++)
    {
      spd = digitalRead(spdPin);
      Tlc.clear();
      Tlc.set(7, bright);
      Tlc.set(channel, 1000);
      Tlc.update();
      delay(21);
    }
    if (spd == LOW)
    {
      bright = 3001;
    }
    else
    {
      bright = bright + 300;
    }
  }
  if (bright != 3001)
  {
    count = 0;
    Tlc.set(1, 4000);
    Tlc.set(2, 4000);
    Tlc.set(3, 4000);
    Tlc.set(4, 4000);
    Tlc.set(5, 4000);
    Tlc.set(6, 4000);
    Tlc.set(7, 4000);
    Tlc.update();
    do
    {
      spd = digitalRead(spdPin);
      delay(1);
      count++;
    } while (spd == HIGH && count < 6000);
  }
  
}

void fail()
{
  for (int channel = 1; channel < 7; channel ++)
  {
    randnum = random(0, 6);
    randon = random(10,900);
    Tlc.clear();
    if (randnum % channel == 0)
    {
      Tlc.set(7, randon);
    }
    Tlc.set(channel, 1000);
    Tlc.update();
    delay(21);    
  }
}

void setup()
{
  /* Call Tlc.init() to setup the tlc.
     You can optionally pass an initial PWM value (0 - 4095) for all channels.*/
  Tlc.init();
  pinMode(spdPin, INPUT);
  pinMode(pwrPin, INPUT);
  pinMode(tcPin, INPUT);
  pinMode(rdyPin, INPUT);
}

/*  NUM_TLCS is defined in "tlc_config.h" in the
   library folder.  After editing tlc_config.h for your setup, delete the
   Tlc5940.o file to save the changes. */

void loop()
{
  spd = digitalRead(spdPin);
  pwr = digitalRead(pwrPin);
  tc = digitalRead(tcPin);
  rdy = digitalRead(rdyPin);
  
  chase();
  
  if (tc == HIGH && rdy == HIGH)
  {
    if(spd == HIGH)
    {
      if(pwr == HIGH)
      {
        charge();
      }
      else
      {
        fail();
      }
     }
   }
 
}

Describe your hardware. How many LEDs is the 5940 driving and what kind of power supply are you using to drive them?

In the working code, the code that is in activate() is conditionally called.

In the no-working code, the activate method is unconditionally called.

Though, since bright is either 0, 3000, or incremented by 300, I don't see that that makes any difference.

The subroutines all execute fine individually, and I can call 3 of the 4 without any problem but when the 4th subroutine is added to the program the chasing lights dim and almost act as if there is a floating input.

Would be nice to know which 3 work, and what the 4th one is that fails.

James, The prototyping breadboard that I am using also has a built in 5V power supply which is what I am primarily using to power the Arduino, TLC5940 and the LEDs. The circuit does run off the USB but I only connect the USB when I am uploading a new program because I know that USB cannot supply much current. As for LEDs, I am driving a total of 7. There is a current limiting resistor on the TLC5940 to limit the current to each channel @20mA. However, thinking current might still be an issue I removed all but one chasing LED and the active LED. This did not fix the issue. I tried new jumper wires, I tried moving to a different spot on the breadboard, I tried tying the inputs directly to 5V to eliminate the possibility of a bad switch (although the switches act fine with the other code I posted). I also changed the input pin numbers in case I had a bad input pin. Nothing.

Ironically, the subroutine that causes the "failing" code to operate erratically is the "fail" subroutine. I can leave it written in the program, but when I make the call in the main part the circuit does not like it. If I combine the "charge" and "activate" subroutines, I can use the "fail" code.

What baffles me is that if current were an issue or an input were floating, removing a subroutine should not have solved the problem since it is not called until the speed trigger (operated by a momentary switch with a pulldown resistor so it is not floating when open) is thrown. If the "fail" subroutine were bad then I should not be able to use it at all. I can call any other subroutine in place of the "fail" and the circuit works fine.

I'll keep plugging away at it until I figure out what is going on.

Ok,
Quick update. Apparently commenting out the “randon” declaration in the fail subroutine and substituting it with a constant (so that there is only one random generation done in the subroutine) allows the circuit to work fine. Now the subroutine looks like this:

void fail()
{
  for (int chan = 1; chan < 7; chan ++)
  {
    randnum = random(0, 6);
    //randon = random(10,900);
    Tlc.clear();
    if (randnum % chan == 0)
    {
      Tlc.set(7, 500);
    }
    Tlc.set(chan, 1000);
    Tlc.update();
    delay(21);    
  }
}

Removing the commented line and leaving the constant in place causes the circuit to act up again. Thoughts?

What happens if you comment out the other call to random, instead?

Same result. It will accept the routine with just one random call regardless of which call it is. Put both of them together and it doesn't seem to like it.

Probably unrelated but: random() returns a long. Both of your random number variables are declared as integers.

Just to clarify you have a few programming/C concepts mixed up.

commenting out the “randon” declaration in the fail subroutine and substituting it with a constant

In your code, you aren’t declaring anything fail(). You are assigning values.

In those assignments you aren’t assigning the variables a constant. You are assigning them the value generated by random() (which is called using constants.)

James, Ok, you got me on the "declaration" part. In this day and age of poor spelling and grammar, I thought I was doing well enough to get the point across. Sorry. As far as the "constant" part, you will notice that when I commented out the "randon" variable assignment I replaced the variable in the Tlc.set declaration with a fixed value of 500. 500 is a constant, and is taking the place of where a variable once was. What I was trying to say is that even if I don't use the variable "randon", the assignment is enough to throw the circuit off. However, a sincere thanks to you for the suggestion and I will try that when I get home. Couldn't hurt. Take care.

-Jeremy

My comments on definition were only intended for clarification, not any form of criticism.

In your original explanation, you left out where you were using randon, so it was confusing to understand what you meant about changing to a constant.

Now that I understand, here is a random (excuse the pun) thought. Try: randon = int(random(0,900));

James,
Thank you for the suggestion. I will try that out later this afternoon and let you know. Even though I created an acceptable workaround, it is great for my learning purposes to try and figure out where I went wrong so that in the development of future projects I can avoid making the same mistakes. :slight_smile:

-Jeremy

I changed the assignments for both randon and randnum to what was suggested and the circuit still acts up. If I leave the code the way it was and remove the mod operator (substituting just regular division instead), the circuit seems to like that.

Please do not see this as trying to debunk the offered help, putting a larger data type in a smaller one is bound to invoke problems, so whatever fixes are suggested.. I endorse the same, this is merely for discussion and enlightment purposes.

If random(0, 900) is used, shouldn't you be able to save it directly in an int without casting it down? It should truncate the excess bits off the value, and simply store the 16 least significant bits in the int value? Now taking that assumption, the value would range between 0 and 900, which falls into the valid positive range of a signet 16 bit integer, so no dataloss would be present due to the value restrictions imposed on the random call. Thus making the return value a valid unchecked cast. Please do correct me if I'm wrong?

I wasn't sure about the casting. You are probably right since the value generated fits inside of an int, it shouldn't be a problem. However, it was the only obvious issue I could see with the code so I thought it would be good to at least try.