When creating an objectA inside another objectB, is objectA local to objectB and will objectA exist beyond objectB's instantiation?

Question

My goal is to create two classes, LED and PWM, with the intent that every LED object creates a local PWM object for controlling the brightness. The idea is that the user can create an LED object without having to set up the PWM object, while retaining the option of creating a PWM object separately for motor speed control, etc.

The problem comes when I instantiate the PWM object inside the LED object. The code compiles with no errors; however, the result is as though the PWM object disappears as soon as the LED object's constructor has completed. (None of the events related to the PWM module are occuring when I trigger a corresponding task. Specifically, the pwm.stop and pwm.start functions use an empty while loop to wait for specific events that signal when a task is complete, and the loop shortcut automatically initiates a task when the loop complete event occurs. None of these events are occurring when the appropriate tasks are triggered. The data sheet warns: "Note that the peripheral must be enabled before tasks and events can be used." This leads me to believe that the pwm.enable() is not running.)

ohlemacher says this about constructors inside constructors:

"...It just doesn't do what you want. The inner constructor will construct a temporary local object which gets deleted once the outer constructor returns."

Is there a better way to auto-generate the PWM object upon creation of an LED object? Is it possible to do what I want without making the PWM object a member of the LED object?

pwm.h

#ifndef rav_nrf52840_pwm_h
#define rav_nrf52840_pwm_h

#include "rav_nrf52840_baseConst.h"

typedef enum {DIV1,DIV2,DIV4,DIV8,DIV16,DIV32,DIV64,DIV128} prescaler;
typedef enum {PWM0,PWM1,PWM2,PWM3} pwmModule;
typedef enum {COMMON,GROUPED,INDIVIDUAL,WAVEFORM} decoderLoad;

class PWM {

private:

    unsigned int base_address;

    void enable_pwm (bool en);

    bool start_pwm (void);

    void stop_pwm (void);

public:

    PWM ();

    PWM (pwmModule module, bool looping, bool mode, int count, prescaler scale);

    void init (decoderLoad load, bool decoder_mode, int loop_count);

    void sequence (int seq_number, unsigned int *pointer, int count, int refresh, int enddelay);

    void pinSelect (int channel, int port, int pin, bool disconnect);

    void enable (void);

    void disable (void);

    bool start (void);

    void stop (void);
};

#endif

pwm.cpp

#include "rav_nrf52840_pwm.h"
#include <cstdint>

PWM::PWM (){
    #ifndef rav_nrf52840_pwm_pwm3
    #define rav_nrf52840_pwm_pwm3
    pwmModule module = PWM3;
    #else
    #ifndef rav_nrf52840_pwm_pwm2
    #define rav_nrf52840_pwm_pwm2
    pwmModule module = PWM2;
    #else
    #ifndef rav_nrf52840_pwm_pwm1
    #define rav_nrf52840_pwm_pwm1
    pwmModule module = PWM1;
    #else
    #ifndef rav_nrf52840_pwm_pwm0
    #define rav_nrf52840_pwm_pwm0
    pwmModule module = PWM0;
    #endif
    #endif
    #endif
    #endif

    bool looping = true;
    bool mode = 0;
    int count = 0x7FFF;
    prescaler scale = DIV4;
    switch (module){

        default:
            ;
        break;

        case PWM0:
            base_address = BASE_ADDRESS_PWM0;
        break;

        case PWM1:
            base_address = BASE_ADDRESS_PWM1;
        break;

        case PWM2:
            base_address = BASE_ADDRESS_PWM2;
        break;

        case PWM3:
            base_address = BASE_ADDRESS_PWM3;
        break;
    }
    unsigned int * pwm_mode_reg = (unsigned int *)(base_address + REGISTER_OFFSET_PWM_MODE);
    unsigned int * countertop_reg = (unsigned int *)(base_address + REGISTER_OFFSET_COUNTERTOP);
    unsigned int * prescaler_reg = (unsigned int *)(base_address + REGISTER_OFFSET_PRESCALER);
    unsigned int * shortcut_reg = (unsigned int *)(base_address + REGISTER_OFFSET_SHORTS);
    *pwm_mode_reg = mode;
    *countertop_reg = count;
    *prescaler_reg = scale;
    if (looping){
        *shortcut_reg = 0x04;  //  Enable looping
    }
}

PWM::PWM (pwmModule module, bool looping, bool mode, int count, prescaler scale){
    switch (module){

        default:
            ;
        break;

        case PWM0:
            base_address = BASE_ADDRESS_PWM0;
        break;

        case PWM1:
            base_address = BASE_ADDRESS_PWM1;
        break;

        case PWM2:
            base_address = BASE_ADDRESS_PWM2;
        break;

        case PWM3:
            base_address = BASE_ADDRESS_PWM3;
        break;
    }
    unsigned int * pwm_mode_reg = (unsigned int *)(base_address + REGISTER_OFFSET_PWM_MODE);
    unsigned int * countertop_reg = (unsigned int *)(base_address + REGISTER_OFFSET_COUNTERTOP);
    unsigned int * prescaler_reg = (unsigned int *)(base_address + REGISTER_OFFSET_PRESCALER);
    unsigned int * shortcut_reg = (unsigned int *)(base_address + REGISTER_OFFSET_SHORTS);
    *pwm_mode_reg = mode;
    *countertop_reg = count;
    *prescaler_reg = scale;
    if (looping){
        *shortcut_reg = 0x04;  //  Enable looping
    }
}





// PRIVATE

void PWM::enable_pwm (bool en){
    unsigned int * pwm_enable_reg = (unsigned int *)(base_address + REGISTER_OFFSET_ENABLE);
    *pwm_enable_reg = en;
}

bool PWM::start_pwm (void){
    unsigned int * start_seq0_task = (unsigned int *)(base_address + TASK_OFFSET_SEQSTART_0);
    volatile unsigned int * seq0_started_event = (unsigned int *)(base_address + EVENT_OFFSET_SEQSTARTED_0);
    *start_seq0_task = true;
    while(!*seq0_started_event){}
    *seq0_started_event = false;
    return 1;
}

void PWM::stop_pwm (void){
    unsigned int * pwm_stop_task = (unsigned int *)(base_address + TASK_OFFSET_PWM_STOP);
    volatile unsigned int * pwm_stopped_event = (unsigned int *)(base_address + EVENT_OFFSET_STOPPED);
    *pwm_stop_task = true;
    while(!*pwm_stopped_event){}
    *pwm_stopped_event = false;
}





// PUBLIC

void PWM::init (decoderLoad load, bool decoder_mode, int loop_count){
    unsigned int * decoder_reg = (unsigned int *)(base_address + REGISTER_OFFSET_DECODER);
    unsigned int * loop_reg = (unsigned int *)(base_address + REGISTER_OFFSET_LOOP);
    *decoder_reg = load;
    if (decoder_mode){
        *decoder_reg |= 0x100;
    }
    *loop_reg = loop_count;
}

void PWM::sequence (int seq_number, unsigned int *pointer, int count, int refresh, int enddelay){
    unsigned int * seq_pointer_reg = (unsigned int *)(base_address + REGISTER_OFFSET_SEQ_0_PTR + (MODIFIER_SEQ * seq_number));
    unsigned int * seq_count_reg = (unsigned int *)(base_address + REGISTER_OFFSET_SEQ_0_CNT + (MODIFIER_SEQ * seq_number));
    unsigned int * seq_refresh_reg = (unsigned int *)(base_address + REGISTER_OFFSET_SEQ_0_REFRESH + (MODIFIER_SEQ * seq_number));
    unsigned int * seq_enddelay_reg = (unsigned int *)(base_address + REGISTER_OFFSET_SEQ_0_ENDDELAY + (MODIFIER_SEQ * seq_number));
    *seq_pointer_reg = reinterpret_cast<std::uintptr_t>(pointer);
    *seq_count_reg = count;
    *seq_refresh_reg = refresh;
    *seq_enddelay_reg = enddelay;
}

void PWM::pinSelect (int channel, int port, int pin, bool disconnect){
    unsigned int * pin_select_reg = (unsigned int *)(base_address + REGISTER_OFFSET_PSEL_OUT_0 + (MODIFIER_PSEL_OUT * channel));
    *pin_select_reg = ((disconnect << 31) | (port << 5) | pin);
}

void PWM::enable (void){
    enable_pwm(true);
}

void PWM::disable (void){
    enable_pwm(false);
}

bool PWM::start (void){
    bool pwm_seq_started = start_pwm();
    return pwm_seq_started;
}

void PWM::stop (void){
    stop_pwm();
}

led.h

#ifndef rav_nrf52840_led_h
#define rav_nrf52840_led_h

#include "rav_nrf52840_macros.h"
#include "rav_nrf52840_pwm.h"

typedef enum {RED = 1,GREEN = 2,YELLOW = 3,BLUE = 4,MAGENTA = 5,CYAN = 6,WHITE = 7} ledState;

class LED {

private:

    PWM pwm;

    bool pwm_sequence_started_flag;

    bool LED_activeLow_flag;

    bool LED_RGB_flag;

    int  LED_portNumber[3];

    int  LED_pinNumber[3];

    int  LED_color;

    int  LED_intensity;

    unsigned int sequence_0[4];

public:

    LED (bool activeLow,int portNumber[3],int pinNumber[3]);  //  Use this format for RGB LEDs. Port and pin numbers must be listed in order: red, green, then blue.

    LED (bool activeLow,int portNumber,int pinNumber);  //  Use this format for single color LEDs

    void on (ledState color, int brightness);  //  Do not use this format with single color LEDs. Valid options for brightness are 0 - 100(%)

    void off (void);
};

#endif

led.cpp

#include "rav_nrf52840_led.h"

LED::LED (bool activeLow,int portNumber[3],int pinNumber[3]) : pwm(){
    LED_RGB_flag = true;
    LED_activeLow_flag = activeLow;
    LED_portNumber[0] = portNumber[0];
    LED_portNumber[1] = portNumber[1];
    LED_portNumber[2] = portNumber[2];
    LED_pinNumber[0] = pinNumber[0];
    LED_pinNumber[1] = pinNumber[1];
    LED_pinNumber[2] = pinNumber[2];
    pwm.init(INDIVIDUAL,0,0xFFFF);
    pwm.sequence(0,sequence_0,4,0,0);
    pwm.pinSelect(0,LED_portNumber[0],LED_pinNumber[0],false);
    pwm.pinSelect(1,LED_portNumber[1],LED_pinNumber[1],false);
    pwm.pinSelect(2,LED_portNumber[2],LED_pinNumber[2],false);
    pwm.enable();
}

LED::LED (bool activeLow,int portNumber,int pinNumber) : pwm(){
    LED_RGB_flag = false;
    LED_activeLow_flag = activeLow;
    LED_portNumber[0] = portNumber;
    LED_pinNumber[0] = pinNumber;
    pwm.init(INDIVIDUAL,0,0xFFFF);
    pwm.sequence(0,sequence_0,4,0,0);
    pwm.pinSelect(0,LED_portNumber[0],LED_pinNumber[0],false);
    pwm.enable();
}





// PRIVATE

int  LED_color = RED;  //  Default value for LED_color is RED.
int  LED_intensity = 0xFFFF;  //  Default value for LED_intensity is 0xFFFF.





// PUBLIC

void LED::on (ledState color, int brightness){
    LED_intensity = brightness;//(scale(brightness,0,100,0x8000,0xFFFF));
    if (pwm_sequence_started_flag){
        pwm.stop();
        pwm_sequence_started_flag = false;
    }
    if (LED_RGB_flag){
        LED_color = color;
        if (brightness >= 0xFFFF){//100){
            LED_activeLow_flag ? writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 0:1) : writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 1:0);
            LED_activeLow_flag ? writePin(LED_portNumber[1],LED_pinNumber[1],readBit(LED_color,1) ? 0:1) : writePin(LED_portNumber[1],LED_pinNumber[1],readBit(LED_color,1) ? 1:0);
            LED_activeLow_flag ? writePin(LED_portNumber[2],LED_pinNumber[2],readBit(LED_color,2) ? 0:1) : writePin(LED_portNumber[2],LED_pinNumber[2],readBit(LED_color,2) ? 1:0);
        }
        else{
            sequence_0[0] = LED_intensity;
            sequence_0[1] = LED_intensity;
            sequence_0[2] = LED_intensity;
            pwm_sequence_started_flag = pwm.start();
        }
    }
    else{
        if (brightness >= 100){
            LED_activeLow_flag ? writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 0:1) : writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 1:0);
        }
    }
}

void LED::off (void){
    if (LED_RGB_flag){
        LED_activeLow_flag ? writePin(LED_portNumber[1],LED_pinNumber[1],1) : writePin(LED_portNumber[1],LED_pinNumber[1],0);
        LED_activeLow_flag ? writePin(LED_portNumber[2],LED_pinNumber[2],1) : writePin(LED_portNumber[2],LED_pinNumber[2],0);
    }
    LED_activeLow_flag ? writePin(LED_portNumber[0],LED_pinNumber[0],1) : writePin(LED_portNumber[0],LED_pinNumber[0],0);
}

main.cpp

#include "rav_nrf52840_base.h"
#include "rav_nrf52840_led.h"


int main(void){  //  TX

    setupClock (HF_64MHz_XTAL, START);
    setupClock (LF_32_768kHz_XTAL, START);

    setupPin (0, 3,INPUT);
    pullPin  (0, 3,PULLUP);
    setupPin (0,18,INPUT);  //  External (? Ohm) pullup resistor.
    setupPin (0,22,OUTPUT);
    writePin (0,22,HIGH);
    setupPin (0,23,OUTPUT);
    writePin (0,23,HIGH);
    setupPin (0,24,OUTPUT);
    writePin (0,24,HIGH);

    int my_led_ports[3] = {0,0,0};
    int my_led_pins[3] = {23,22,24};  //  RED, GREEN, BLUE
    LED led(true,my_led_ports,my_led_pins);


    for(;;){
        if (readPin(0,3)){
            ;
        }
        else{
            led.on(RED,0xFFFF);
        }
        if (readPin(0,18)){
            ;
        }
        else{
            led.on(RED,0xF000);
        }
    }
    return -1;
}

For comparison, here is an old version of the code that works (without a PWM member of LED). The files pwm.h and pwm.cpp are the same as in the new version.

led.h -- Old (Working) Version

#ifndef rav_nrf52840_led_h
#define rav_nrf52840_led_h

#include "rav_nrf52840_macros.h"

typedef enum {RED = 1,GREEN = 2,YELLOW = 3,BLUE = 4,MAGENTA = 5,CYAN = 6,WHITE = 7} ledState;

class LED {

private:

    bool LED_activeLow_flag;

    bool LED_RGB_flag;

    int  LED_portNumber[3];

    int  LED_pinNumber[3];

    int  LED_color;

    int  LED_intensity;

public:

    LED (bool activeLow,int portNumber[3],int pinNumber[3]);  //  Use this format for RGB LEDs. Port and pin numbers must be listed in order: red, green, then blue.

    LED (bool activeLow,int portNumber,int pinNumber);  //  Use this format for single color LEDs

    void on (ledState color, int brightness);  //  Do not use this format with single color LEDs. Valid options for brightness are 0 - 100(%)

    void off (void);
};

#endif

led.cpp -- Old (Working) Version

#include "rav_nrf52840_led.h"

LED::LED (bool activeLow,int portNumber[3],int pinNumber[3]){
    LED_RGB_flag = true;
    LED_activeLow_flag = activeLow;
    LED_portNumber[0] = portNumber[0];
    LED_portNumber[1] = portNumber[1];
    LED_portNumber[2] = portNumber[2];
    LED_pinNumber[0] = pinNumber[0];
    LED_pinNumber[1] = pinNumber[1];
    LED_pinNumber[2] = pinNumber[2];
}

LED::LED (bool activeLow,int portNumber,int pinNumber){
    LED_RGB_flag = false;
    LED_activeLow_flag = activeLow;
    LED_portNumber[0] = portNumber;
    LED_pinNumber[0] = pinNumber;
}





// PRIVATE

bool LED_activeLow_flag;
bool LED_RGB_flag;
int  LED_portNumber[3];
int  LED_pinNumber[3];
int  LED_color = RED;  //  Default value is RED.
int  LED_intensity = 0xFFFF;  //  Default value is 0xFFFF.





// PUBLIC

void LED::on (ledState color, int brightness){
    LED_intensity = brightness;//(scale(brightness,0,100,0x8000,0xFFFF));
    if (LED_RGB_flag){
        LED_color = color;
        if (brightness >= 0xFFFF){//100){
            LED_activeLow_flag ? writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 0:1) : writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 1:0);
            LED_activeLow_flag ? writePin(LED_portNumber[1],LED_pinNumber[1],readBit(LED_color,1) ? 0:1) : writePin(LED_portNumber[1],LED_pinNumber[1],readBit(LED_color,1) ? 1:0);
            LED_activeLow_flag ? writePin(LED_portNumber[2],LED_pinNumber[2],readBit(LED_color,2) ? 0:1) : writePin(LED_portNumber[2],LED_pinNumber[2],readBit(LED_color,2) ? 1:0);
        }
    }
    else{
        if (brightness >= 0xFFFF){//100){
            LED_activeLow_flag ? writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 0:1) : writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 1:0);
        }
    }
}

void LED::off (void){
    if (LED_RGB_flag){
        LED_activeLow_flag ? writePin(LED_portNumber[1],LED_pinNumber[1],1) : writePin(LED_portNumber[1],LED_pinNumber[1],0);
        LED_activeLow_flag ? writePin(LED_portNumber[2],LED_pinNumber[2],1) : writePin(LED_portNumber[2],LED_pinNumber[2],0);
    }
    LED_activeLow_flag ? writePin(LED_portNumber[0],LED_pinNumber[0],1) : writePin(LED_portNumber[0],LED_pinNumber[0],0);
}

main.cpp -- Old (Working) Version

#include "rav_nrf52840_base.h"
#include "rav_nrf52840_led.h"
#include "rav_nrf52840_pwm.h"


int main(void){  //  TX

    setupClock (HF_64MHz_XTAL, START);
    setupClock (LF_32_768kHz_XTAL, START);

    setupPin (0, 3,INPUT);
    pullPin  (0, 3,PULLUP);
    setupPin (0,18,INPUT);  //  External (? Ohm) pullup resistor.
    setupPin (0,22,OUTPUT);
    writePin (0,22,HIGH);
    setupPin (0,23,OUTPUT);
    writePin (0,23,HIGH);
    setupPin (0,24,OUTPUT);
    writePin (0,24,HIGH);

    int my_led_ports[3] = {0,0,0};
    int my_led_pins[3] = {23,22,24};  //  RED, GREEN, BLUE
    LED led(true,my_led_ports,my_led_pins);

    bool pwm_sequence_started_flag = 0;
    int  LED_portNumber[3] = {0,0,0};
    int  LED_pinNumber[3] = {23,22,24};
    unsigned int sequence_0[4];
    PWM pwm(PWM0,true,0,0x7FFF,DIV4);
    pwm.init(INDIVIDUAL,0,0xFFFF);
    pwm.sequence(0,sequence_0,4,0,0);
    pwm.pinSelect(0,LED_portNumber[0],LED_pinNumber[0],false);
//  pwm.pinSelect(1,LED_portNumber[1],LED_pinNumber[1],false);
//  pwm.pinSelect(2,LED_portNumber[2],LED_pinNumber[2],false);
    pwm.enable();


    for(;;){
        if (readPin(0,3)){
            ;
        }
        else{
            if (pwm_sequence_started_flag){
                pwm.stop();
                pwm_sequence_started_flag = false;
            }
            led.on(RED,0xFFFF);
        }
        if (readPin(0,18)){
            ;
        }
        else{
            led.off();
            if (pwm_sequence_started_flag){
                pwm.stop();
                pwm_sequence_started_flag = false;
            }
            int LED_intensity = 0xF000;
            sequence_0[0] = LED_intensity;
            sequence_0[1] = LED_intensity;
            sequence_0[2] = LED_intensity;
            pwm_sequence_started_flag = pwm.start();
        }
    }
    return -1;
}

Since PWM is member of LED, it dissappears only when the LED dissappears, not when it's constructor is done. — Michael Chourdakis, Jul 06 '19 at 23:01
That's what I originally thought. I must have misunderstood ohlemacher. What then, is the error in my code above? As I understood it, the `PWM` object should be created when the instantiation list for the `LED` object runs. I can verify that the `pwm.enable()` function inside the `LED` constructor does not run, or its effects aren't seen. — KC Engel, Jul 06 '19 at 23:42
@KCEngel How have you verified that `pwm.enable()` does not run? That is, what are your symptoms? What are your actual results, compared to your expected results? What sort of [debugging](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) have you done? — JaMiT, Jul 07 '19 at 02:41
@JaMiT It's more of an educated guess. I'm using an NRF52840 USB dongle which does not have an on-board debugger. From their forum: [The short answer is: It is not possible to debug on the nRF52840 dongle.](https://devzone.nordicsemi.com/f/nordic-q-a/40701/debugging-on-nrf52840-dongle) I should have bought the dev kit. — KC Engel, Jul 07 '19 at 05:52
@JaMiT I've observed that none of the events related to the PWM module are occuring when I trigger a corresponding task. Specifically, the pwm.stop and pwm.start functions use an empty while loop to wait for specific events that signal when a task is complete, and the loop shortcut automatically initiates a task when the `loop complete` event occurs. None of these events are occurring when the appropriate tasks are triggered. The data sheet warns: "Note that the peripheral must be enabled before tasks and events can be used." This leads me to believe that the pwm.enable() is not running. — KC Engel, Jul 07 '19 at 05:52
To be clear, if I don't comment out the while loops in the `pwm.start` and `pwm.stop` functions, they become infinite loops and the program freezes. The loop shortcut fails to function as the required event never occurs, resulting in the pwm pattern running once per push of the button, instead of looping indefinitely. If you're not familiar with the nRF52840, it uses built in tasks and events as a sort of interrupt that runs independently of the microcontroller. — KC Engel, Jul 07 '19 at 06:05
@KCEngel More accurate: it's not possible to debug *with a debugger*. Unfortunately, we might be left with trial-and-error. Presumably something is wrong with your logic. Let's try to get something that works, then push that to what you want. Maybe start by making your `LED` class handle only one type of LED (presumably RGB, since that's your current test case), and remove the `PWM` object from the class, moving it to its own object in `main`. Also initialize `pwm_sequence_started_flag`. Hmm... anything else you can think of to simplify the test. Simple, no frills, and working is the goal. — JaMiT, Jul 07 '19 at 22:25
@JaMiT I have an old version that works. The `PWM` object is not a member of the `LED` object, so the user has to include the files for and declare both objects in main.cpp. I'll edit my post to include this code. — KC Engel, Jul 08 '19 at 01:12

score 1 · Accepted Answer · answered Jul 15 '19 at 03:09

There are several logical differences between the old and new code. Most likely, one of these is the culprit. Also, there is a bit of useless code you could get rid of. Finally, it seems you have misinterpreted ohlemacher's answer to that other question.

The most significant logical difference I see between the old and new versions is that the new version default-constructs the PWM object, while the old version used a parameterized constructor. You can see if this is a reason for your problem by taking the old, working version and changing the line PWM pwm(PWM0,true,0,0x7FFF,DIV4); to PWM pwm{};. You can fix the new version by changing both LED constructors to specify parameters for the contained PWM object; change : pwm(){ to : pwm(PWM0,true,0,0x7FFF,DIV4){.

Another logical difference is that the old version called led.off(), when the new version calls led.on(RED,0xF000);. Reconciling the logic here is not as simple as changing the call to off(), since the LED::off() function was not updated. To check this difference, comment out all code in the old LED::off() function. Fixing the new version is a bit more involved. I'll skip how to do this for now, since I suspect the choice of PWM constructors is the true culprit.

The other logical differences seem unlikely to cause your symptoms, but I'll list them anyway. The old version does not call PWM::pinSelect for channels 1 and 2, while the new version does. The old version initializes pwm_sequence_started_flag to 0 (which implicitly converts to false), while the new version has no explicit initialization for this flag.

In both the old and new versions of led.cpp, there is a part labeled "// PRIVATE" that accomplishes nothing. This section defines some (global) variables that have the same names as members of the LED class. Your LED member functions do not refer to these variables; when LED::on() sets LED_intensity, it sets the member this->LED_intensity, not the global ::LED_intensity. Also, your other source files are unlikely to refer to these global variables since they are not declared in led.h. Drop these unused declarations.

While not strictly useless, you could also get rid of some uses of void. In C++ (as opposed to C) it is acceptable to use bool start_pwm (); instead of bool start_pwm (void); to declare a function that takes no parameters. Skipping "void" means a smidgen less brainpower devoted to reading the code, so a smidgen more brainpower for understanding the code. Smidgens can add up.

The use of typedef to give a name to an enumeration also looks like a C-ism that was brought into C++. Try using enum pwmModule {PWM0,PWM1,PWM2,PWM3}; instead of typedef enum {PWM0,PWM1,PWM2,PWM3} pwmModule;. Again, a smidgen less code to read.

On the subject of simplifying code, the line

LED_activeLow_flag ? writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 0:1) : writePin(LED_portNumber[0],LED_pinNumber[0],readBit(LED_color,0) ? 1:0);

is a rough read. It would be easier to see what is going on if you separated the calculation of the last parameter from the call to writePin(). Logically equivalent (assuming the parameter is an int):

int foo = readBit(LED_color,0) ? (LED_activeLow_flag ? 0 : 1) : (LED_activeLow_flag ? 1 : 0);
writePin(LED_portNumber[0], LED_pinNumber[0], foo);

This makes it clear that regardless of conditions, writePin() will be called, and that the first two parameters are also independent of conditions. It's only the final parameter that can vary.

Let's move on to ohlemacher's answer. The claim "It just doesn't do what you want" refers to calling the constructor for a class within the constructor body. That is, given class A, writing something like A::A(bool) { A(); }. Create an object within the constructor body, which naturally will be destroyed by the time the constructor returns.

Furthermore, the question that ohlemacher answered was about calling the constructor for the same class as the constructor's (a.k.a. the "delegating constructor" introduced in C++11). The situation you are concerned with is calling the constructor for a data member. For a data member, member initializer lists have been available since the language was standardized.

Good catches. Spot on. Unfortunately, they don't seem to be the source of my problem. I get the same results regardless of which constructor I use. I removed the globals, fixed the C-isms, and added that flag to the initializer list, but no joy. — KC Engel, Jul 17 '19 at 02:26
I spoke too soon. Adding the `pwm_sequence_started_flag` to the initializer list was the last thing I tried, and I didn't think that would be it, but it was! Thanks for all your help! — KC Engel, Jul 17 '19 at 02:53

score 0 · Answer 2 · answered Jul 06 '19 at 23:05

0

The easiest method is to use smart pointers. Have the LED object contain a std::shared_ptr<PWM> object, which can be created whenever you want with std::make_shared, either at the constructor or at a member function. That way, you have both auto-cleanup of the object when LED dies, and you can copy the PWM object somewhere else so it persists when LED is destructed.

That way you can have any class contain another class on demand, and still be able to cleanup easily.

answered Jul 06 '19 at 23:05

Michael Chourdakis

10,345
3
42
78

Why is it easier to introduce dynamic allocation than to simply have the `PWM` object be a field directly in each `LED` object? – JaMiT Jul 07 '19 at 22:27

When creating an objectA inside another objectB, is objectA local to objectB and will objectA exist beyond objectB's instantiation?

2 Answers2