Do I need to use Loops? Help with streamlining code - Beginner Questions

Projects Programming
1

Hi, first post here.
I'm looking for help in streamlining the attached code.
It's very basic (as I'm a novice programmer) and is based around an Arduino tutorial.
The end result is a 'chasing' effect involving three LEDS with varying brightnesses, across a total of six LEDS.

I wanted to write it to put my (limited) knowledge of 'C' and the Arduino to use and build further upon what I learned in the tutorials.
(The circuit I have made is very simple, so I felt it unnecessary to include a schematic. It uses pins ~3, ~5, ~6, ~9, ~10, ~11 and x1 220ohm resistor & x1 LED for each. (My issue is really with the programming and not the circuit!))

My first draft progressed from one LED up to six and the code became lengthy very quickly. It works, but I'm sure there must be a way to streamline it, so I began working on a 2nd draft (see below).

In the second draft my intention is to simply set "ledsUsed = 'x'" and achieve the same result of fading LEDS over 'x' LEDS... but with far less code.
I have started with a nested 'for' loop which iterates through the correct number of times based on ledsUsed. But afer that, I am stumped. I guessed I would need to call a function to determine brightnesses..?

I am really not looking for somebody to spoon-feed me a solution per se, but some guidance on what I should look into/learn to use in 'C' would be very much appreciated, or perhaps what/how I should be thinking in order to solve my problem.
(I understand that the Arduino accepts C# too, but I felt that I needed to get a good grasp on 'C' before moving upto 'C#'.)

Many Thanks,
Tom

First draft:

int totalLeds = 6;                            //Total LEDS connected to Uno

int delayTime = 100;                          //Delay time(ms)

int bri[] = {100, 33, 5, 0};                  //Brightness array
int led[] = {3, 5, 6, 9, 10, 11};             //Uno pin number array

void setup() {
  // put your setup code here, to run once:
  for (int i = 0; i < totalLeds; i++) {
    pinMode(led[i], OUTPUT);
  }
}

void loop() {
  useSix();
}

void useOne() {
  analogWrite(led[0], bri[0]);
  delay(delayTime);

  analogWrite(led[0], bri[1]);
  delay(delayTime);

  analogWrite(led[0], bri[2]);
  delay(delayTime);

  analogWrite(led[0], bri[3]);
  delay(delayTime);
}

void useTwo() {
  analogWrite(led[0], bri[0]);
  delay(delayTime);

  analogWrite(led[1], bri[0]);
  analogWrite(led[0], bri[1]);
  delay(delayTime);

  analogWrite(led[1], bri[1]);
  analogWrite(led[0], bri[2]);
  delay(delayTime);

  analogWrite(led[1], bri[2]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);
}

void useThree() {
  analogWrite(led[0], bri[0]);
  delay(delayTime);

  analogWrite(led[1], bri[0]);
  analogWrite(led[0], bri[1]);
  delay(delayTime);

  analogWrite(led[2], bri[0]);
  analogWrite(led[1], bri[1]);
  analogWrite(led[0], bri[2]);
  delay(delayTime);

  analogWrite(led[2], bri[1]);
  analogWrite(led[1], bri[2]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[2], bri[2]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);
}

void useFour() {
  analogWrite(led[0], bri[0]);
  delay(delayTime);

  analogWrite(led[1], bri[0]);
  analogWrite(led[0], bri[1]);
  delay(delayTime);

  analogWrite(led[2], bri[0]);
  analogWrite(led[1], bri[1]);
  analogWrite(led[0], bri[2]);
  delay(delayTime);

  analogWrite(led[3], bri[0]);
  analogWrite(led[2], bri[1]);
  analogWrite(led[1], bri[2]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[3], bri[1]);
  analogWrite(led[2], bri[2]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[3], bri[2]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[3], bri[3]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);
}

void useFive() {
  analogWrite(led[0], bri[0]);
  delay(delayTime);

  analogWrite(led[1], bri[0]);
  analogWrite(led[0], bri[1]);
  delay(delayTime);

  analogWrite(led[2], bri[0]);
  analogWrite(led[1], bri[1]);
  analogWrite(led[0], bri[2]);
  delay(delayTime);

  analogWrite(led[3], bri[0]);
  analogWrite(led[2], bri[1]);
  analogWrite(led[1], bri[2]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[4], bri[0]);
  analogWrite(led[3], bri[1]);
  analogWrite(led[2], bri[2]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[4], bri[1]);
  analogWrite(led[3], bri[2]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[4], bri[2]);
  analogWrite(led[3], bri[3]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[4], bri[3]);
  analogWrite(led[3], bri[3]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);
}

void useSix() {
  analogWrite(led[0], bri[0]);
  delay(delayTime);

  analogWrite(led[1], bri[0]);
  analogWrite(led[0], bri[1]);
  delay(delayTime);

  analogWrite(led[2], bri[0]);
  analogWrite(led[1], bri[1]);
  analogWrite(led[0], bri[2]);
  delay(delayTime);

  analogWrite(led[3], bri[0]);
  analogWrite(led[2], bri[1]);
  analogWrite(led[1], bri[2]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[4], bri[0]);
  analogWrite(led[3], bri[1]);
  analogWrite(led[2], bri[2]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[5], bri[0]);
  analogWrite(led[4], bri[1]);
  analogWrite(led[3], bri[2]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[5], bri[1]);
  analogWrite(led[4], bri[2]);
  analogWrite(led[3], bri[3]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[5], bri[2]);
  analogWrite(led[4], bri[3]);
  analogWrite(led[3], bri[3]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

  analogWrite(led[5], bri[3]);
  analogWrite(led[4], bri[3]);
  analogWrite(led[3], bri[3]);
  analogWrite(led[2], bri[3]);
  analogWrite(led[1], bri[3]);
  analogWrite(led[0], bri[3]);
  delay(delayTime);

}

Second draft:

const int totalLeds = 6;                            //Total number of LEDS used on Uno.

int ledsUsed = 6;                                   //Amount of LEDS to use in chase effect.
int led[] = {3, 5, 6, 9, 10, 11};                   //Array containing Uno LED pin numbers.

int delayTime = 500;

void setup() {
  // put your setup code here, to run once:
  for (int i = 0; i < totalLeds; i++) {
    pinMode(led[i], OUTPUT);                          //Do this while i < totalLeds
  }
}

void loop() {
  for (int a = 0; a < (ledsUsed + 3); a++) {
    for (int b = 0; b < ledsUsed; b++) {                          //Do this (ledsUsed + 3) times.

      //analogWrite(led[b], findBrightness(b));                   //Do this while (b < ledsUsed). 
                                                                  //findBrightness(b) calls function findBrightness with 'b' as parameter.
      delay(delayTime);
    }
  }
}

void findBrightness(int b) {
 //Looking for help with this function?
}
2

Hi tomtomhitter,

(hope the TomTom is still functional ;-))

you defined a function "findBrightness"

as

void findBrightness(int b)

void means the function does return nothing.

then you try to use the function as a parameter in analogWrite

analogWrite(led[b], findBrightness(b));

this would require a function definition with a return-value
analogWrite expects an integer between 0 and 255
so your function-definition would look like this

byte findBrightness(int b) {
// do whatever nescessary

return myVar
}

a parameter named "b" says nothing what the parameter is good for
you should always use selfexplaining names
It is not clear to me what kind of functionality you want to achieve by using your function findBrightness
If I understood right you want to have a "knight-Rider-" effect on your 6 LEds
K I T T Scanner Sound HD - YouTube
This means the leds shall become brighter and darker in a time-shifted way.
So there are two things fade in / fade out each LED
start the fade in for each LED some time later than the "leading" led
inverse direction and do the same backwards.
So I would divide it in these steps.
As the LEDs have to change brighntes in parallal you have to avoid the function delay()
delay() does what its name says delay() maybe it should have been named delayEVERYTHING()
because this what it does.
You will have to use timing based on the function millis() which is a bit advanced
Timing using millis() has a very different concept. You have to undersatnd this different concept first.
You don't just wait like delay()
you do a very fast repeated compare of the actual time with a certain well defined timepoint.
You can imagine this like baking a frosted pizza
baking-time 10 minutes
you take a look onto your watch 13:05
and do calculate a time-difference
currentTime - startTime
13:05 - 13:05 = 0 minutes (smaller than baking time) => pizza not ready yet
13:06 - 13:05 = 1 minutes (smaller than baking time) => pizza not ready yet
13:07 - 13:05 = 2 minutes (smaller than baking time) => pizza not ready yet
13:08 - 13:05 = 3 minutes (smaller than baking time) => pizza not ready yet
13:09 - 13:05 = 4 minutes (smaller than baking time) => pizza not ready yet
13:10 - 13:05 = 5 minutes (smaller than baking time) => pizza not ready yet
13:11 - 13:05 = 6 minutes (smaller than baking time) => pizza not ready yet
13:12 - 13:05 = 7 minutes (smaller than baking time) => pizza not ready yet
13:13 - 13:05 = 8 minutes (smaller than baking time) => pizza not ready yet
13:14 - 13:05 = 9 minutes (smaller than baking time) => pizza not ready yet
13:15 - 13:05 = 10 minutes equal to baking-time pizza ready to eat
In code This is done by a special designed if-condition

if ( currentTime - previousTime) >= WaitingTime) {
// start timed action
}

You did a repeated comparing how much time has passed by
This is what non-blocking timing based on function millis() does

here is a tutorial about it
baldengineers: millis() Tutorial: Arduino Multitasking

best regards Stefan

3

Thank you Stefan. Some great help there!
(tomtom refers to the drums - as in, i'm a drummer. My tomtom satnav died in the winter of '07)

The Knight Rider effect is almost what I was looking for. (Great video btw!)
My original idea was to have it only "sweeping" in one direction from LED 0 -> LED 'x'. Allowing final LED 'x' to be defined by:
int ledsUsed = x;

But, I understand your millis() concept and will start my research there. (Maybe it won't be that difficult to split it down the middle and use it in one direction only, and then tie it into ledsUsed).

Also, thank you for the notes about my function findBrightness().
I understand that I hadn't returned a value using it but had intended the function to simply run a secondary repetative code block using int b as a variable available in that scope - do void functions not work in that way?
Leaving the function written as it appeared was purely my way of inferring that I might use some function to find the brightness of 'b'; ('b' being determined by the current iteration of the for loop), I had no idea what code should be included in the function to do this, but that was my thinking. My apologies.
i.e.
for loop is currently at LED 3
so, LED 'b' = Bri 100.
LED 'b-1' = Bri 33.
LED 'b-2' = Bri 5.
(Hope that makes sense)

Thanks again.

4 
void loop()
{
  for (int a = 0; a < (ledsUsed + 3); a++)
  {
    for (int b = 0; b < ledsUsed; b++)                            //Do this (ledsUsed + 3) times.
    {
      //analogWrite(led[b], findBrightness(b));                   //Do this while (b < ledsUsed).
      //findBrightness(b) calls function findBrightness with 'b' as parameter.
      delay(delayTime);
    }
  }
}

I think 'findBrightness()' needs to know both 'a' and 'b' to look up the brightness. And it should return a value.

byte findBrightness(int headPosition, int whichLED)
{
  if (whichLED == headPosition)  // 'head' of the commet
    return 100;
  if (whichLED == headPosition - 1) // middle of the commet.
    return 33;
  if (whichLED == headPosition - 2) // tail of the commet.
    return 5;
  return 0;  // Outside the commet
}
5

tomtomhitter:
my way of inferring that I might use some function to find the brightness of 'b'; ('b' being determined by the current iteration of the for loop),

The FadeLed library GitHub - septillion-git/FadeLed: Makes fading LEDs on an Arduino easy has functions/methods to do this for you, among others. The library could handle LED brightness but you'd still have to manage the sequencing.

6

I'd..

//Set up your arrays different..

int pins[] = {3, 5, 6, 9, 10, 11}; //Uno pin number array.
int bright[6]; // How bright they are..

// Then I'd write a few functions..

// Set the brighness value for any LED 0..6
void setBrite(int LED,int brightVal) {

bright[LED] = brightVal;
}

// read back the brightness value from any LED 0..6
int readBright(int LED) {

return bright[LED] ;
}

// Made ALL your LED values this brightness
void setAll(int brightVal) {

for(int i=0;i<6;i++) {
bright*, brightVal;*

  • }*
    }
    // Move whatever patter you have on your LEDs shift over by one.
    void rollOne(void) {

  • int temp;*

  • temp = bright[5];*

  • for(int i=4;i>=0;i--) {*
    _ bright[i+1] = bright*;_
    _     }_
    _     bright[0] = temp;_
    _    }_
    _    // Make the LEDs reflect your saved brightness values._
    _    // This takes whatever you did in the other functions show up on your LEDs._
    void show(void) {
    _     for(int i=0;i<6;i++) {_
    _ analogWrite(led[0], bright);
     }
    }
    Now with these simple building blocks, your problem, and many others should be a breeze to solve.
    Hint, Set them all to zeo..
    setAll(0) ;
    Then set the pattern into your pattern buffer. using setLED() calls.
    then show them,
    roll them
    wait
    show them again.
    -jim lee*_

7

Well, hours later, I've solved it.

I spent some time going over the suggested methods and they all seem to make sense to me and definitely offer valuable further reading/experimenting. Thank you. I think (hope) I ended up learning something and put it into practice in my code.

Here's my finished code. I'm not sure that it conforms to "best practices" (multiple nested for loops etc) and it is still a little bulky in the ledRunOff() function... but, it's basic, it works and I'm happy.

I am able to change ledsUsed to x.
I also added a restartTime to delay between pulses.

I think I will now explore using other inputs, such as POTs or Serial, to set ledsUsed and restartTime and perhaps add a button to turn it on/off. Or, maybe I should just follow a 'fairy light' tutorial!!! I think I may end up using the millis(); function when using a button, as I now understand that delay(); literally pauses the code.

Anyway, it's been a nice learning curve in C programming basics such as loops, functions and basic logic. Again, thank you everybody for your help!!

const int totalLeds = 6;                            //Total number of LEDS used on Uno.

int ledsUsed = 6;                                   //Amount of LEDS to use in chase effect.
int led[] = {3, 5, 6, 9, 10, 11};                   //Array containing Uno LED pin numbers.

int delayTime = 75;                                 //Delay time between LEDS lighting.
int restartTime = 500;                              //Interval between pulses.

void setup() {
  // put your setup code here, to run once:
  for (int i = 0; i < totalLeds; i++) {
    pinMode(led[i], OUTPUT);
  }
}

void loop() {
  for (int a = 0; a < (ledsUsed + 3); a++) {
    for (int ledNumber = 0; ledNumber < ledsUsed; ledNumber++) {
      analogWrite(led[ledNumber], findBrightness(ledNumber, ledNumber));
      for (int checkTail = 0; checkTail <= ledNumber; checkTail++) {
        analogWrite(led[checkTail], findBrightness(checkTail, ledNumber));
      }
      delay(delayTime);
      if (ledNumber == ledsUsed - 1) {
        ledRunOff(ledNumber);
      }
    } delay(restartTime);
  }
}

void ledRunOff(int ledNumber) {
  analogWrite(led[ledNumber], 33);
  analogWrite(led[ledNumber - 1], 5);
  analogWrite(led[ledNumber - 2], 0);
  delay(delayTime);
  analogWrite(led[ledNumber], 5);
  analogWrite(led[ledNumber - 1], 0);
  delay(delayTime);
  analogWrite(led[ledNumber], 0);
  delay(delayTime);

}

int findBrightness(int whichLED, int headPosition) {
  if (whichLED == headPosition)
    return 100;
  if (whichLED == headPosition - 1)
    return 33;
  if (whichLED == headPosition - 2)
    return 5;
  return 0;
}