1

I have a class with a byte (stripLength) that seems to just disappear in the middle of the frameStep() method. I'm not especially skilled with C++ (or non-VM languages in general), but I believe it's only written or modified at the initialization of the class. What am I doing wrong?

BugFinding.ino

#define PIN 11
#define NUM_LEDS 100
#define WAIT 5
#define TAIL_LENGTH 40

#include "Libraries/Adafruit_NeoPixel.h"
#include "LightingStrip.h"


Adafruit_NeoPixel strip = Adafruit_NeoPixel(NUM_LEDS, PIN, NEO_RGB + NEO_KHZ800);
LightingStrip lightStrip = LightingStrip(strip, NUM_LEDS, 20);

void setup() {
    Serial.begin(9600);
    strip.begin();
    strip.show();
    Serial.println("BugFinding");
    lightStrip.setColors();
}

void loop() {
    Serial.println("loop");
    lightStrip.frameStep();
    lightStrip.write();
    delay(WAIT);
}

LightingStrip.h

#ifndef LIGHTINGSTRIP_H_
#define LIGHTINGSTRIP_H_

#include "Libraries/Adafruit_NeoPixel.h"

class LightingStrip {
    public:
        LightingStrip(Adafruit_NeoPixel ledStrip, uint8_t stripLength, uint8_t lengthOfTail);
        void frameStep();
        void write();
        void setColors();
    protected:
        Adafruit_NeoPixel strip;
        uint8_t mode;
        uint8_t frame;
        uint8_t colorFrame;
        uint8_t stripLength;
        uint8_t tailLength;
        uint32_t buf[];
        uint32_t colors[7];
        uint8_t currentColor[3];

        void cycleStep();
};

#endif /* LIGHTINGSTRIP_H_ */

LightingStrip.cpp

#include "LightingStrip.h"
#include "Libraries/Adafruit_NeoPixel.h"

LightingStrip::LightingStrip(Adafruit_NeoPixel ledStrip, uint8_t length, uint8_t lengthOfTail) {
    strip = ledStrip;
    stripLength = length;
    tailLength = lengthOfTail;

    buf[stripLength] = {0};
    colorFrame = 0;

    cycleStep();
}

void LightingStrip::setColors(){
    //colors = uint8_t[8][3];
    colors[0] = strip.Color(255, 120,   0); // Orange
    Serial.println("Color 0: " + String(colors[0]));
    Serial.println("Color 0: " + String(strip.Color(255, 120,   0)));
    Serial.println("stripLength: " + String(this->stripLength));
    colors[1] = strip.Color(  0, 255,   0); // Green
    Serial.println("setColors");
    Serial.println("stripLength: " + String(stripLength));
}

void LightingStrip::cycleStep(){
    frame = 0;
    colorFrame++;
}

void LightingStrip::frameStep(){
    Serial.println("frameStep");
    Serial.println("Color 0: " + String(colors[0]));
    int16_t j;
    Serial.println("frame: " + String(frame));
    Serial.println("colorFrame: " + String(colorFrame));
    Serial.println("stripLength 1: " + String(stripLength));
    if(frame > stripLength + tailLength)
        cycleStep();
    Serial.println("stripLength 1.2: " + String(stripLength));

    for(j=0; j<stripLength; j++){
        buf[j] = strip.Color(255,255,255);
    }
    Serial.println("stripLength 2: " + String(stripLength));
    for(j=frame; j>0 && frame-j<=stripLength; j--){
        if(j>stripLength) continue;
        buf[j] = colors[colorFrame];
        Serial.print("j: " + String(j));
        Serial.print(" color: " + String(colors[colorFrame]));
        Serial.println();
    }
    Serial.println("stripLength 3: " + String(stripLength));
    Serial.println("stripLength 4: " + String(stripLength + tailLength));
    Serial.println("stripLength 5: " + String(stripLength));

    frame++;
}
void LightingStrip::write(){
    Serial.println("write");
    Serial.println("stripLength: " + String(stripLength));

    int16_t j;

    for(j=0; j<stripLength; j++){
        Serial.print(" j: " + String(j));
        strip.setPixelColor(j, buf[j]);
    }
    Serial.println();
    Serial.println(String(buf[0]));
    Serial.println(String(strip.getPixelColor(0)));
    strip.show();
}

Serial Output:

BugFinding
Color 0: 16742400
Color 0: 16742400 
stripLength: 100
setColors
stripLength: 100 
loop
frameStep
Color 0: 16742400
frame: 0
colorFrame: 1
stripLength 1: 100
stripLength 1.2: 100
2
  • 1
    Have you checked for memory error? You are using a lot of Strings and if you are using an Arduino Uno (Nano, etc) there might be some memory issues with only 2K byte available. Start by removing all the "literal string" + String(value) and replacing them with multiple println() lines. – Mikael Patel Nov 12 '16 at 23:30
  • It looks like you're right. I'm running out of memory in general. If you'll make this an answer, I'll mark it as correct. – Chris Adams Nov 13 '16 at 0:42
0

At first glance this looks like a sketch with typical memory (heap) issues. When using a lot of string literals, String variables and expressions heap memory becomes fragmented or simple exhausted.

Some programming rules to avoid this:

  1. If possible use the macro F() with string literals. Typically F() should be used when printing a string.

  2. If possible do not use String especially when there is limited amount of SRAM. Remember that ATmega328 based boards such as Arduino Uno, Nano, etc, have only 2 Kbyte SRAM.

  3. Avoid String expressions especially in Serial print statements. Use several print statements instead.

Applying these programming rules to some of the statements in the code above:

Serial.println("frameStep");
Serial.println("Color 0: " + String(colors[0]));
Serial.println("frame: " + String(frame));

Becomes:

Serial.println(F("frameStep"));
Serial.print(F("Color 0: "));
Serial.println(colors[0]);
Serial.print(F("frame: "));
Serial.println(frame);

With no String expressions, and a lot less memory used.

Cheers!

PS: As @MarkSmith points out there is also a pure allocation error. The statement below in the LightingStrip constructor:

buf[stripLength] = {0};

Does not mean allocate and assign. It means element stripLength is assigned the value of the pointer to an array with the single value zero. Guess that the compiler is also warning about this. The value of buf is random as it is not initiated. Assigning an element in the buf will write to a random position.

0

I think what you mean by stripLength "disappearing" is that your stripLength 2 trace does not appear. This is going to be because you never reach this point in the code.

Look at buf: it's defined as an array in the .h file, but you don't ever allocate any memory for it. buf[stripLength] = {0} doesn't do what you think it does. It puts a 0 into element 100, which at that point is going to trash over some memory you don't mean to write to. When you write to every element in the loop, you're going to trash over lots of memory you didn't mean to. You want something like

buf = (uint32_t*) malloc(stripLength*sizeof(uint32_t));

Remember to free it again in the destructor.

(Full disclosure: I'm writing this on my phone and I can't remember or test whether it's legal syntax to do this with your array defined (in the .h) as uint32_t buf[]. It's more common, and certainly legal, to use uint32_t* buf.)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.