Need help with my first non linear program

Using Arduino Programming Questions
#1

Hi this is my first attempt at doing non-linear programming.
Ive done more complex stuff but it always gets bogged down due to bad code techniques so I decided to start fresh.

This code does a simple cylon pattern on a Neopixel programmable RGB strip.
Since we cant do instances of a class I wanted to find a way around that and this is how I solved the problem.

The problem is that for every “instance” I need to basically create a whole copy of the function, rename it to something else (cyon1,cylon2, etc), an then also duplicate all of my variables (time1, time 2, etc)
Right now I only am using two instances, but if I tried say 20, then I would have 20 times the functions and 20 times the variables

I also noticed that I cant define the variables inside the functions so in a large complex sketch, the variable list get to be very confusing.

Is there a better way to do this?

Is it possible to define the variables in the functions instead of at the top of the sketch. The reason for this is that in my arduino sketch I opened up many new tabs, each tab is one sequence such as a tab for cylon, another for cylon2, another for flicker3, etc.

It would be easy to understand it if the variables specific to each tab could be in that tab itself (int flag1 in the cylon1 tab, int flage2 in the cylon2 tab, etc)

Any feedback would be really helpful.

neopixel_generic_patterns (main tab)

#include <Adafruit_NeoPixel.h>
unsigned long time1; //timer variable
unsigned long time2; //timer variable
int flag1=1;
int flag2=1;
int r;
int g;
int b;
int currentLED1;
int currentLED2;
uint32_t stripValues[40];//create array to store all led position values
#define PIN 2
Adafruit_NeoPixel strip = Adafruit_NeoPixel(40, PIN, NEO_GRB + NEO_KHZ800);

void setup() {
  strip.begin();
  for (int i=0; i<strip.numPixels(); i++){//set all leds to off and load array of off values
    stripValues[i]=0;
    strip.setPixelColor(i, stripValues[i]); 
  }
  strip.show(); // Initialize all pixels to 'off'
}

void loop() {
  cylon1(strip.Color(0, 255,0),10,21,40,3);//color, speed(millis),firstLED,LastLED,number of LEDs in chase
  cylon2(strip.Color(0, 0, 255),30,0,20,3);//color, speed(millis),firstLED,LastLED,number of LEDs in chase
  showLEDS();//display the loaded array on LED strip
}

void showLEDS(){//display the loaded array on LED strip
  for (int i=0; i<strip.numPixels(); i++){
    strip.setPixelColor(i, stripValues[i]); 
  }
  strip.show();
}

Cylon1 TAB

//Cylon lights (back and forth)

void cylon1(uint32_t c, int wait, int firstLED, int lastLED,int numLEDS) { //color, speed(millis),firstLED,LastLED,number of LEDs in chase
  if (millis()-time1>wait){/do something only if delay parameter is met
    if (flag1==1) {// if at start of sequence determine first led position
      currentLED1=firstLED;
      flag1=2;
    }

    if (flag1==2){//slide leds down strip
      for (int i=firstLED; i <= currentLED1; i++) {
        stripValues[i]=0;    //turn start of chain off
      }
      for (int i=currentLED1; i < currentLED1+numLEDS; i++) {
        stripValues[i]= c;    //turn every third pixel on
      }
      for (int i=currentLED1+numLEDS; i < lastLED; i++) {
       stripValues[i]=0;    //turn every third pixel on
      }

      strip.show(); 
      currentLED1+=1;
      if (currentLED1+numLEDS>=lastLED || currentLED1>=strip.numPixels()){// if at end of strip or working space then move to next section
        flag1=3;
      }
    }
    if (flag1==3){/move LEDS backwards towards start
      for (int i=firstLED; i <= currentLED1; i++) {
        stripValues[i]=0;    //turn start of chain off
      }
      for (int i=currentLED1; i < currentLED1+numLEDS; i++) {
        stripValues[i]=c;    //turn every third pixel on
      }
      for (int i=currentLED1+numLEDS; i < lastLED; i++) {
        stripValues[i]=0;    //turn every third pixel on
      }

      strip.show(); 
      currentLED1-=1;
      if (currentLED1<=firstLED || currentLED1<=0){//if at start of strip or strip work area then restart requence
        flag1=1;
      }
    }
    time1=millis();
  }
}

Cylon2 TAB

//Cylon lights (back and forth)

void cylon2(uint32_t c, int wait, int firstLED, int lastLED,int numLEDS) { //color, speed(millis),firstLED,LastLED,number of LEDs in chase
  if (millis()-time2>wait){ //do something only if delay parameter is met
    if (flag2==1) {// if at start of sequence determine first led position
      currentLED2=firstLED;
      flag2=2;
    }


    if (flag2==2){//slide leds down strip

      for (int i=firstLED; i <= currentLED2; i++) {
        stripValues[i]=0;    //turn start of chain off
      }
      for (int i=currentLED2; i < currentLED2+numLEDS; i++) {
        stripValues[i]=c;    //turn every third pixel on
      }
      for (int i=currentLED2+numLEDS; i < lastLED; i++) {
        stripValues[i]=0;    //turn every third pixel on
      }

      strip.show(); 
      currentLED2+=1;
      if (currentLED2+numLEDS>=lastLED || currentLED2>=strip.numPixels()){// if at end of strip or working space then move to next section
        flag2=3;
      }
    }
    if (flag2==3){//move LEDS backwards towards start

      for (int i=firstLED; i <= currentLED2; i++) {
        stripValues[i]=0;    //turn start of chain off
      }
      for (int i=currentLED2; i < currentLED2+numLEDS; i++) {
        stripValues[i]=c;    //turn every third pixel on
      }
      for (int i=currentLED2+numLEDS; i < lastLED; i++) {
        stripValues[i]=0;    //turn every third pixel on
      }

      strip.show(); 
      currentLED2-=1;
      if (currentLED2<=firstLED || currentLED2<=0){//if at start of strip or strip work area then restart requence
        flag2=1;
      }
    }
    time2=millis();//reset timer
  }
}
#2

Is it possible to define the variables in the functions instead of at the top of the sketch.

Of course. They will be local to the function they are defined in, which may not be what you want. They will also be reused/loose their values between function calls, unless they are static.

Since we cant do instances of a class

Why not? And, how would they help?

#3

I though ideally I would take one function (cylon) and feed it values, then create a new instance of it an feed it seperate values, so that in an example of 20 cylon sequences on a strip doing each different speeds, colors, etc
I would only need one function instead of 20 and I would need only one set of variables. I am reusing a bunch such as int wait which is called into the function but other variables such as time1, flag1, etc need to be recreated with a new name (time2, flag2, etc) in order for my sketch to work...

Just feels like there has to be a more efficient way to do this...

#4

There is, you just need to structure your code correctly.

I recommend creating a new tab called “cylon_pattern.h” with the following code:

#ifndef CYLON_PATTERN_H
#define CYLON_PATTERN_H

typedef struct
{
  unsigned long wait_delay;
  unsigned long pattern_start_time;
  unsigned char flag;
  int current_LED;
} cylon_pattern;

#endif

Then, if your main sketch, use the following for your cylon function:

#include "cylon_pattern.h"

...
...

cylon_pattern c1;
cylon_pattern c2;

setup()
{
  // initialize the struct member variables.
}

loop
{
  cylon(strip.Color(0, 255,0),21,40,3, &c1);//color,firstLED,LastLED,number of LEDs in chase, pointer to cylon struct
  cylon(strip.Color(0, 0, 255),0,20,3, &c2);//color,firstLED,LastLED,number of LEDs in chase, pointer to cylon struct
}

void cylon(uint32_t c, int firstLED, int lastLED,int numLEDS, cylon_pattern* cp) { //color, speed(millis),firstLED,LastLED,number of LEDs in chase
  if (millis()-cp->pattern_start_time > cp->wait_delay){//do something only if delay parameter is met
    if (cp->flag==1) {// if at start of sequence determine first led position
      cp->current_LED=firstLED;
      cp->flag=2;
    }

    if (cp->flag==2){//slide leds down strip
      for (int i=firstLED; i <= currentLED1; i++) {
        stripValues[i]=0;    //turn start of chain off
      }
      for (int i=cp->current_LED; i < cp->current_LED+numLEDS; i++) {
        stripValues[i]= c;    //turn every third pixel on
      }
      for (int i=cp->current_LED+numLEDS; i < lastLED; i++) {
        stripValues[i]=0;    //turn every third pixel on
      }

      strip.show(); 
      cp->current_LED+=1;
      if (cp->current_LED+numLEDS>=lastLED || cp->current_LED>=strip.numPixels()){// if at end of strip or working space then move to next section
        cp->flag=3;
      }
    }
    if (cp->flag==3){//move LEDS backwards towards start
      for (int i=firstLED; i <= cp->current_LED; i++) {
        stripValues[i]=0;    //turn start of chain off
      }
      for (int i=cp->current_LED; i < cp->current_LED+numLEDS; i++) {
        stripValues[i]=c;    //turn every third pixel on
      }
      for (int i=cp->current_LED+numLEDS; i < lastLED; i++) {
        stripValues[i]=0;    //turn every third pixel on
      }

      strip.show(); 
      cp->current_LED-=1;
      if (cp->current_LED<=firstLED || cp->current_LED<=0){//if at start of strip or strip work area then restart requence
        cp->flag=1;
      }
    }
    cp->pattern_start_time=millis();
  }
}

That should give you an idea of how to work it. In fact, since the parameter values are different for each cylon pattern, you could make the struct have all those values (firstLED, lastLED, color, number of LED) and just pass the struct pointer to the function.

There may be some typos, I just did this with copy+paste.

#5

That looks like something that could be great to understand but unfortunately I dont :frowning:
I am new to anything outside of the basic arduino language.

The cp class structure defines 3 variables but when you call them in my cylon function how does it know which variable is being accessed?

Also the setup field says to use that space to initialize my struct variables but when I try to asign say...flag=1; I get an error saying its not defined in the scope.

Another odd thing is that flag was defined as a char and in my code it was an integer... I tried switching it to an integer but still did not get any leds to light up at all.

also was confused by the way cp was being referenced (" cp-") the hyphen is not something I am familiar with...

It looks like a great concept to learn but dont even know where to begin getting info on how that works :frowning:

#6

Start by looking in the ref section at arrays. That will get rid of the need to have a named var for each cylon eg cylon1, cylon2 ect become cylon ( where x is the number of the cylon.). You can get rid of the need for a function for each cylon by passing the cylon number as a parameter.

A struct takes this to the next level. Is all in the ref section.

The language is C/C++ and with a very few exceptions every thing from any of the C/C++ sites can be used.

The main exceptions are I/O (Streams/stdio) and String.

Mark

#7

THANKS FOR THE HELP!!!

Did a little reading up on C++ and Just got it working!
Here is the code. I do have one last question…
My setup() section looks like this:
void setup() {
strip.begin();
…
…
…
c1.flag=1;
c2.flag=1;
c3.flag=2;
}

for every instance I still need to define these variables for my sketch to work.
Is there a way to put these initial values somewhere in the class so that I only have to say to start with a value of 1 once and not have to assign a value to many individual instances???

MAIN SKETCH

#include <Adafruit_NeoPixel.h>
#include "cylon_pattern.h"

cylon_pattern c1;
cylon_pattern c2;
cylon_pattern c3;
uint32_t stripValues[40];//create array to store all led position values
#define PIN 2
#define num_LEDS 40
Adafruit_NeoPixel strip = Adafruit_NeoPixel(num_LEDS, PIN, NEO_GRB + NEO_KHZ800);

void setup() {
  strip.begin();
  for (int i=0; i<strip.numPixels(); i++){//set all leds to off and load array of off values
    stripValues[i]=0;
    strip.setPixelColor(i, stripValues[i]); 
  }
  strip.show(); // Initialize all pixels to 'off'
  c1.flag=1;
    c2.flag=1;
    c3.flag=2;
}

void loop() {

   cylon(strip.Color(0, 255,0),20,10,25,4, &c1);//color,firstLED,delay,LastLED,number of LEDs in chase, pointer to cylon struct
  cylon(strip.Color(0, 0, 255),10,0,40,2, &c2);
  cylon(strip.Color(255, 0, 0),5,0,40,2, &c3);
  showLEDS();//display the loaded array on LED strip
}

void showLEDS(){//display the loaded array on LED strip
  for (int i=0; i<strip.numPixels(); i++){
    strip.setPixelColor(i, stripValues[i]); 
  }
  strip.show();
}

CYLON TAB

//Cylon lights (back and forth)

void cylon(uint32_t c, int wait, int firstLED, int lastLED,int numLEDS, cylon_pattern* cp) { //color, speed(millis),firstLED,LastLED,number of LEDs in chase
  if (millis()-cp->pattern_start_time > wait){//do something only if delay parameter is met
    if (cp->flag==1) {// if at start of sequence determine first led position
      cp->current_LED=firstLED;
      cp->flag=2;
    }

    if (cp->flag==2){//slide leds down strip
      if (cp->current_LED>firstLED  && cp->current_LED>0)stripValues[cp->current_LED-1]= 0;
      for (int i=cp->current_LED; i < cp->current_LED+numLEDS; i++) {
        stripValues[i]= c;    //turn every third pixel on
      }
      strip.show(); 
      cp->current_LED+=1;
      if (cp->current_LED+numLEDS>=lastLED || cp->current_LED>=strip.numPixels()){// if at end of strip or working space then move to next section
        cp->flag=3;
      }
    }
    if (cp->flag==3){//move LEDS backwards towards start

      for (int i=cp->current_LED; i < cp->current_LED+numLEDS; i++) {
        stripValues[i]=c;    //turn every third pixel on
      }
      if (cp->current_LED+numLEDS<lastLED && cp->current_LED+numLEDS<strip.numPixels()-1 )stripValues[cp->current_LED+numLEDS+1]= 0;

      strip.show(); 
      cp->current_LED-=1;
      if (cp->current_LED<=firstLED || cp->current_LED<=0){//if at start of strip or strip work area then restart requence
        cp->flag=1;
      }
    }
    cp->pattern_start_time=millis();
  }
}

CYLON_PATTERN.H TAB

#ifndef CYLON_PATTERN_H
#define CYLON_PATTERN_H

typedef struct
{
 // unsigned long wait_delay;
  unsigned long pattern_start_time;
  int flag;
  int current_LED;
} cylon_pattern;

#endif
#8