Transitioning to Object Oriented Model, but stuck!

Hi,

I have a simple program that works as intended, but in order to grow the complexity I have realized that I need to transition to an object oriented model to allow multi-tasking.

So at the moment this program makes a random LED on a ws2811 strip of 50 LEDs come on when a button is released.

#include <FastLED.h>
#include <Bounce.h>

#define NUM_LEDS 50
#define ledPin 2

struct CRGB leds[NUM_LEDS];

const int buttonPin = 3;
Bounce pushbutton = Bounce(buttonPin, 10);  // 10 ms debounce
int LED_NUMBER;
int LIT_LEDS[NUM_LEDS];

void setup() {
  pinMode(buttonPin, INPUT_PULLUP);
  FastLED.addLeds<WS2811, ledPin, RGB>(leds, NUM_LEDS);
  randomSeed(analogRead(0));
}

void loop()
{
  LED_NUMBER = random(0, NUM_LEDS);
  if (LIT_LEDS[LED_NUMBER] != 0) {
    LED_NUMBER++ ;
  } else {
    if (pushbutton.update()) {
      if (pushbutton.risingEdge()) {
        
        leds[LED_NUMBER] = CRGB::Red;
        LIT_LEDS[LED_NUMBER] = 1;
        FastLED.show();
      }
    }
  }
}

It works as intended, using the FastLED Library and the Bounce Library on a Teensy 3.1

My next step is to move the LED actions into separate objects so that more complex things can be done, like keeping LED on for 20 seconds before fading out, while other LEDs can be triggered. I want to store the status of each LED so that it is either ON, FADING OUT, or OFF.

So this is as far as I have got in separating out the code but I’m stuck and not sure where to go.

#include <FastLED.h>
#include <Bounce.h>

#define NUM_LEDS 50
#define ledPin 2

struct CRGB leds[NUM_LEDS];

const int buttonPin = 3;
Bounce pushbutton = Bounce(buttonPin, 10);  // 10 ms debounce
int LIT_LEDS[NUM_LEDS];
int LED_NUMBER;

class CandleTable
{
  public:
    CandleTable(int LED_NUMBER, int LIT_LEDS[NUM_LEDS]);
    void begin();
    void on();
    void fade();
    void off();
    byte CandleStatus();

  private:
    int _LED_NUMBER;
    int _LIT_LEDS[NUM_LEDS];
    byte _CandleStatus;
};

CandleTable::CandleTable(int LED_NUMBER, int LIT_LEDS[NUM_LEDS])
{
  _LED_NUMBER = LED_NUMBER;
  _LIT_LEDS[NUM_LEDS] = LIT_LEDS[NUM_LEDS];
}

void CandleTable::begin()
{
  FastLED.addLeds<WS2811, ledPin, RGB>(leds, NUM_LEDS);
}

void CandleTable::on()
{ leds[_LED_NUMBER] = CRGB::Red;
  _CandleStatus = 1;
  _LIT_LEDS[_LED_NUMBER] = 1;
  FastLED.show();
  Serial.println(_LED_NUMBER);
}

void CandleTable::fade()
{
  _CandleStatus = 2;
}

void CandleTable::off()
{ leds[_LED_NUMBER] = CRGB::Black;
  _CandleStatus = 3;
}

byte CandleTable::CandleStatus()
{
  return _CandleStatus;
}

void setup() {
  pinMode(buttonPin, INPUT_PULLUP);
  randomSeed(analogRead(0));
  Serial.begin(9600);
}

void loop()
{
  LED_NUMBER = random(0, NUM_LEDS);
  if (LIT_LEDS[LED_NUMBER] != 0) {
    LED_NUMBER++ ;
  } else {
    if (pushbutton.update()) {
      if (pushbutton.risingEdge()) {

       CandleTable::on();

      }
    }
  }
}

This is the error message that I am getting:

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

In file included from C:\Users\jay-jay\Documents\Dropbox\Arduino\Pushbutton_random_light-2\Pushbutton_random_light-2.ino:1:0:
C:\Users\jay-jay\Documents\Dropbox\Arduino\libraries\FastLED-3.1.0/FastLED.h:12:2: warning: #warning FastLED version 3.001.000 (Not really a warning, just telling you here.) [-Wcpp]
#warning FastLED version 3.001.000 (Not really a warning, just telling you here.)
^
C:\Users\jay-jay\Documents\Dropbox\Arduino\Pushbutton_random_light-2\Pushbutton_random_light-2.ino:64:57: error: invalid conversion from ‘int’ to ‘int*’ [-fpermissive]
CandleTable CandleNumber(LED_NUMBER, LIT_LEDS[LED_NUMBER]);
^
C:\Users\jay-jay\Documents\Dropbox\Arduino\Pushbutton_random_light-2\Pushbutton_random_light-2.ino:30:1: error: initializing argument 2 of ‘CandleTable::CandleTable(int, int*)’ [-fpermissive]
CandleTable::CandleTable(int LED_NUMBER, int LIT_LEDS[NUM_LEDS])
^
exit status 1
Error compiling.

Any pointers would be very gratefully received!

Rob

I can't comment on how accurately your class models your led array problem, but I would have expected to see an explicit creation somewhere of an object from the CandleTable class.
For example:

CandleTable myCandleTable1( parameter1, parameter2 )

It is also not clear to me what your intention here is in the constructor function:

_LIT_LEDS[NUM_LEDS] = LIT_LEDS[NUM_LEDS];

Thanks very much for your reply.

6v6gt:
I can't comment on how accurately your class models your led array problem, but I would have expected to see an explicit creation somewhere of an object from the CandleTable class.
For example:

CandleTable myCandleTable1( parameter1, parameter2 )

The idea is that the button will generate a random number that will be passed to each object instance to define which LED is being activated. Is it possible to create multiple instances dynamically?

It is also not clear to me what your intention here is in the constructor function:

_LIT_LEDS[NUM_LEDS] = LIT_LEDS[NUM_LEDS];

LIT_LEDS[NUM_LEDS] is supposed to track the status of each LED in my array.

This tutorial recommended that I separate public and private functions, so the line you mentioned was attempting to pass the public array LIT_LEDS[NUM_LEDS] to the private function.

I may have got over complicated there though as I am very new to this and am replying on that tutorial I linked to for structuring my code.

Honestly, YouTube doesn't seem to me like a good place to learn OO.

_LIT_LEDS[NUM_LEDS] = LIT_LEDS[NUM_LEDS];

This code takes the value behind the original array and stores it behind the local, absolutely superfluous, RAM eating, uninitialized array.

The name of the class is confusing. It describes a single led, there is no table.

If you want the class to act on only one array element, you could just store its address in the class.
If you want to use the array notation or have easy access to the index after creation, store the address of the array and the index.

This might help you think more OO (hint:when you are mostly making calls using the :: operator, and not the . operator, you don’t really have objects).

The LED array should be inside your candle object so only it can access the leds, and not declared globally.

Your main loop code should look more like this as an example (untested. Just an example):

CandleTable candle; // create a candle table object

void loop()
{
    if (pushbutton.update()) 
    {
        if (pushbutton.risingEdge()) 
        {
            // find a LED that is currently off.
            int ledNumber = 0;
            do
            {
                ledNumber = random(0, NUM_LEDS);
            }
            while (candle.isLedLit(ledNumber) != 0);
                
            candle.on(ledNumber);
        }
    }
}

or even better:

CandleTable candle; // create a candle table object

void loop()
{
    if (pushbutton.update()) 
    {
        if (pushbutton.risingEdge()) 
        {             
            candle.turnOnRandomLed();
        }
    }
}

Whandall:
The name of the class is confusing. It describes a single led, there is no table.

You're absolutely right. I originally thought that I would have to define the CandleTable as one function in order to monitor the status of each 'candle' on the CandleTable

store the address of the array and the index.

I'll have to try and get my head around this!

arduinodlb:
This might help you think more OO (hint:when you are mostly making calls using the :: operator, and not the . operator, you don't really have objects).

The LED array should be inside your candle object so only it can access the leds, and not declared globally.

You main loop code should look more like this as an example (untested. Just an example):

CandleTable candle; // create a candle table object

void loop()
{
    if (pushbutton.update())
    {
        if (pushbutton.risingEdge())
        {
            // find a LED that is currently off.
            int ledNumber = 0;
            do
            {
                ledNumber = random(0, NUM_LEDS);
            }
            while (candle.isLedLit(ledNumber) != 0);
               
            candle.on(ledNumber);
        }
    }
}




or even better:



CandleTable candle; // create a candle table object

void loop()
{
    if (pushbutton.update())
    {
        if (pushbutton.risingEdge())
        {           
            candle.turnOnRandomLed();
        }
    }
}

This is great, thanks.

Am I right then in thinking that if there is only ever going to be one CandleTable then it doesn’t need to defined as an object? Just the elements on the table?

Sorry I’m trying to get my head around how to structure my code. If I eventually want all the LEDs to work together in an animation, would they be grouped in an object that contains the individual LED/Candle object?

At the moment I’m just trying to work out where my array goes that stores which LEDs are lit, inside the LED/Candle object or not?

Sorry I’m trying to get my head around how to structure my code. If I eventually want all the LEDs to work together in an animation, would they be grouped in an object that contains the individual LED/Candle object?

If object oriented programming is relevant to your situation, the objects should be (relatively) obvious. You seem to have a CandleTable object, which implies that it will hold Candle objects. If you really can’t figure out whether that is an appropriate breakdown, then perhaps OO isn’t appropriate for this project.

If you decide that it is, then make a list of the things that a Candle should do. Add a separate section to list what the Candle needs to know to do its job(s).

Make a list of the things a CandleTable object should do. Add a separate section to list what the CandleTable needs to know to do its job(s).

The what-to-do stuff becomes methods, The what-needs-to-be-known stuff becomes member fields.

If you can’t figure out what an object needs to do, or what it needs to know, then the object probably doesn’t need to exist, and, therefore, there is no need for a class.

Sorry I’m trying to get my head around how to structure my code. If I eventually want all the LEDs to work together in an animation, would they be grouped in an object that contains the individual LED/Candle object?

You may be getting closer. How about a class that is just an LED? You can store the RGB colors and any relevant
funcitons and other data in the class (object). Then instantiate each LED object into an array:
YourLEDclass LEDs = {0, 1, 2, 3, …}

Your objects would be named LEDs[0], LEDs[1], etc. Not very pretty names but what else would you name each LED?

The trouble is that you are using the FastLED library which is essentially doing this already.
If you can inherit from that FastLED – I don’t know if that’s possible – you could extend its functionality.
Or you can dump that library and use your own but you would also have to generate the serial LED data stream yourself.

dillonradio:
Am I right then in thinking that if there is only ever going to be one CandleTable then it doesn’t need to defined as an object? Just the elements on the table?

Not necessarily. Even if you only have one instance of the class (one object) it still encapsulates the data and methods. That is, it contains and groups together the essence of the object, and forces access only through the object, making the overall program structure cleaner (more grouped).

Sorry I’m trying to get my head around how to structure my code. If I eventually want all the LEDs to work together in an animation, would they be grouped in an object that contains the individual LED/Candle object?

That’s certainly a way to go. An array of LED objects held by the Candle object. Your FastLED use might make that difficult of ugly. You might want to just put your FastLed stuff inside the Candle object, then just move your existing LED stuff inside the Candle object too, but not necessarily using a new LED object.

At the moment I’m just trying to work out where my array goes that stores which LEDs are lit, inside the LED/Candle object or not?

If you have a Candle object then yes, inside the Candle object.