Help with arrays and code simplification

Hello all,

So here is my set up: -28 LEDs in 7 groups of 4, each group with their own pull down resistor to a common ground.
-Based on the reading from my ping sensor, I want the LEDs to progressively turn on. While the threshold of 4 feet has not been cross the first two groups of LEDs will be on (pins 2 &3). Once the threshold is crossed the next two groups will turn on (pins 4 & 5). Again, once another threshold is crossed the next two groups of LEDs will turn on (pins 6 & 7), and past the last threshold the last group will turn on (pin 8.)
-Within the last threshold (1.5 feet away from the sensor) the LEDs will flash from the beginning of the array to the end over and over until a person steps outside of the 1.5 ft threshold

My first issue:
Once a person steps out of the 1.5 foot threshold, I want the LEDs to decay, or in other words, go in the opposite order as you get further away from the sensor. How could I code this so that the turning on and off of the LEDs is fluid and in synch with a persons distance and doesn’t get stuck with all LEDs on and looping.

My second issue:
Is there a more elegant way to call to two different array elements at the same time? So, instead of having two different digitalWrite commands, could I collapse those two into one?

Here is my code thus far:

const int pingPin = 10;

int ledPins[] = { 
  2, 3, 4, 5, 6, 7, 8 };       // an array of pin numbers to which LEDs are attached
int pinCount = 7; 


void setup() {
  // initialize serial communication:
  Serial.begin(9600);

  for (int thisPin = 0; thisPin < pinCount; thisPin++)  {
    pinMode(ledPins[thisPin], OUTPUT);      
  }

}

void loop()
{
  digitalWrite(ledPins[0], HIGH);  //Green LED group 1 
  digitalWrite(ledPins[1], HIGH);  //Green LED group 2  --- need to always both be on together
  
  // establish variables for duration of the ping, 
  // and the distance result in inches and centimeters:
  long duration, inches, cm;

  pinMode(pingPin, OUTPUT);
  digitalWrite(pingPin, LOW);
  delayMicroseconds(2);
  digitalWrite(pingPin, HIGH);
  delayMicroseconds(5);
  digitalWrite(pingPin, LOW);

  pinMode(pingPin, INPUT);
  duration = pulseIn(pingPin, HIGH);

  // convert the time into a distance
  cm = microsecondsToCentimeters(duration);


  Serial.print(cm);
  Serial.print("cm");
  Serial.println();

  delay(100);

  if (cm < 122 && cm > 76){
    digitalWrite(ledPins[2], HIGH);  //Yellow LEDs group1
    digitalWrite(ledPins[3], HIGH);  //Yellow LEDs group2 ---- always need to be on together
  }
  else if (cm < 76 && cm > 46){
    digitalWrite(ledPins[4], HIGH);    //Red LEDs group1
    digitalWrite(ledPins[5], HIGH);    //Red LEDs group2 --- always need to be on together
  }
  else if (cm < 46){
    digitalWrite(ledPins[6], HIGH);  //Red LEDs group 3
  }
  while (cm < 46){
    for (int thisPin = 0; thisPin < pinCount; thisPin++) { 
      // turn the pin on:
      digitalWrite(ledPins[thisPin], HIGH);   
      delay(100);                  
      // turn the pin off:
      digitalWrite(ledPins[thisPin], LOW);    

    }
    // I want the code to be able to go in reverse order
    // and also to not get stuck in a while loop
  }

}





//------------------------------------------------------------//
//ping sensor code

long microsecondsToCentimeters(long microseconds)
{
  // The speed of sound is 340 m/s or 29 microseconds per centimeter.
  // The ping travels out and back, so to find the distance of the
  // object we take half of the distance travelled.
  return microseconds / 29 / 2;
}

Please let me know if there was anything I was unclear about. Thanks in advance for your help.

EDIT:
In attempt to illustrate things a little better, here is a video of my breadboard and LEDs using basic array code. Each color group should both be on at the same time as each other, but they aren’t in this video. http://brigidrose.tumblr.com/post/12207558989/28-leds-in-7-different-segments-of-4-leds-in

Here is what I understand: You have the centimeters part down and good to go, you just want to convert the information into 7 pins, which are each connected to 4 leds. From what you’ve stated, your problem with the “stuck” is due to the while loop.

You are stuck inside this loop because nothing will change the cm variable. To fix this, just call that method inside the loop. Like so.

while (cm < 46)
{
    for (int thisPin = 0; thisPin < pinCount; thisPin++) 
    { 
      digitalWrite(ledPins[thisPin], HIGH);   
      delay(100);                  
      digitalWrite(ledPins[thisPin], LOW);    
    }
    cm = microsecondsToCentimeters(duration);

I assume from the video, you are showing me the result of the while loop.

Each color group should both be on at the same time as each other, but they aren’t in this video.

To make the color groups on at the same time, you need to call the next ledPin. So this is the next step.

while (cm < 46)
{
    for (int thisPin = 0; thisPin < pinCount; thisPin++) 
    { 
      digitalWrite(ledPins[thisPin], HIGH);   
      digitalWrite(ledPins[thisPin+1], HIGH;
      delay(100);                  
      digitalWrite(ledPins[thisPin], LOW);
      digitalWrite(ledPins[thisPin+1], LOW);    
    }
    cm = microsecondsToCentimeters(duration);

This will however, mess with pins 0 and 1. It may or may not matter for you.
As for elegance, just study what the more experienced programmers do, understand it, and emulate it. If it works within your goals, it works. :slight_smile:

This will however, mess with pins 0 and 1.

int ledPins[] = { 
  2, 3, 4, 5, 6, 7, 8 };

How will index = 0 mess with pin 0?

    digitalWrite(ledPins[2], HIGH);  //Yellow LEDs group1
    digitalWrite(ledPins[3], HIGH);  //Yellow LEDs group2 ---- always need to be on together

So, create a function:

void setPair(int index, int state)
{
    digitalWrite(ledPins[index], state);
    digitalWrite(ledPins[index+1], state);
}

Then, call it:

   setPair(2, HIGH);

Catcher and Paul,

Thanks a lot for your help. I took into consideration the code you gave me and tried to rewrite my existing code. What resulted worked well until all of the LEDs had been turned on, then they got stuck looping from the first spot in the array to the last.

EDIT: Once all of the LEDs are activated and start looping my sensor also stops reading, therefore my while statement always tests true.

To be more specific then I was last time, here is a break down of how I want my code to work:

-while ping sensor reads more than 4 feet, first row of green LEDs is on.
-when threshold of 4 feet is crossed the next row of yellow LEDs turns on.
-As distance between object and ping sensor becomes slower, new rows of LEDs turn on until the object is only 1.5 feet from the sensor.
-Once 1.5 foot threshold has been crossed, the last row of red LEDs then turns on and the LEDs begin to flash from the beginning green LEDs to the end red LEDs.
-IF distance between object and ping sensor increase then the LEDs decay in the opposite direction : 1)stop blinking 2)last row turns off 3)second to last row turns off… and so on.

So in other words… the closer to the ping sensor, the more LEDs that are on. The further away, the less that are on. It’s like a graph of distance with lights.

Here is what I did to my code based on your input.

const int pingPin = 10;

int ledPins[] = { 
  2, 3, 4, 5, 6, 7, 8 };       // an array of pin numbers to which LEDs are attached
int pinCount = 7; 


void setup() {
  // initialize serial communication:
  Serial.begin(9600);

  for (int thisPin = 0; thisPin < pinCount; thisPin++)  {
    pinMode(ledPins[thisPin], OUTPUT);      
  }

}

void loop()
{
  setPair(0, HIGH);
  
  // establish variables for duration of the ping, 
  // and the distance result in inches and centimeters:
  long duration, inches, cm;

  pinMode(pingPin, OUTPUT);
  digitalWrite(pingPin, LOW);
  delayMicroseconds(2);
  digitalWrite(pingPin, HIGH);
  delayMicroseconds(5);
  digitalWrite(pingPin, LOW);

  pinMode(pingPin, INPUT);
  duration = pulseIn(pingPin, HIGH);

  // convert the time into a distance
  cm = microsecondsToCentimeters(duration);


  Serial.print(cm);
  Serial.print("cm");
  Serial.println();

  delay(100);

  if (cm < 122 && cm > 76){
    setPair(2, HIGH);
  }
  else if (cm < 76 && cm > 46){
    digitalWrite(ledPins[4], HIGH);    //Red LEDs group1
    digitalWrite(ledPins[5], HIGH);    //Red LEDs group2 --- always need to be on together
  }
  else if (cm < 46){
    digitalWrite(ledPins[6], HIGH);  //Red LEDs group 3
  }
  while (cm <= 46){
    for (int thisPin = 0; thisPin < pinCount; thisPin++) { 
      // turn the pin on:
      digitalWrite(ledPins[thisPin], HIGH);   
      delay(100);                  
      // turn the pin off:
      digitalWrite(ledPins[thisPin], LOW);   
    }
  cm = microsecondsToCentimeters(duration); 

    
    // I want the code to be able to go in reverse order
    // and also to not get stuck in a while loop
  }

}

Lastly, if you follow this linkhttp://brigidrose.tumblr.com/post/12292583620/hey-all-you-brilliant-people…
you can see a journal entry I made about my progress thus far based on another bit of code someone suggested I used. There are a lot of links embedded within that journal entry to look at.

Again, thanks so much for any help you can offer up!

The point about using functions was to get you to start developing and using them. For instance, you read data from the ping sensor. That is not a single step operation. Put all that code in a function, ping() that returns a distance.

  if (cm < 122 && cm > 76){

I find this code hard to read.

if(cm > 76 && cm < 122)

performs the same check, with no head scratching to understand it.

  else if (cm < 76 && cm > 46){

This test will not be reached unless cm is less than (or equal) 76, so the first test is not needed. Less code is nearly always better.

Boundary conditions should abut, not be separated. If cm IS 76, the first test is not true, and neither is the second test. One of them should be, unless 76 has some specific, non-obvious meaning. If it does, and there is a reason that nothing happens at that specific value, there should be some comments that explain why not.

// I want the code to be able to go in reverse order
// and also to not get stuck in a while loop

Then a while loop is almost certainly not what you want to be using.

Put all that code in a function, ping() that returns a distance.

When you've done this, make sure you call it to get a value for cm in your while loop too - should stop it getting stuck there. As you have it now, once you're in the while, duration will never change.

brigidrose:
-while ping sensor reads more than 4 feet, first row of green LEDs is on.
-when threshold of 4 feet is crossed the next row of yellow LEDs turns on.
-As distance between object and ping sensor becomes slower, new rows of LEDs turn on until the object is only 1.5 feet from the sensor.
-Once 1.5 foot threshold has been crossed, the last row of red LEDs then turns on and the LEDs begin to flash from the beginning green LEDs to the end red LEDs.
-IF distance between object and ping sensor increase then the LEDs decay in the opposite direction : 1)stop blinking 2)last row turns off 3)second to last row turns off… and so on.

I think the problem is that your code doesn't ever turn the LEDS off, except for inside the while loop that gets stuck.

What I'd do is to write down at each distance range exactly which LEDs need to be on and which ones need to be off. So for example at the range 76-122cm you might want pair 0,1 on, pair 2,3 on, pair 4,5 off and LED 6 off. Then use setPair and digitalWrite to set them high or low as you need (3 setPairs and 1 digitalWrite). Repeat that for all the distance ranges. The aim is to be in control of every LED at every distance range. The code will probably look a bit ugly and repetitive but it should work.

Thank you for your help everyone! I am rewriting everything now and I will let you know how it goes.

Paul, I don't mean to let my newness show too much, but could you explain why using while is not optimal? Should I include a do with the while? Should I just stick with if and else? When it comes to creating conditional statements I still have a struggle with trying to think like my computer and not bog it down with superfluous code.

Paul, I don't mean to let my newness show too much, but could you explain why using while is not optimal?

Not optimal and not right are two different things. There is nothing in what you said you wanted to do that involves doing stuff while some condition is true.

The loop() function gets called over and over. I know you said that "while ping sensor reads more than 4 feet, first row of green LEDs is on.", but really, what you should have said was "if ping sensor reads more than 4 feet...". There is, in this case, no need for a while loop in an infinite loop.

Should I include a do with the while?

How would that help. Do/while is just a variation of while, where the condition is checked at the end rather that at the beginning.

When it comes to creating conditional statements I still have a struggle with trying to think like my computer and not bog it down with superfluous code.

Because you are thinking in terms of while some condition is true, rather than in terms of if some condition is true each time loop() is called.

Keep in mind that an if test is evaluated hundreds, thousands, or millions of times a second, and you can see that while is rarely needed.

SUCCESS! Thank you everyone for your help!

Here is my completed working code.

const int pingPin = 10;

int ledPins[] = { 
  2, 3, 4, 5, 6, 7, 8 };       // an array of pin numbers to which LEDs are attached
int pinCount = 7; 

long duration, inches, cm;

void setup() {
  // initialize serial communication:
  Serial.begin(9600);

  for (int thisPin = 0; thisPin < pinCount; thisPin++)  {
    pinMode(ledPins[thisPin], OUTPUT);      
  }

}

void loop()
{
  runPing();
  setPair(0, HIGH);

}

void setPair(int thisPin, int state)
{
  digitalWrite(ledPins[thisPin], state);
  digitalWrite(ledPins[thisPin+1], state);
}

long runPing(){

  pinMode(pingPin, OUTPUT);
  digitalWrite(pingPin, LOW);
  delayMicroseconds(2);
  digitalWrite(pingPin, HIGH);
  delayMicroseconds(5);
  digitalWrite(pingPin, LOW);

  pinMode(pingPin, INPUT);
  duration = pulseIn(pingPin, HIGH);

  // convert the time into a distance
  cm = microsecondsToCentimeters(duration);

  Serial.print(cm);
  Serial.print("cm");
  Serial.println();

  delay(300);
  test();
}

long microsecondsToCentimeters(long microseconds)
{
  // The speed of sound is 340 m/s or 29 microseconds per centimeter.
  // The ping travels out and back, so to find the distance of the
  // object we take half of the distance travelled.
  return microseconds / 29 / 2;
}

void test(){
  if (cm > 76 && cm < 122){
    setPair(2, HIGH);
  }
  else if(cm > 122){
    setPair(2, LOW);
  }

  if (cm > 46 && cm < 76){
    setPair(4, HIGH);
    setPair(2, HIGH);
  }
  else if(cm > 76){
    setPair(4, LOW);
  }

  if (cm < 46){
    digitalWrite(ledPins[6], HIGH);  //Red LEDs group 3

    for (int thisPin = 0; thisPin < pinCount; thisPin++) { 
      // turn the pin on:
      digitalWrite(ledPins[thisPin], HIGH);   
      delay(100);                  
      // turn the pin off:
      digitalWrite(ledPins[thisPin], LOW);   
    }
  }
  else if (cm > 46){
    digitalWrite(ledPins[6], LOW);
  }
}

Not to rain on your parade, or anything like that, but…

if (cm > 76 && cm < 122){
setPair(2, HIGH);
}
else if(cm > 122){
setPair(2, LOW);
}

What happens if cm is 76?

  if (cm > 46 && cm < 76){
    setPair(4, HIGH);
    setPair(2, HIGH);
  }
  else if(cm > 76){
    setPair(4, LOW);
  }

Again, 76 causes nothing to happen.

if (cm < 46){
}
else if (cm > 46){
}

This time, you are doing nothing if cm is 46.

Why is that?

void test(){
  if (cm >= 76 && cm <= 122){
    setPair(2, HIGH);
  }
  else if(cm >= 122){
    setPair(2, LOW);
  }

  if (cm >= 46 && cm <= 76){
    setPair(4, HIGH);
    setPair(2, HIGH);
  }
  else if(cm >= 76){
    setPair(4, LOW);
  }

  if (cm <= 46){
    digitalWrite(ledPins[6], HIGH);  //Red LEDs group 3

    for (int thisPin = 0; thisPin < pinCount; thisPin++) { 
      // turn the pin on:
      digitalWrite(ledPins[thisPin], HIGH);   
      delay(100);                  
      // turn the pin off:
      digitalWrite(ledPins[thisPin], LOW);   
    }
  }
  else if (cm >= 46){
    digitalWrite(ledPins[6], LOW);
  }

Can I have my parade back now?

Can I have my parade back now?

Sure. Watch where you step. There are horses and elephants up front.

Sure. Watch where you step. There are horses and elephants up front.

Charming!