arduino - Why does the ledstrip stop partway and then go all the way and can I make this more efficient

Question

So I need some help reviewing and editing the code below. Is there a way to make it more efficient instead of what I did? Also, are there better ways to do some of the methods I did? Answers are appreciated greatly (I'm fairly new to Arduino).

The code below is for a corridor ledstrip that lights up when the distance sensor on the Arduino detects anything below a certain distancenear it (yes, I thought of using a motion sensor but I only had distance sensors on hand).

const int trigPin = 9;
const int echoPin = 10;

float duration, distance;
#include <FastLED.h>
#define LED_PIN     6
#define NUM_LEDS    60
int timingnum = 0;
//#define BRIGHTNESS  bright
int bright = 100;///the brightness of the leds
int lednum = 60;///the number of leds available
int timer;
CRGB leds[NUM_LEDS];

void setup() {
  pinMode(trigPin, OUTPUT);
  pinMode(echoPin, INPUT);
  Serial.begin(9600);
  FastLED.addLeds<WS2812, LED_PIN, GRB>(leds, NUM_LEDS);
  // FastLED.setBrightness(  BRIGHTNESS );
}

void loop() {
  lights();
}
        
void sensor() { ///THIS IS CLEAN
  digitalWrite(trigPin, LOW);
  delayMicroseconds(timingnum);
  digitalWrite(trigPin, HIGH);
  delayMicroseconds(timingnum);
  digitalWrite(trigPin, LOW);

  duration = pulseIn(echoPin, HIGH);
  distance = (duration*.0343)/2;
  Serial.print("Distance: ");
  Serial.println(distance);
}

// in the lights the reason why it wouldnt output the data for the 
// represented while loops is because the while loop continued running 
// without the data from the distance and thus it needs to constantly 
// be updated in the while loop conditional statments
void lights() { 
  sensor(); //data update
  while (distance <= 10) {
    for (int x = 0; x <= 60; x++) {
      leds[x] = CRGB(255,255,255);
      FastLED.show();
      sensor(); //sensor update
    }
    for (timer = 0; timer <=800; timer++) {
      sensor();
    }
    sensor(); //replaces the data updates above
  }
  sensor(); //sensor update
  while (distance >= 11) {
    for (int x = 0; x <= 60; x++) {
      leds[x] = CRGB(0,0,0);
      FastLED.show();
    }
    sensor(); //data update
  }
}

Answer 1

I would be tempted to make a little function to set the RGB colors of your LED strip: void fillup(byte r, byte g, byte b). Then you would call it in two places: fillup(0,0,0) and fillup(127,127,127) (yeah, not 100% on).

Also, I'm confused why you have so many sensor() calls in the first while loop. Seems like you only need to call it once when you need updated values for distance.

Also, the comments on the sensor calls are confusing... after reading them I have no idea what sensor is supposed to do. I tend to put a comment before functions that describes what they do, and put a stand-alone comment in the code to explain what the next "paragraph" does. And I avoid banners - they get in the way.

Do you want FastLED.show() inside the loop that updates the colors? Or just after the loop, to update the hardware after the array is changed? IDK, just asking.

I usually do not mind globals, but in this case you would be better off letting sensor return the distance. Then you could while ( sensor() <= 10 ). Or use a do.. while loop with one call to sensor at the bottom if you want to keep the global.

I'd try to get rid of the floating point, too... just calculate what your thresholds are in the raw echo values, and use those. Do we really care what the internal units of measurement are?

Sorry for the long unload... There's nothing wrong with what you have.