Turning my sketch into a class / library - stuck on structs

Hi,

I am trying to turn my sketch into a library and am a bit unsure of how to include my struct in the header.

I have attached the main sketch and the header file. You will see that I have not attempted to include the struct yet so I just need a pointer on how to do that and I can't find a good simple tutorial anywhere.

I have not started the .cpp file yet.

Thanks for any help!

Rob

Candle.h (942 Bytes)

NeoCandle_for_FastLED_v11__library_.ino (12.3 KB)

Simply move the struct declaration into candle.h.
I'd change candleAnimation() to accept a reference to the struct, not an index:
void candleAnimation(struct candleAnimation &params); //or *params
Different names for the function and struct were nice.

OK sounds straight forward but where it's where it goes that I am confused by:

Here:

/*
  Candle.h - Library for FastLED Candle simulation.
  Original candle for Adafruit NeoPixel
  by Tim Bartlett, December 2013
  https://github.com/timpear/NeoCandle

  Non-blocking version for FastLED
  Adapted by Rob Hilken, March 2016
  Released into the public domain.
*/
#ifndef Candle_h
#define Candle_h

#include "Arduino.h"
#include "FastLED.h"
#include "elapsedMillis.h"

#define LED_PIN 2
#define NUM_LEDS 50
#define BRIGHTNESS  100 // set at 25 to run powersaving
struct CRGB leds[NUM_LEDS];

struct candleProperties
{
  byte fireState;
  byte cycleTime;
  unsigned int onForMs;
  elapsedMillis animTimeElapsed;
  elapsedMillis fireTimeElapsed;
  bool fDimming = true;
  int grnPx = grnHigh;
  byte maxRep;
  byte fDelay;
  byte burnRep;
  byte burnDepth;
  byte burnDelay;
  byte burnLow;
  byte flickerState;
  byte flickerRep;
  byte flickerDepth;
  byte flickDelay;
  byte flickLow;
  byte flutterState;
  byte flutterRep;
  byte flutterDepth;
  byte flutDelay;
  byte flutLow;
};
candleProperties candle[NUM_LEDS];

class myCandle
{
  public:
    myCandle(
      byte redPx,
      byte grnHigh,
      byte bluePx
    );
    void initializeCandleRandoms(byte n);
    void candleAnimation(byte n);
    void on(byte n);
    bool burn(unsigned int f, byte n);
    bool flicker(unsigned int f, byte n);
    bool flutter(unsigned int f, byte n);
    bool fire(byte grnLow, byte n);

  private:
    byte _redPx;
    byte _grnHigh;
    byte _bluePx;
}

#endif

or here:

/*
  Candle.h - Library for FastLED Candle simulation.
  Original candle for Adafruit NeoPixel
  by Tim Bartlett, December 2013
  https://github.com/timpear/NeoCandle

  Non-blocking version for FastLED
  Adapted by Rob Hilken, March 2016
  Released into the public domain.
*/
#ifndef Candle_h
#define Candle_h

#include "Arduino.h"
#include "FastLED.h"
#include "elapsedMillis.h"

#define LED_PIN 2
#define NUM_LEDS 50
#define BRIGHTNESS  100 // set at 25 to run powersaving
struct CRGB leds[NUM_LEDS];

class myCandle
{
  public:
    myCandle(
      byte redPx,
      byte grnHigh,
      byte bluePx

     struct candleProperties
      {
        byte fireState;
        byte cycleTime;
        unsigned int onForMs;
        elapsedMillis animTimeElapsed;
        elapsedMillis fireTimeElapsed;
        bool fDimming = true;
        int grnPx = grnHigh;
        byte maxRep;
        byte fDelay;
        byte burnRep;
        byte burnDepth;
        byte burnDelay;
        byte burnLow;
        byte flickerState;
        byte flickerRep;
        byte flickerDepth;
        byte flickDelay;
        byte flickLow;
        byte flutterState;
        byte flutterRep;
        byte flutterDepth;
        byte flutDelay;
        byte flutLow;
     };
    candleProperties candle[NUM_LEDS];

    );
    void initializeCandleRandoms(byte n);
    void candleAnimation(byte n);
    void on(byte n);
    bool burn(unsigned int f, byte n);
    bool flicker(unsigned int f, byte n);
    bool flutter(unsigned int f, byte n);
    bool fire(byte grnLow, byte n);

  private:
    byte _redPx;
    byte _grnHigh;
    byte _bluePx;
}

#endif

or here?

/*
  Candle.h - Library for FastLED Candle simulation.
  Original candle for Adafruit NeoPixel
  by Tim Bartlett, December 2013
  https://github.com/timpear/NeoCandle

  Non-blocking version for FastLED
  Adapted by Rob Hilken, March 2016
  Released into the public domain.
*/
#ifndef Candle_h
#define Candle_h

#include "Arduino.h"
#include "FastLED.h"
#include "elapsedMillis.h"

#define LED_PIN 2
#define NUM_LEDS 50
#define BRIGHTNESS  100 // set at 25 to run powersaving
struct CRGB leds[NUM_LEDS];

class myCandle
{
  public:
    myCandle(
      byte redPx,
      byte grnHigh,
      byte bluePx
    );

struct candleProperties
{
      byte fireState;    
      byte cycleTime;
      unsigned int onForMs;
      elapsedMillis animTimeElapsed;
      elapsedMillis fireTimeElapsed;
      bool fDimming = true;
      int grnPx = grnHigh;
      byte maxRep;
      byte fDelay;
      byte burnRep;
      byte burnDepth;
      byte burnDelay;
      byte burnLow;
      byte flickerState;
      byte flickerRep;
      byte flickerDepth;
      byte flickDelay;
      byte flickLow;
      byte flutterState;
      byte flutterRep;
      byte flutterDepth;
      byte flutDelay;
      byte flutLow;
    };

    candleProperties candle[NUM_LEDS];

    void initializeCandleRandoms(byte n);
    void candleAnimation(byte n);
    void on(byte n);
    bool burn(unsigned int f, byte n);
    bool flicker(unsigned int f, byte n);
    bool flutter(unsigned int f, byte n);
    bool fire(byte grnLow, byte n);

  private:
    byte _redPx;
    byte _grnHigh;
    byte _bluePx;
}

#endif

I'd change candleAnimation() to accept a reference to the struct, not an index:
void candleAnimation(struct candleAnimation &params); //or *params

I'm not exactly sure how I would implement this, I've never used references before.

Leave the array definitions (candle[NUM_LEDS], leds[NUM_LEDS]...) in the user code. The user shall create and size these arrays as required in his code, and when needed passes a reference or pointer to the class instance.

References are equivalent to pointers, except that a reference never can be NULL (the compiler checks that). And there is no reference arithmetic. The syntax is a bit different, ptr->member becomes ref.member. You can continue using pointers, but references are better C++ style.

Why no make candleProperties a class? I suspect that if you did that, you'd see that defing the thing inside another class is not the proper way.

So it's:

#ifndef Candle_h
#define Candle_h

#include "Arduino.h"
#include "FastLED.h"
#include "elapsedMillis.h"

struct candleProperties
{
 byte fireState;
 byte cycleTime;
 unsigned int onForMs;
 elapsedMillis animTimeElapsed;
 elapsedMillis fireTimeElapsed;
 bool fDimming = true;
 int grnPx = grnHigh;
 byte maxRep;
 byte fDelay;
 byte burnRep;
 byte burnDepth;
 byte burnDelay;
 byte burnLow;
 byte flickerState;
 byte flickerRep;
 byte flickerDepth;
 byte flickDelay;
 byte flickLow;
 byte flutterState;
 byte flutterRep;
 byte flutterDepth;
 byte flutDelay;
 byte flutLow;
};

class myCandle
{
 public:
   myCandle(
     byte redPx,
     byte grnHigh,
     byte bluePx
   );
   void initializeCandleRandoms(byte n);
   void candleAnimation(byte n);
   void on(byte n);
   bool burn(unsigned int f, byte n);
   bool flicker(unsigned int f, byte n);
   bool flutter(unsigned int f, byte n);
   bool fire(byte grnLow, byte n);

 private:
   byte _redPx;
   byte _grnHigh;
   byte _bluePx;
}

#endif

and leave

#define LED_PIN 2
#define NUM_LEDS 50
#define BRIGHTNESS  100
struct CRGB leds[NUM_LEDS];
candleProperties candle[NUM_LEDS];

in the main code with the globals?

Was I right to put the other global variables from my main sketch into myCandle() in the class?

Was I right to put the other global variables from my main sketch into myCandle() in the class?

Yes, yes, and no.

myCandle is a lousy name for a class. A class defines an object, like a Candle, an Animal, a Dog, a HardwareSerial, an EthernetClient, etc.

Can you hold up an object, and say this is a myCandle? Of course not. So, why give the class a silly name?

PaulS:
Yes, yes, and no.

Could you elaborate?

PaulS:
So, why give the class a silly name?

OK fair enough, but I was constructing a 'candle' with the struct candleProperties so thought I needed to call it something different.

I think I'm missing something important in my understanding here.

What's the intended purpose of your class/library?
Who shall use it, and how?

If you want it only for your private purposes, you can keep all values in global variables, accessible to both user and library code. As a general library, you have to separate the user specific data from the library code, and implement means to pass the data from the main program to the library, by pointers or references.

DrDiettrich:
What's the intended purpose of your class/library?
Who shall use it, and how?

If you want it only for your private purposes, you can keep all values in global variables, accessible to both user and library code. As a general library, you have to separate the user specific data from the library code, and implement means to pass the data from the main program to the library, by pointers or references.

That's a useful pointer thanks.

Originally I just wanted to use it to organise my code as I will have lots of different things that the array of leds can do depending on different inputs... but then I thought that this would be useful to share for people working on other projects so I wanted to use best practices to make a robust library suitable for sharing - also to learn how to do this.

Am I right then in thinking that inside

public:
   myCandle(
     
//in here


   );

I just put the variables that the user will be passing to the object and not internal variables?

The constructor is a good place for passing in the user specific data. Another place is a begin() method, to be called in setup().

DrDiettrich:
The constructor is a good place for passing in the user specific data. Another place is a begin() method, to be called in setup().

In relation my first question about the struct that I use to hold the candle properties, which is not user specific, is using a struct the best way to hold that data?

I have used the name candle to reference those variables, but I think, based on PaulS's remarks, that candle would be better as a name for the class. I'm getting confused as it seems that a struct is very similar to a class and I'm not sure if I should be thinking about the library as one class or two now.

Thanks for helping me get my head around this!

I'm getting confused as it seems that a struct is very similar to a class and I'm not sure if I should be thinking about the library as one class or two now.

A struct is a C thing - a way to hold related data. A class is a C++ thing - a way to hold related data AND methods.

C++ added the ability to have methods in structs. The only difference between a class and a struct, in C++, is whether the methods are public or private by default.

Personally, I think of candle properties as being a class, because you should have (public) getters and setters for the (private) fields.

In C#, you would not have the choice. candleProperies would have to be a class.

Every time I start creating a struct, I end up determining that the instances should be smarter, and should have methods, like knowing how to add themselves to a list, so I end up refactoring the code to make the struct a class.

PaulS:
A struct is a C thing - a way to hold related data. A class is a C++ thing - a way to hold related data AND methods.

C++ added the ability to have methods in structs. The only difference between a class and a struct, in C++, is whether the methods are public or private by default.

Personally, I think of candle properties as being a class, because you should have (public) getters and setters for the (private) fields.

In C#, you would not have the choice. candleProperies would have to be a class.

Every time I start creating a struct, I end up determining that the instances should be smarter, and should have methods, like knowing how to add themselves to a list, so I end up refactoring the code to make the struct a class.

That makes it much clearer thanks.

I think I've figured out what I was missing. My sketch already uses the struct to make the sketch work on an array of LEDs but when making a library it just needs to work on one LED and then instances of the class handle the array of LEDs.

I think I still should be including the methods in the class as well as the candleProperties. I hope that sounds right?

dillonradio:
That makes it much clearer thanks.

I think I've figured out what I was missing. My sketch already uses the struct to make the sketch work on an array of LEDs but when making a library it just needs to work on one LED and then instances of the class handle the array of LEDs.

I think I still should be including the methods in the class as well as the candleProperties. I hope that sounds right?

There is magic in Paul's note above

PaulS:
Every time I start creating a struct, I end up determining that the instances should be smarter, and should have methods, like knowing how to add themselves to a list, so I end up refactoring the code to make the struct a class.

I recommend that instead of thinking about classes and structs, you think about Objects.

each instance of a Candle object has properties. An obvious analogy is a candle.

Candles have Color, Height, Diameter and they are lit or not lit. if you want to get/set these properties outside the instance of the class, you make them public, otherwise you make them private:

class Candle{
  public:

  private:
    int Color;
    int Height;
    int Diameter;
    bool isLit;
};

what will you do with a candle? Well you can lite it or put it out.... you want to be able to lite or put out any candle from anywhere, so you have to create public member functions:

class Candle{
  public:
    void lite();
    void putOut();
  private:
    int Color;
    int Height;
    int Diameter;
    bool isLit;
};

you also may want to know if it is burning or not:

class Candle{
  public:
    void lite();
    void putOut();
    bool isBurning();

  private:
    int Color;
    int Height;
    int Diameter;
    bool isLit;
};

its height may have something to to with remaining run time, so you add a function to return its height (which is private):

class Candle{
  public:
    void lite();
    void putOut();
    bool isBurning();
    bool getHeight();

  private:
    int Color;
    int Height;
    int Diameter;
    bool isLit;
};

Let the class evolve as you find the need for properties and or member functions (methods).

Thanks for your help and pointers so far.

I am currently getting an error that I can't figure out:

Arduino: 1.6.7 (Windows 10), TD: 1.27, Board: "Teensy 3.2 / 3.1, Serial, 96 MHz optimized (overclock), US English"

C:\Users\jay-jay\Documents\Dropbox\Arduino\Votive Gambler\NeoCandle_for_FastLED_v10.10_-_Class\NeoCandle_for_FastLED_v10.10_-_Class.ino:19:1: error: 'Candle' does not name a type
Candle myCandle[NUM_LEDS];
^
C:\Users\jay-jay\Documents\Dropbox\Arduino\Votive Gambler\NeoCandle_for_FastLED_v10.10_-_Class\NeoCandle_for_FastLED_v10.10_-_Class.ino: In function 'void setup()':
C:\Users\jay-jay\Documents\Dropbox\Arduino\Votive Gambler\NeoCandle_for_FastLED_v10.10_-_Class\NeoCandle_for_FastLED_v10.10_-_Class.ino:36:5: error: 'myCandle' was not declared in this scope
myCandle.begin(i);
^
C:\Users\jay-jay\Documents\Dropbox\Arduino\Votive Gambler\NeoCandle_for_FastLED_v10.10_-_Class\NeoCandle_for_FastLED_v10.10_-_Class.ino: In function 'void loop()':
C:\Users\jay-jay\Documents\Dropbox\Arduino\Votive Gambler\NeoCandle_for_FastLED_v10.10_-_Class\NeoCandle_for_FastLED_v10.10_-_Class.ino:44:5: error: 'myCandle' was not declared in this scope
myCandle.animateCandle(i);
^
exit status 1
Error compiling.

  This report would have more information with
  "Show verbose output during compilation"
  enabled in File > Preferences.

My main sketch is:

/*  ========== FastLED Candle ============
  Original candle for Adafruit NeoPixel
  by Tim Bartlett, December 2013
  https://github.com/timpear/NeoCandle

  Non-blocking version for FastLED
  Adapted by Rob Hilken, March 2016
*/

#include <FastLED.h>
#include <elapsedMillis.h>
#include "Candle.h"

#define LED_PIN 2
#define NUM_LEDS 50
#define BRIGHTNESS  100 // set at 25 to run powersaving
struct CRGB leds[NUM_LEDS];

Candle myCandle[NUM_LEDS];


/*  ========== main arduino sketch ============
    in the loop -  animateCandle(0); // pass candleNumber
*/

void setup()
{
  FastLED.addLeds<WS2811, LED_PIN, RGB>(leds, NUM_LEDS);
  FastLED.setBrightness( BRIGHTNESS );
  fill_solid(leds, NUM_LEDS, CRGB::Black);

  randomSeed(analogRead(0));
  Serial.begin(9600);

  for (byte i = 0; i < NUM_LEDS; i++) {
    myCandle.begin(i);
  }
}


void loop()
{
  for (byte i = 0; i < NUM_LEDS; i++) {
    myCandle.animateCandle(i);
  }
  FastLED.show();
}

Candle.h

/*  ========== FastLED Candle ============
  Original candle for Adafruit NeoPixel
  by Tim Bartlett, December 2013
  https://github.com/timpear/NeoCandle

  Non-blocking version for FastLED
  Adapted by Rob Hilken, March 2016
*/

// ensure this library description is only included once
#ifndef Candle_h
#define Candle_h



class Candle
{
  public:

    Candle(byte candlePosition);
    void begin(byte n);
    void animateCandle(byte n);
   

  private:

    byte _redPx;
    byte _grnHigh;
    byte _bluePx;

    byte _fireState;
    byte _cycleTime;
    unsigned int _onForMs;
    elapsedMillis _animTimeElapsed;
    elapsedMillis _fireTimeElapsed;
    bool _fDimming;
    int _grnPx;
    byte _maxRep;
    byte _fDelay;
    byte _burnRep;
    byte _burnDepth;
    byte _burnDelay;
    byte _burnLow;
    byte _flickerState;
    byte _flickerRep;
    byte _flickerDepth;
    byte _flickDelay;
    byte _flickLow;
    byte _flutterState;
    byte _flutterRep;
    byte _flutterDepth;
    byte _flutDelay;
    byte _flutLow;
    
    void initializeCandleRandoms(byte n);
    void on(byte n);
    bool burn(unsigned int f, byte n);
    bool flicker(unsigned int f, byte n);
    bool flutter(unsigned int f, byte n);
    bool fire(byte _grnLow, byte n);

};
#endif

.cpp file attached

Candle.cpp (7.13 KB)

Here you create and array of Candle instances of size NUM_LEDS (50):

Candle myCandle[NUM_LEDS];

here you forget that you created 50 and try to access a member function by using the pointer to the first element of the array of instances:

for (byte i = 0; i < NUM_LEDS; i++) 
{
  myCandle.begin(i);
}

try:

for (byte i = 0; i < NUM_LEDS; i++) 
{
  myCandle[i].begin();  // each instance can be addressed like any array
}

That makes sense, and I'm sure I have many problems with how I am addressing the array later in the program, but for now I am still getting the same error messages when trying to compile.

I then thought it might be due to an arduino bug that I have run into before - Errors compiling Fastled library teensy 3.2 · Issue #234 · FastLED/FastLED · GitHub so I tried adding to define the class again in my sketch and the sketch doesn't compile but at least those errors have now gone...

/*  ========== FastLED Candle ============
  Original candle for Adafruit NeoPixel
  by Tim Bartlett, December 2013
  https://github.com/timpear/NeoCandle

  Non-blocking version for FastLED
  Adapted by Rob Hilken, March 2016
*/

#include <FastLED.h>
#include <elapsedMillis.h>
#include "Candle.h"

#define LED_PIN 2
#define NUM_LEDS 50
#define BRIGHTNESS  100 // set at 25 to run powersaving
struct CRGB leds[NUM_LEDS];

class Candle myCandle[NUM_LEDS];
Candle myCandle[NUM_LEDS];


/*  ========== main arduino sketch ============
    in the loop -  animateCandle(0); // pass candleNumber
*/

void setup()
{
  FastLED.addLeds<WS2811, LED_PIN, RGB>(leds, NUM_LEDS);
  FastLED.setBrightness( BRIGHTNESS );
  fill_solid(leds, NUM_LEDS, CRGB::Black);

  randomSeed(analogRead(0));
  Serial.begin(9600);

  for (byte i = 0; i < NUM_LEDS; i++) {
    myCandle[i].begin();
  }
}


void loop()
{
  for (byte i = 0; i < NUM_LEDS; i++) {
    myCandle[i].animateCandle();
  }
  FastLED.show();
}

Was my error really due to that bug or was I doing something more fundamental wrong with the way I am using the library files?

class Candle myCandle[NUM_LEDS];

Can you point to any example that has a line like this in it?

PaulS:

class Candle myCandle[NUM_LEDS];

Can you point to any example that has a line like this in it?

Ha! No, you're right but it does stop that compile error I was having. I hoped that the class would be declared by using #include "Candle.h" in the cpp file and main sketch.

I'm missing something obviously...