global definition of CRGB matrix for different classes

I am currently programming my RGB LEDCube and therefor I created the LEDCube class with many different functions that create together animations. To control my WS2811 LEDs do I use the FastLED library which does store the RGB Data of all LEDs (in my case, 343) in a CRGB Matrix[343]. To change the color of the LEDs I have to access this matrix. I have declared it inside my main class "LEDCube" as public member. Until now it was only neccessary that this matrix could be accessed of members of the "LEDCube" class. But now am I making an animation that is executed in instances of its own class but in this class the CRGB Matrix needs to be accessed. I somehow need to make this matrix global. I tried moving it just outside of the "LEDCube" class but this throws the errors of multiple definitions of this matrix:

libraries\LEDCube\animations.cpp.o (symbol from plugin): In function `LEDCube::test(int)':

(.text+0x0): multiple definition of `leds'

sketch\LED_Cube.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here

c:/program files (x86)/arduino/hardware/tools/avr/bin/../lib/gcc/avr/4.9.2/../../../../avr/bin/ld.exe: Disabling relaxation: it will not work with multiple definitions

libraries\LEDCube\anitilities.cpp.o (symbol from plugin): In function `LEDCube::clearLEDs()':

(.text+0x0): multiple definition of `leds'

sketch\LED_Cube.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here

libraries\LEDCube\fading.cpp.o (symbol from plugin): In function `LEDCube::fade(unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, unsigned char, int, unsigned char, int)':

(.text+0x0): multiple definition of `leds'

sketch\LED_Cube.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here

libraries\LEDCube\text.cpp.o (symbol from plugin): In function `LEDCube::loadLetter(char, xy8bit*)':

(.text+0x0): multiple definition of `leds'

sketch\LED_Cube.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here

libraries\LEDCube\utils.cpp.o (symbol from plugin): In function `LEDCube::xyz2lednum(unsigned char, unsigned char, unsigned char)':

(.text+0x0): multiple definition of `leds'

sketch\LED_Cube.ino.cpp.o (symbol from plugin):(.text+0x0): first defined here

Code:

/*
LEDCube.h
*/
#ifndef LEDCube_H
#define LEDCube_H
#include "Arduino.h"
#include <FastLED.h>
#include <Adafruit_NeoPixel.h>
#include "colorgnt.h"
#include "rainbowgnt.h"
#define NUMLEDS 343

typedef struct xy8bit {  //this is declared like the matrix but this does work globally
	short x;
	short y;
};
CRGB leds[343];  //<--- Matrix
class LEDCube
{
  protected:
  static const int  limit = 7;
  static const int  limitsqu = limit * limit;
  static const int  numLeds = limitsqu * limit;
  public:
  LEDCube() {
  FastLED.addLeds<WS2811, 7>(leds, 343);
  FastLED.setBrightness(255);
  FastLED.clear();
};
  //OTHER STUFF
  //CRGB leds[343]  works 
};
#endif

I only posted "LEDCube.h" because I do not think that other classes or files are relevant but if you think they are, I will post a shorted version of the whole code.

Remember the other day when I told you to google "code smell". Did you do that? You're about to have another.

Don't give any other class direct access to that array. It would be fine for now but later when this program grows you'll end up cussing yourself. If any other class needs numbers out of that array then the LEDCube class needs a member function that goes and gets it for them and returns a copy. Let the other classes call getter and/or setter functions but don't give them direct access to the data in the class. Make the array itself private to LEDCube.

Delta_G:
Remember the other day when I told you to google "code smell". Did you do that? You're about to have another.

Don't give any other class direct access to that array. It would be fine for now but later when this program grows you'll end up cussing yourself. If any other class needs numbers out of that array then the LEDCube class needs a member function that goes and gets it for them and returns a copy. Let the other classes call getter and/or setter functions but don't give them direct access to the data in the class. Make the array itself private to LEDCube.

Thank you.

I need to change them in other classes so I would need a setter function. I can write a setter function but how do I call/execute this function in an other class?

Actually I just looked at your header again and it appears that array isn't part of the class. Move the definition of it to a .cpp and put an extern declaration in any other file that needs it.

It was part of the class until I tried to make it global. initalizing the array like in the code posted above throws the errors. Putting it inside the LEDCube class causes no problems.
Putting "CRGB leds[343];" inside a .cpp throws the error:

C:\Program Files (x86)\Arduino\libraries\LEDCube/LEDCube.h:26:30: error: 'leds' was not declared in this scope

   FastLED.addLeds<WS2811, 7>(leds, 343);

                              ^

I need to access/set values in this matrix inside another class. Therefor I made the matrix private as you told me to. I also created a setter function called "setledmatrix(int pos,nB uint8_t r, uint8_t g, uint8_t b)" as a public member of the LEDCube class. This function can access the 'leds' matrix but how can I call/execute them in another class? I can not just write setledmatrix() can I?

new LEDCube.h:

/*
LEDCube.h
*/
#ifndef LEDCube_H
#define LEDCube_H
#include "Arduino.h"
#include <FastLED.h>
#include <Adafruit_NeoPixel.h>
#include "colorgnt.h"
#include "rainbowgnt.h"
#define NUMLEDS 343

typedef struct xy8bit {
	short x;
	short y;
};

class LEDCube
{
  protected:
  static const int  limit = 7;
  static const int  limitsqu = limit * limit;
  static const int  numLeds = limitsqu * limit;
  public:
  LEDCube() {
  FastLED.addLeds<WS2811, 7>(leds, 343);
  FastLED.setBrightness(255);
  FastLED.clear();
};
  //OTHER STUFF
  setledmatrix(int pos, uint8_t r, uint8_t g, uint8_t b) {
	leds[pos].r = r;
	leds[pos].r = g;
	leds[pos].r = b;
  }
  private:
  CRGB leds[numLeds]; //numLeds = 343
};
#endif

Sorry if I am acting dumb

Yeah, that line with the error is because you have the implementation of your constructor in your header. That belongs in the .cpp as well.

Delta_G:
Yeah, that line with the error is because you have the implementation of your constructor in your header. That belongs in the .cpp as well.

Then it throws the error:

C:\Program Files (x86)\Arduino\libraries\LEDCube\anitilities.cpp:5:1: error: 'FastLED' does not name a type

 FastLED.addLeds<WS2811, 7>(leds, 343);

 ^

C:\Program Files (x86)\Arduino\libraries\LEDCube\anitilities.cpp:6:1: error: 'FastLED' does not name a type

 FastLED.setBrightness(255);

 ^

C:\Program Files (x86)\Arduino\libraries\LEDCube\anitilities.cpp:7:1: error: 'FastLED' does not name a type

 FastLED.clear();

 ^

exit status 1

.cpp:

#include "Arduino.h"
#include "LEDCube.h"
#include "FastLED.h"
CRGB leds[343];
FastLED.addLeds<WS2811, 7>(leds, 343);
FastLED.setBrightness(255);
FastLED.clear();
//OTHER STUFF

LEDCube.h:

/*
LEDCube.h
*/
#ifndef LEDCube_H
#define LEDCube_H
#include "Arduino.h"
#include <FastLED.h>
#include <Adafruit_NeoPixel.h>
#include "colorgnt.h"
#include "rainbowgnt.h"
#define NUMLEDS 343

typedef struct xy8bit {
	short x;
	short y;
};

class LEDCube
{
  protected:
  static const int  limit = 7;
  static const int  limitsqu = limit * limit;
  static const int  numLeds = limitsqu * limit;
  public:
  LEDCube() {};
  //OTHER STUFF
};
#endif

Isnt it possible to somehow call/execute setledmatrix in members of other classes? This would work as well and it would make the code smell good. But how can I call the setledmatrix function(Member of LEDCube class) in member functions of other classes?

No, you don't just stick the lines out of the constructor wherever in the .cpp. Please go look at a few examples of classes and how they are written.

In the .h this line:

LEDCube() {};

needs to lose the {} as that defines it as an empty function. We're declaring the function here but not defining it until the .cpp

In the .cpp define the constructor function:

LEDCube::LEDCube(){
   FastLED.addLeds<WS2811, 7>(leds, 343);
   FastLED.setBrightness(255);
   FastLED.clear();
}

Delta_G:
No, you don't just stick the lines out of the constructor wherever in the .cpp. Please go look at a few examples of classes and how they are written.

In the .h this line:

LEDCube() {};

needs to lose the {} as that defines it as an empty function. We're declaring the function here but not defining it until the .cpp

In the .cpp define the constructor function:

LEDCube::LEDCube(){

FastLED.addLeds<WS2811, 7>(leds, 343);
  FastLED.setBrightness(255);
  FastLED.clear();
}

By changing the line in LEDCube.h and writing the .cpp like:

#include "Arduino.h"
#include "LEDCube.h"
#include "FastLED.h"

CRGB leds[343];
LEDCube::LEDCube(){
  FastLED.addLeds<WS2811, 7>(leds, 343);
  FastLED.setBrightness(255);
  FastLED.clear();
}
//OTHER STUFF

it throws the error

C:\Program Files (x86)\Arduino\libraries\LEDCube\utils.cpp: In member function 'void LEDCube::setLED(int, uint8_t, uint8_t, uint8_t)':

C:\Program Files (x86)\Arduino\libraries\LEDCube\utils.cpp:16:5: error: 'leds' was not declared in this scope

     leds[lednumber].r = r;

     ^

I generally know how classes are written but I do not know how to access the leds matrix in different classes.

hallolo:
Isnt it possible to somehow call/execute setledmatrix in members of other classes? This would work as well and it would make the code smell good. But how can I call the setledmatrix function(Member of LEDCube class) in member functions of other classes?

Is this possible?

OK so now you have an issue in utils.cpp Why don't you show that. I think you just need to go back and reread...

Delta_G:
Move the definition of it to a .cpp and put an extern declaration in any other file that needs it.

hallolo:
I generally know how classes are written but I do not know how to access the leds matrix in different classes.

Is this possible?

¿why so many classes?

you are all over the place with your code, sort of like whacking a stick around in the dark.

this:

#include <FastLED.h>
#include <Adafruit_NeoPixel.h>

to me speaks volumes...

Start off with the question: "How best to structure my code in such a way that I may..." and put together a list of what you want to accomplish.

I keep recommending that the OP spend some time with this topic. I had the same problem when I was green with OOP. I had all these grandiose ideas about how to make all my classes perfectly maintainable and ended up making them perfectly uncompilable. Spend some time learning about the smells in OOP and what you'll really pick up is a lot of good idea on how to organize OOP code.

For example, all these methods in other classes that need access to data from LEDCube should probably be in LEDCube itself. It's hard to say though how to best refactor this without seeing all the code in one place and a good explanation and walk through of how it is all supposed to work.

Delta_G:
OK so now you have an issue in utils.cpp Why don't you show that. I think you just need to go back and reread...

What exactly do you mean with external declaration?

BulldogLowell:
¿why so many classes?

you are all over the place with your code, sort of like whacking a stick around in the dark.

this:

#include <FastLED.h>

#include <Adafruit_NeoPixel.h>



to me speaks volumes...

Start off with the question: "How best to structure my code in such a way that I may..." and put together a list of what you want to accomplish.

Until now I have only the LEDCube class and want to add a needed class for an animation. The class for an animation needs like the LEDCube class somehow access a CRGB 'leds' matrix. Until now I declared this matrix in the LEDCube class and I asked how another class can access this matrix or how another class can access a public setter function in LEDCube.
I already wrote several animations and member functions in this files and I also know how to write classes I just do not understand how to access an public member of class LEDCube in another class.

I will post what I want to accomplish with my programm and the whole code in a few minutes.

hallolo:
What exactly do you mean with external declaration?

I did not say external. I said extern. Why don't you look that up?

It sounds like you need to think about who owns what in your code. These animation classes shouldn't be mucking around with things that belong by all rights to LEDCube. Either have LEDCube have some functions that they can call, which will involve being able to pass a pointer to your instance of LEDCube to the animation classes so they can use that pointer to call methods from said instance. OR LEDCube could be a singleton or static class if there really is only one, but that gets ugly. OR (and this is what I would do) the animations get passed to the LEDCube class and the LEDCube class has the code to look at the animations and decide what to do with his leds.

But think of either you need to give the animations a parameter that says, here this is the cube you're going to run on or you need to be telling the cube, here this is the animation I want you to run. What you don't want to do is tell LEDCube that he is in charge of handling the leds and then tell some animation class that they can just go in there and start messing with them while LEDCube isn't looking.

I want to control 343 WS2811 LEDs via the FastLED library. The LED Colors are stored in a CRGB matrix that contains the r, g and b value for every led. 24bit per led. That means If I want to change the color of the leds what an animation needs to I have to set values in this matrix. I already made several animations that can access this matrix because they are also in the LEDCube class. My complete Code looks like this:

I would not recommend looking at all functions of LEDCube because there is nothing doing that but I am posting the whole code that you have an overview of my Code and how it works.

/*
LEDCube.h
*/
#ifndef LEDCube_H
#define LEDCube_H
#include "Arduino.h"
#include <FastLED.h>
#include <Adafruit_NeoPixel.h>
#include "colorgnt.h"
#include "rainbowgnt.h"
#define NUMLEDS 343

typedef struct xy8bit {
 short x;
 short y;
};

class LEDCube
{
  protected:
  static const int  limit = 7;
  static const int  limitsqu = limit * limit;
  static const int  numLeds = limitsqu * limit;
  public:
  LEDCube() {
  FastLED.addLeds<WS2811, 7>(leds, 343);
  FastLED.setBrightness(255);
  FastLED.clear();
};
  int xyz2lednum(uint8_t x, uint8_t y, uint8_t z);
  void setLED(int lednumber, uint8_t r, uint8_t g, uint8_t b);
  void led(uint8_t x, uint8_t y, uint8_t z, uint8_t r, uint8_t g, uint8_t b);
  void setslice(char a, uint8_t pos, uint8_t r, uint8_t g, uint8_t b);
  void test(int wait);
  void ranColor(uint8_t r, uint8_t g, uint8_t b, uint8_t maxpercoffset, uint8_t *rgb);
  void sticks(int wait1, int wait2, boolean normalgravity, uint8_t r, uint8_t g, uint8_t b, uint8_t perc);
  void sticksi(int wait1, int wait2, boolean normalgravity, uint8_t r, uint8_t g, uint8_t b, uint8_t perc);
  void ani1();
  void fade(uint8_t startr, uint8_t startg, uint8_t startb, uint8_t endr, uint8_t endg, uint8_t endb, int wait, uint8_t step, int lednumber);
  void fademulti(uint8_t startr, uint8_t startg, uint8_t startb, uint8_t endr, uint8_t endg, uint8_t endb, int wait, uint8_t step, int quantity, int ledmultitest[]);
  void clearLEDs();
  void setall(uint8_t r, uint8_t g, uint8_t b);
  void loadLetter(char letter, xy8bit *setxy);
  int textxy2lednum(xy8bit xy);
  void displayHG(uint8_t r1, uint8_t g1, uint8_t b1, uint8_t r2, uint8_t g2, uint8_t b2, uint8_t wait, int cycles);
  void testprogress();
  void rendergradient(colorgnt &gradient, int cycles, char a, uint8_t wait);
  short LEDCube::cut24(short num);
  Adafruit_NeoPixel simulated = Adafruit_NeoPixel(1, 3, NEO_GRB + NEO_KHZ800);
  CRGB leds[343];
};
#endif
/*LEDCube.ino*/
#include <FastLED.h>
#include <Adafruit_NeoPixel.h>
#include "LEDCube.h"
#define DATA_PIN 7

Adafruit_NeoPixel simulated = Adafruit_NeoPixel(1, 3, NEO_GRB + NEO_KHZ800);

void setup() {
  delay(1000);
  Serial.begin(9600);
}

static const int  limit = 7;
static const int  limitsqu = limit * limit;
static const int  numLeds = limitsqu * limit;

LEDCube cube;
rainbowgnt gnt1;
rainbowgnt &gnt1ref = gnt1;

void loop() {
  cube.ani1();
  cube.clearLEDs();
  cube.sticks(40, 60, true, 255, 255, 255, 20);
  for (int i = 0; i < 21; i++) {
    cube.setall(255, 255, 255);
    delay(100);
  }
  cube.clearLEDs(); 
  cube.rendergradient(gnt1, 256, 'x', 5);
  cube.rendergradient(gnt1, 256, 'y', 5);
  cube.rendergradient(gnt1, 500, 'z', 5);
  cube.clearLEDs();
  cube.displayHG(255,125,10,255,10,125,200, 100);
  for (int i = 0; i < limit; i++) {
    cube.setslice('x', i, 255, 0, 0);
    FastLED.show();
    delay(100);
  }
  for (int i = 0; i < limit; i++) {
    cube.setslice('y', i, 0, 255, 0);
    FastLED.show();
    delay(100);
  }
  for (int i = 0; i < limit; i++) {
    cube.setslice('z', i, 0, 0, 255);
     FastLED.show();
     delay(100);
  }
  cube.sticksi(28, 0, false, 0, 0, 0, 0);
}

All .cpp files combined:

#include "Arduino.h"
#include "LEDCube.h"

void LEDCube::clearLEDs() {
  for (int i = 0; i < numLeds; i++) {
    setLED(i, 0, 0, 0);
  }
  FastLED.show();
}

void LEDCube::setall(uint8_t r, uint8_t g, uint8_t b) {
  for (int i = 0; i < numLeds; i++) {
    setLED(i, r, g, b);
  }
  FastLED.show();
}

void LEDCube::setslice(char a, uint8_t pos, uint8_t r, uint8_t g, uint8_t b) {
  switch (a) {
    case 'x':
      for (int z = 0; z < limit; z++) {
        for (int y = 0; y < limit; y++) {
          led(pos, y, z, r, g, b);
        }
      }
      break;

    case 'y':
      for (int z = 0; z < limit; z++) {
        for (int x = 0; x < limit; x++) {
          led(x, pos, z, r, g, b);
        }
      }
      break;

    case 'z':
      for (int x = 0; x < limit; x++) {
        for (int y = 0; y < limit; y++) {
          led(x, y, pos, r, g, b);
        }
      }
      break;
  }
}
/*
colorgnt.h
*/
#ifndef colorgnt_H
#define colorgnt_H
#include "LEDCube.h"

typedef struct rgb8bit {
 uint8_t r;
 uint8_t g;
 uint8_t b;
};

class colorgnt {
  public:
  colorgnt() {}
  virtual rgb8bit rgb(uint8_t position) {}
};
#endif
/*
rainbowgnt.h
*/
#ifndef rainbowgnt_H
#define rainbowgnt_H
#include "colorgnt.h"
#include "LEDCube.h"
class colorgnt;  //is this neccessary? 
class rainbowgnt: public colorgnt {
  public:
  rainbowgnt() {}
  
  virtual rgb8bit rgb(uint8_t position) {
 rgb8bit rgb;
 position = 255 - position;
  if (position < 85) {
 rgb.r = 255 - position * 3;
 rgb.g = 0;
 rgb.b = position * 3;
    return rgb;
  }
  if (position < 170) {
    position -= 85;
 rgb.r = 0;
 rgb.g = position * 3;
 rgb.b = 255 - position * 3;
    return rgb;
  }
  position -= 170;
  rgb.r = position * 3;
  rgb.g = 255 - position * 3;
  rgb.b = 0;
  return rgb;
}
};
#endif

I had to leave some functions because of the character limitation out.

The member functions of LEDCube are using each other to create animations. The last step of these animations is the function

void setLED(int lednumber, uint8_t r, uint8_t g, uint8_t b);

this function changes values in the CRGB 'leds' matrix.
To display the animations I am creating one instance of LEDCube in the .ino file and calling the public member functions of the LEDCube instance.
I want to make an animation that lets random LEDs light up and then fade to black. These LEDs should not fade all at the same time. Therefor I would make a class(lets call it ledclass) that represents one LED with member functions handling the fading and lighting up and other stuff needed therefor. To create the animation I would create in an member function of LEDCube an array of instances of ledclass.

My problem and reason why I created this topic is that one member function of ledclass has to change values in the CRGB 'leds' matrix and I ask how a member of ledclass can access/change the CRGB 'leds' matrix that is declared in LEDCube.

One important distinction that you need to think about. You're not calling methods from LEDCube class. You're calling methods from some instance of LEDCube. You have to pass that instance around if you want things to be able to access it.

Delta_G:
One important distinction that you need to think about. You're not calling methods from LEDCube class. You're calling methods from some instance of LEDCube. You have to pass that instance around if you want things to be able to access it.

Now I understand.... To do that I have to rebuild the code and classes. I will try doing what you recommended and explained me and if I am failing I will ask again in the next day. Thank you.

Here's an example of one class having an instance of another class as a member and calling its functions:

class ClassA {

   public:

   void output();
  
};


void ClassA::output(){
   Serial.println("Class A output!");
}


class ClassB {

  private:
  ClassA *myA;

  public:
  ClassB(ClassA *aInstance);

  void output();
  
};

ClassB::ClassB(ClassA *aInstance){
  myA = aInstance;
}

void ClassB::output(){
  myA->output();
  Serial.println("Called from B;");
}



ClassA theA;
ClassB theB(&theA);

void setup(){

Serial.begin(115200);
  delay(500);
  Serial.println("Setup running");
  theB.output();
}


void loop(){}

hallolo:
Now I understand.... To do that I have to rebuild the code and classes. I will try doing what you recommended and explained me and if I am failing I will ask again in the next day. Thank you.

and do ONE THING successfully... don't try and muddy the water trying to make multiple animation classes... do exactly ONE successfully!

:wink: