Class pointer trouble

Troubleshooting
1

Hi Spark gurus! I can’t figure out why my call to LedArray::on() or LedArray::off() works from the LedArray Constructor, but not when I call it outside of that(array.on() in blink-an-led.ino). What silliness am I doing? Thanks for any help!

// This #include statement was automatically added by the Spark IDE.
#include "LedArray.h"

// This #include statement was automatically added by the Spark IDE.
#include "SimpleLed.h"

#define LED1 A3
#define LED2 A2
#define LED3 A1
#define LED4 A0

LedArray array(LED1,LED2,LED3,LED4);

// This routine runs only once upon reset
void setup() {
  
}

// This routine gets called repeatedly, like once every 5-15 milliseconds.
// Spark firmware interleaves background CPU activity associated with WiFi + Cloud activity with your code. 
// Make sure none of your code delays or blocks for too long (like more than 5 seconds), or weird things can happen.
void loop() {
  array.on();
  delay(1000);               // Wait for 1000mS = 1 second
  array.off();
  delay(1000);
}

LedArray.cpp:

// SimpleLed - Library for maintaining and changing LED state.

#include "LedArray.h"
#include "application.h"

LedArray::LedArray(uint8_t pin1, uint8_t pin2, uint8_t pin3, uint8_t pin4) {
    SimpleLed led1(pin1);
    SimpleLed led2(pin2);
    SimpleLed led3(pin3);
    SimpleLed led4(pin4);
    
    leds[0] = &led1;
    leds[1] = &led2;
    leds[2] = &led3;
    leds[3] = &led4;
    
    off();
    delay(100);
    on();
    delay(100);
    off();
}

// turns led on
void LedArray::on() {
    for(int i = 0; i < 4; i++) {
        leds[i]->on();
    }
}

// turns led off
void LedArray::off() {
    for(int i = 0; i < 4; i++) {
        leds[i]->off();
    }
}

LedArray.h

// LedArray - Library for maintaining and changing LED state.

#ifndef LedArray_h
#define LedArray_h

#include "application.h"
#include "SimpleLed.h"

#define ON HIGH
#define OFF LOW

class LedArray
{
    public:
        LedArray(uint8_t pin1, uint8_t pin2, uint8_t pin3, uint8_t pin4);
        void on();
        void off();
    private:
        SimpleLed * leds[];
};

#endif

SimpleLed.cpp

// SimpleLed - Library for maintaining and changing LED state.

#include "SimpleLed.h"
#include "application.h"

SimpleLed::SimpleLed(uint8_t pin, bool initialState) {
    _pin = pin;
    pinMode(_pin, OUTPUT);
    setState(initialState);
}

// returns state of led
bool SimpleLed::getState() {
    return _state;
}

// set state of led and updates led
void SimpleLed::setState(bool state) {
    _state = state;
    updateLed();
}

// turns led on
void SimpleLed::on() {
    setState(ON);
}

// turns led off
void SimpleLed::off() {
    setState(OFF);
}

// toggles state of led and updates led
void SimpleLed::toggle() {
    setState(!_state);
}

// updates led to match the state
void SimpleLed::updateLed() {
    digitalWrite(_pin, _state);
}

SimpleLed.h

// SimpleLed - Library for maintaining and changing LED state.

#ifndef SimpleLed_h
#define SimpleLed_h

#include "application.h"

#define ON HIGH
#define OFF LOW

class SimpleLed
{
    public:
        SimpleLed(uint8_t pin, bool initialState = OFF);
        bool getState();
        void setState(bool state);
        void on();
        void off();
        void toggle();
    private:
        bool _state;
        int _pin;
        void updateLed();
};

#endif
2

Looks like you’re trying to take the address of local variables in the constructor, so when the constructor ends, those variables get deallocated and your pointers are no longer valid.

The simplest solution, if you don’t have a problem using dynamic memory allocation in your code, would be to change your LedArray constructor as follows:

LedArray::LedArray(uint8_t pin1, uint8_t pin2, uint8_t pin3, uint8_t pin4) {
  leds[0] = new SimpleLed(pin1);
  leds[1] = new SimpleLed(pin2);
  leds[2] = new SimpleLed(pin3);
  leds[3] = new SimpleLed(pin4);

  off();
  delay(100);
  on();
  delay(100);
  off();
}

By using dynamic allocation, the objects remain on the heap when the constructor exits, so your pointers will stay valid.

Technically you should also then make sure to delete the SimpleLed objects in your LedArray destructor to avoid memory leaks, but since you’re just creating a single global LedArray object it will never get destroyed anyway so it doesn’t matter in your case.

1 Like
3

@dpursell thank you so much! That worked perfectly and I re-educated myself as well: http://www.cplusplus.com/doc/tutorial/dynamic/