Code Efficiencies

system · November 15, 2014, 11:11pm

Hi Guys,

So I'm in the final stages of implementing my project and am finding that with a lot (27 pixels) of NeoPixel lights the code lags a bit, I think it must be chewing up processing or ram. I'm using Arduino Mega 2560s.

Just wondering if you guys can offer any efficiencies in my code that might help.

Basically what happens is I have a control unit sending characters over Xbee and when this unit receives the code a switch case triggers a pixel animation where they cascade on and then off again. There's a status light that lights up when the particular switch case is triggered, this just for troubleshooting.

I had a function called Curve that was doing a nicer fade on the LEDs but I've commented that out for the time being.

Thanks!

Chris

Here's the code for the cascading light function called Chase:

// Main function used to chase the LEDs up and down
void chase(byte valR, byte valG, byte valB)
{ 
  currenttime = millis();
  
    if(currenttime - previoustime > interval) {// tells the function the interval at which to increment the LED brightness
      
      for(byte i=0; i<QTY; i++) {// incremments the LED so that you can light up multiple
        
        if (timeArray[0] < WAITINT) { // fades up the LED
          fadeR = constrain(map(timeArray[i], 0, 255, 0, valR),0,FADESTEP);
          fadeG = constrain(map(timeArray[i], 0, 255, 0, valG),0,FADESTEP);
          fadeB = constrain(map(timeArray[i], 0, 255, 0, valB),0,FADESTEP);
        }
        else { // fades down the LED when its reached full
          fadeR = constrain(map(timeArray[i]-WAITINT, 0, 255, valR, 0),0,FADESTEP);
          fadeG = constrain(map(timeArray[i]-WAITINT, 0, 255, valG, 0),0,FADESTEP);
          fadeB = constrain(map(timeArray[i]-WAITINT, 0, 255, valB, 0),0,FADESTEP);
        }          
        
        // sends the actual level to the LED
        // pixel.setPixelColor(i,curve(fadeR),curve(fadeG),curve(fadeB));
        pixel.setPixelColor(i,fadeR,fadeG,fadeB);
        pixel.show();
        
        // increases the time
        timeArray[i] = timeArray[i] + ANIMSPEED;        
                  
        if (timeArray[QTY-1] >= (255 + WAITINT)) {  // code to end the switch case when the last LED reaches its final time value
           dmx = 0;
        }
      }
    previoustime = currenttime;
  }
}

What I have happening here is the code makes an array for each pixels R, G, B value and offsets the beginning value below zero by the amount that I want the wait between each pixel to be.... I wonder if all this data is making the Arduino chug.

system · November 15, 2014, 11:12pm

Here's the full code:

// ------------------------------ LIBRARIES ------------------------------ //

#include <Wire.h>
#include <Adafruit_NeoPixel.h>
#include <LEDFader.h>

// ------------------------------ DEFINITIONS ------------------------------ //

//Status LEDs
#define LEDSTART 44
#define LEDQTY 8

// NeoPixels
#define PIN 22 // data pin for NeoPixels
#define QTY 27 // quantity of NeoPixels (DS SET)

#define ANIMSPEED 4 // overall speed of the animation
#define FADEINTERVAL 10 // the speed of the fade up of the LED
#define DELAYINT 0 // interval between each LED
#define WAITINT 255 // the wait time between the first led getting to full then starting to fade out, with 255 it starts straight away
#define FADESTEP 255 // the number of steps in the fade (generally 100 or 255)

Adafruit_NeoPixel pixel = Adafruit_NeoPixel(QTY, PIN, NEO_RGB + NEO_KHZ800);

// ------------------------------ DECLARATIONS ------------------------------ //

byte statusLED[LEDQTY];

// Xbee
char dmx;
char xbeeraw;

// State Counter declarations
char colorString[] = {'R', '2', '3', '4', '5', '6', '7', '8', '9', 'Q', 'S', 'X', 'T'};

// declarations for the Chase function
int interval = FADEINTERVAL;
byte delayint = DELAYINT;
unsigned long currenttime;
unsigned long previoustime;
int timeArray[QTY];

//final value of bright LED
uint8_t valR;
uint8_t valG;
uint8_t valB;

uint8_t fadeR;
uint8_t fadeG;
uint8_t fadeB;

// value used in the map functions

// The number of Steps between the output being on and off
const int pwmIntervals = FADESTEP;
// The R value in the graph equation
float R;

// ------------------------------ SETUP ------------------------------ //

void setup()
{
  //Serial transmission
  Serial.begin(115200);
  
  // begin NeoPixel code
  pixel.begin();
  pixel.show();

  // calculation for the LOG curve
  R = (pwmIntervals * log10(2))/(log10(255));
  
  // fill array for LED delay stuff
  for (int c=0; c<(QTY+1); c++){
    timeArray[c] = (c*(-DELAYINT));
  }
  
  // status LEDs
  for (byte i = 5; i < 13; i++) {
    pinMode(i,OUTPUT);
  }
  
  for (byte i = LEDSTART; i < (LEDSTART + LEDQTY); i++) {
    pinMode(i,OUTPUT);
    statusLED[i - LEDSTART] = i;
  }
  
  // Debugging Lights
  pinMode(12,OUTPUT); //Power
  pinMode(13,OUTPUT); //Serial TX/RX
  
  //12v LED PWM (MOSFETs)
  pinMode(5,OUTPUT);
  pinMode(6,OUTPUT);
}

// ------------------------------ LOOP ------------------------------ //

void loop()
{  
  
  // Power Light
  digitalWrite(12,HIGH);
  
  // NeoPixel output
  pixel.show();
  
  // Serial read stuff
  if (Serial.available() > 0) {
    xbeeraw = Serial.read();
    //digitalWrite(13,HIGH); // Serial RX/TX On
  }
  else {
    xbeeraw = 'N';
    //digitalWrite(13,LOW);// Serial RX/TX Off
  }
  
  if (Serial.available() > 0) {
    digitalWrite(13,HIGH);
  }
  else {
    digitalWrite(13,LOW);
  }
  
  // Switch Case
  switch (xbeeraw)
  {
    case 'R':        // button 1 (RED)
      chase(255,0,0);
      status(0);
      break;
    case '2':        // button 2 (ORANGE)
      chase(230,192,0);
      status(1);
      break;
    case '3':        // button 3 (YELLOW)
      chase(230,217,0);
      status(2);
      break;
    case '4':        // button 4 (GREEN1) DS SET
      chase(0,255,0);
      status(3);
      break;
    case 'Q':        // button 4 (GREEN2) MS SET
      chase(0,255,0);
      status(3);
      break;
    case 'S':        // button 4 (GREEN3) US SET
      chase(0,255,0);
      status(3);
      break;
    case '5':       // button 5 (BLUE)
      chase(0,0,255);
      status(4);
      break;
    case '6':       // button 6 (INDIGO)
      chase(217,0,255);
      status(5);
      break;
    case '7':       // button 7 (VIOLET)
      chase(192,0,255);
      status(6);
      break;
    case '8':      // button 8 (WHITE) 
      chase(255,255,255);
      status(7);
      break;
    case 'X':      // Grug House
      analogWrite(5,255);
      break;
    case 'T':      // Snoot House
      analogWrite(6,255); 
      break;
    default:       // default (OFF)
      analogWrite(5,0);
      analogWrite(6,0);
      for(int i=0; i<pixel.numPixels(); i++) 
      {
        // set all pixels to off
        pixel.setPixelColor(i,0,0,0);
        pixel.show();
        // reset time array as per the setup
        for (int c=0; c<(QTY+1); c++){
          timeArray[c] = (c*(-DELAYINT));
        }
      } 
      // set status LEDs to off
      for (byte i = LEDSTART; i < (LEDSTART + LEDQTY); i++) {
        digitalWrite(i,LOW);
      }
      break;
  }
}

// ------------------------------ FUNCTIONS ------------------------------ //


// Main function used to chase the LEDs up and down
void chase(byte valR, byte valG, byte valB)
{ 
  currenttime = millis();
  
    if(currenttime - previoustime > interval) {// tells the function the interval at which to increment the LED brightness
      
      for(byte i=0; i<QTY; i++) {// incremments the LED so that you can light up multiple
        
        if (timeArray[0] < WAITINT) { // fades up the LED
          fadeR = constrain(map(timeArray[i], 0, 255, 0, valR),0,FADESTEP);
          fadeG = constrain(map(timeArray[i], 0, 255, 0, valG),0,FADESTEP);
          fadeB = constrain(map(timeArray[i], 0, 255, 0, valB),0,FADESTEP);
        }
        else { // fades down the LED when its reached full
          fadeR = constrain(map(timeArray[i]-WAITINT, 0, 255, valR, 0),0,FADESTEP);
          fadeG = constrain(map(timeArray[i]-WAITINT, 0, 255, valG, 0),0,FADESTEP);
          fadeB = constrain(map(timeArray[i]-WAITINT, 0, 255, valB, 0),0,FADESTEP);
        }          
        
        // sends the actual level to the LED
        // pixel.setPixelColor(i,curve(fadeR),curve(fadeG),curve(fadeB));
        pixel.setPixelColor(i,fadeR,fadeG,fadeB);
        pixel.show();
        
        // increases the time
        timeArray[i] = timeArray[i] + ANIMSPEED;        
                  
        if (timeArray[QTY-1] >= (255 + WAITINT)) {  // code to end the switch case when the last LED reaches its final time value
           dmx = 0;
        }
      }
    previoustime = currenttime;
  }
}

// LED Logarithmic Curve so the dimming looks nicer (value out of 0 - 255)
byte curve(uint8_t increment)
{
  if (increment <= 0 || increment > FADESTEP) {
    return 0;
    }
  else {
    return pow (2, (increment / R)) - 1;
    }
}

void status(byte led) {
  digitalWrite(statusLED[led],HIGH);
  for (byte i = LEDSTART; i < (LEDSTART + LEDQTY); i++) {
    if(i != statusLED[led]) {
      digitalWrite(i,LOW);
    }
  }
}

system · November 16, 2014, 1:33am

          fadeR = constrain(map(timeArray[i], 0, 255, 0, valR),0,FADESTEP);
          fadeG = constrain(map(timeArray[i], 0, 255, 0, valG),0,FADESTEP);
          fadeB = constrain(map(timeArray[i], 0, 255, 0, valB),0,FADESTEP);

The map function is not the most efficient function. Calling it three times, as opposed to saving the value is a waste of effort.

The constrain function is more efficient, but calling it three times is wasteful.

    timeArray[c] = (c*(-DELAYINT));

(The) (proper) (number) (of) (parentheses) (in) (this) (statement) (is) (zero

  // Serial read stuff
  if (Serial.available() > 0) {
    xbeeraw = Serial.read();
    //digitalWrite(13,HIGH); // Serial RX/TX On
  }
  else {
    xbeeraw = 'N';
    //digitalWrite(13,LOW);// Serial RX/TX Off
  }

  if (Serial.available() > 0) {
    digitalWrite(13,HIGH);
  }
  else {
    digitalWrite(13,LOW);
  }

Why are there TWO if/else blocks based on the SAME conditional? Apparently, efficiency is not an issue.

system · November 16, 2014, 4:23am

Hi Paul S,

Thanks for the suggestions.

I'm not sure what you mean by this:

The map function is not the most efficient function. Calling it three times, as opposed to saving the value is a waste of effort.

The constrain function is more efficient, but calling it three times is wasteful.

nickgammon · November 16, 2014, 5:26am

The constrain function is more efficient, but calling it three times is wasteful.

Can you suggest an alternative? The three calls have different arguments.

system · November 16, 2014, 6:13am

is using Min and Max more efficient than using constrain? It seems like it would be the same..

system · November 16, 2014, 7:05am

I'm thinking of trying a version where I just make an array of 100 values each for RGB, is there a way to do something only once in a function, like:

call the chase (R, G, B) value

create array of R/100*index to make a 100 index array

then just reference the spot of the array

I think the efficiency would be if I could do this only once at the beginning of the function.. I cant do it in void setup as the RGB values change in the switchcase

thanks!

robtillaart · November 16, 2014, 12:42pm

fadeR = constrain(map(timeArray[i], 0, 255, 0, valR),0,FADESTEP);

first thought is that one could merge the constrain and the mapping

assuming timeArray has only values between 0..255 (?) this would be not to hard.
FADESTEP = 255

fadeR = min(timeArray[i] * valR / 255, 255);

or a faster version (with a minimal error, probably not visible)

fadeR = min(timeArray[i] * valR / 256, 255); // division by 256 can be optimized.

tmd3 · November 16, 2014, 7:57pm

Disclaimer: I've never used neopixels, so anything I suggest here is could easily be wrong.

Here's my recommendation:

Don't execute pixel.show() after each pixel update. Instead, execute it after all the pixels are updated. Move the call to pixel.show() outside the loop in function chase(). That will improve performance quite a lot, but, by my calculations, it won't get the execution time for chase() below 10 milliseconds.

Here's the tl;dr:

When I run this code with the adafruit neopixel library installed, I find that function chase() takes about 30 to 35 milliseconds to execute. It looks like the sketch is intended to update the pixels every 10 milliseconds, so it appears to take way too long.

A look into the adafruit code for setPixelColor() reveals that setPixelColor() stores color data for a single pixel in a RAM buffer. That's a short, fast operation. show() appears to output the contents of the entire buffer to the neopixel array. That's a communication task, so it has some timing constraints, and it affects every pixel. It's quite a bit longer and slower than pixelSetColor().

Looking at the adafruit tutorial on the neopixel library, here - Arduino Library Use | Adafruit NeoPixel Überguide | Adafruit Learning System - I find language that suggests that pixel.show() doesn't have to be called after every pixel update. In fact, it appears to be preferable to call it after all the pixels have been updated:

strip.show();
This updates the whole strip at once, and despite the extra step is actually a good thing. If every call to setPixelColor() had an immediate effect, animation would appear jumpy rather than buttery smooth.

I presume you'd prefer buttery smooth.

My timing tests on this code show that pixel.show() takes about 840 microseconds. That matches fairly well with the calculated minimum time to send 3 8-bit bytes for each of 27 pixels at 800 MHz, 810 microseconds. If pixel.show() is called only once in chase(), instead of 27 times, it should reduce the execution time for chase() by about 22 milliseconds, and it would take about [Edit: correct this] ~~2/3~~ 1/3 as long as it does now.

That leaves chase() executing for about 11 milliseconds, by my tests - still too slow for a 10 millisecond update time, but a whole lot faster than the original. It looks like beating 10 milliseconds will require other improvements.

Finally, remember that I've never used a neopixel, and I don't have any to test with. My cursory look through the adafruit neopixel library didn't find any communication coming from the neopixels, though, so I think that the fact that I have no neopixels connected won't affect execution timing. Still, I could be wrong about any particular, and maybe about every particular.

system · November 19, 2014, 11:43pm

Hi tmd3,

I tried removing pixel.show() and it made a huuuuge difference! Thank you so much!

It's now triggering in the mail loop() function.

I'm even trying a version where it triggers in the switch case so that it's even more localised. I'll try this today and see how it goes.

Thanks again!

Topic		Replies	Views
Maybe Faster Code Needed Programming	26	532	January 17, 2025
Arduino Sketch Gets Hung up in Function Call Programming	26	363	March 26, 2026
Neopixel strip flicker/glitch issue General Guidance	18	501	January 23, 2025
Delay help with neopixel strip General Guidance	6	221	November 17, 2025
Code processing at slow speed General Guidance	28	567	August 13, 2024

Code Efficiencies

Related topics