How to use AceButton object as a struct member

[bcelary]

I am using an Arduino Mega to build a Dimmer module.

It has 16 buttons that I would like to group together with 16 dimmers. I thought about using a struct for a dimmer-button pair. I am also using a third item to glue this together - a home assistant class for managing the MQTT states and communicating with the MQTT server so that the dimmers can be easily controlled via the MQTT server.

I have created the following struct:

struct Dimmer {
    HALight ha_light;
    dimmerLamp dimmer;
    ace_button::AceButton button;

    Dimmer(const char* haUniqueId, int dimmerPin, uint8_t buttonPin) :
        ha_light(haUniqueId, HALight::BrightnessFeature), dimmer(dimmerPin), button(buttonPin)
        { }
};

Dimmer dimmers_array[] = {
    Dimmer("room_1", 22, 54),
    Dimmer("room_2", 23, 55),
    Dimmer("room_3", 24, 56)
};

It worked well when I tested it without Buttons but when I added AceButton to the Dimmer class, I am getting this error:

(Dimmer)dynamic-init: <constructor-call>
"Dimmer::Dimmer(const Dimmer &)" (declared implicitly), required for copy that was eliminated, cannot be referenced -- it is a deleted functionC/C++(1815)

My very limited knowledge of C++ tells me that it is likely because AceButton has a deleted copy constructor which prevented creating one automatically for Dimmer class. I did notice that AceButton has deleted copy constructor but I don't see any notes as to why in the code.

The reason I am creating this array of Dimmer structs is because it make it much easier to create logic in all the callbacks. I can simply find the right Dimmer with all the mapped modules and there is just a single array I keep the mappings defined.

Any suggestions on how to handle AceButton differently to make it work this way?

Thanks!

[bcelary]

I made some modifications (with the help of chatGPT).

Decided to stick with the pointers in that wrapper struct, so it looks like this now:

struct Dimmer {
    HALight* ha_light;
    dimmerLamp* dimmer;
    ace_button::AceButton* button;

    Dimmer(HALight& _hl , dimmerLamp& _dl, ace_button::AceButton& _ab) :
        ha_light(&_hl), dimmer(&_dl), button(&_ab)
        { }
};

And I am initializing all the objects before I use them in the array of Dimmers:

ace_button::AceButton b54(54);
ace_button::AceButton b55(55);

dimmerLamp d22(22);
dimmerLamp d23(23);

HALight room1("room1", HALight::BrightnessFeature);
HALight room2("room2", HALight::BrightnessFeature);

Dimmer dimmers_array[] = {
    Dimmer(room1, d22, b54),
    Dimmer(room2, d23, b55),
};

Might be a better way to do it as I guess I do not want to make any object copies at all.

[bxparks]

The source of the error is that the C++ compiler automatically creates a copy constructor for your Dimmer class. That copy constructor depends on the copy constructors of the internal member fields of Dimmer, which are the HALight, dimmerLamp and AceButton objects. But the copy constructor for AceButton doesn't exist, because I explicitly removed it, so the compiler spits out an error message.

The reason I deleted the copy constructor for AceButton is because it does not make semantic sense to allow a button object to be copied. There is one physical button bound to one instance of AceButton. Does it make sense to allow a copy of the AceButton object to be created? No, a copy may introduce bugs, because the internal parameters of the copy can be updated without modifying the original, and now we have 2 different sets of AceButton parameters associated with a single physical button. It becomes difficult, if not impossible, to predict the behavior of the AceButton code in that scenario.

The solution from ChatGPT is ok. But I think a simpler solution is to just disable the copy-constructor on your Dimmer class:

class Dimmer {
  ...
private:
  Dimmer(const Dimmer&) = delete;

If you have 16 buttons and 16 dimmers, and so on, this should allow your original code to work, and save you from typing 16 x 3 = 48 extra lines of code where you manually instantiate all those objects.

[bxparks]

By the way, I'm not sure how you plan to map an AceButton back to the Dimmer array element that it is associated with, but I added theid field and the getId() method to solve this exact problem. See examples/ArrayButtons.

[bcelary]

Yes, I know about the id and was going to use it 👌

Going back to that Dimmer array, I think a copy constructor is required during the initialization of the array elements for some reason as when it is private, it errors out on this.

struct Dimmer {
    HALight ha;
    dimmerLamp dimmer;
    ace_button::AceButton button;

    Dimmer(uint8_t id, const char* ha_unique_id, uint8_t dimmer_pin, uint8_t button_pin) :
        ha(ha_unique_id, HALight::BrightnessFeature), dimmer(dimmer_pin), button(button_pin, LOW, id)
        { }

    private:
    Dimmer(const Dimmer&) = delete;
    Dimmer& operator=(const Dimmer&) = delete;
};

Dimmer d1(0, "room1", 22, 54);

Dimmer dimmers_array[] = {
    Dimmer(1, "room2", 23, 55),
};

This results in error in the Dimmer array:

src/main.cpp:118:1: error: 'Dimmer::Dimmer(const Dimmer&)' is private within this context
 };
 ^
src/main.cpp:89:5: note: declared private here
     Dimmer(const Dimmer&) = delete;
     ^~~~~~
src/main.cpp:118:1: error: use of deleted function 'Dimmer::Dimmer(const Dimmer&)'
 };
 ^
src/main.cpp:89:5: note: declared here
     Dimmer(const Dimmer&) = delete;
     ^~~~~~

The d1 Dimmer is created without any issue. The error is for the dimmer_array.

Still this allows to simply initialize all the objects contained in the Dimmer class and use an array of pointers to stuff them in. Should be less lines and a little bit cleaner than what I initially wanted to do as a workaround.

[bxparks]

Ok, I'm starting to remember some of the problems with embedding AceButton inside a struct like Dimmer. Object initialization is some of the more annoyingly complex and confusing parts of the C++ language, which unfortunately went through a bunch of changes for C++11, C++17, and C++20, so I can never remember anything anymore.

If AceButton, HALight and DimmerLamp were "aggregates" (i.e. something that resembles a simple data struct), then you could use C++ aggregate initialization to initialize your Dimmer array. In other words, the following will not invoke the copy-constructor:

Dimmer dimmers_array[] = {
    {1, "room2", 23, 55},
    ....
};

Unfortunately, AceButton is not an aggregate because I made all its constructors explicit. The reason that I did that is because its constructors have default arguments which causes a constructor with a single argument to be valid. But that allows the compiler to perform automatic cast-conversion through the single-argument-constructor, which often leads to incredibly hard to find bugs caused by a simple typo. The explicit prevents automatic cast conversion.

So you can't use aggregate initialization (the nested { {}, {}, } initialization) for AceButton. And you can't use an explicit constructor initialization (your previous code), because AceButton does not have a copy-constructor (which prevents certain types of bugs).

I think the best you can do is to use a separate array of AceButton objects like this:

struct Dimmer {
    HALight ha;
    dimmerLamp dimmer;
    uint8_t button_pin;

    // constructor
    Dimmer(const char* name, uint8_t dimmer_pin, uint8_t bpin) : 
      ha(...), 
      dimmer(...),
      button_pin(bpin) 
   {}
};

Dimmer dimmers[COUNT] = {
   Dimmer("room1", 22, 54),
   Dimmer("room2", 23, 55),
   ...
};

AceButton buttons[COUNT]; // no constructors invoked

void setup() {
    for (uint8_t id = 0; id < COUNT; id++) {
      buttons.init(dimmers[id].button_pin, LOW, id);
    }
    ...
}

(Notice that you don't need to store the id in Dimmer because it will be automatically generated by the for loop).

Also, you can use the following idiom to avoid manually defining the COUNT:

Dimmer dimmers[] = {
   ...
};

#define COUNT = (sizeof(dimmers) / sizeof(dimmers[0]))

[bcelary]

great idea!

In the meantime I went with yet another way as follows. Seems to be also working (I have not tested the code yet).

I am a very casual user of C++ (especially when it comes to writing anything) and these initialization issues were always a bit tricky :) thanks for all the help you provided!

struct Dimmer {
    HALight ha;
    dimmerLamp dimmer;
    ace_button::AceButton button;

    Dimmer(const char* ha_unique_id, uint8_t dimmer_pin, uint8_t button_pin) :
        ha(ha_unique_id, HALight::BrightnessFeature), dimmer(dimmer_pin), button(button_pin)
        { }

    private:
    Dimmer(const Dimmer&) = delete;
    Dimmer& operator=(const Dimmer&) = delete;
};

Dimmer d0("room-1", 22, 54);
Dimmer d1("room-2", 23, 55);
// ...
Dimmer d15("room-x", 35, 67);

Dimmer *dimmers_array[] = {
    &d0, &d1, /* ... */ &d15,
};
constexpr int dimmers_count = sizeof(dimmers_array) / sizeof(dimmers_array[0]);

But tbh that code you proposed seems better. There's less editing when the mapping changes and the button id is also automatically initialized. Will change to that.

[bxparks]

Your version will work. But yeah, having all the configuration parameters in one place inside the curly-brace initialization block is easier to maintain.

[bxparks]

Side Note: I'm not sure when they added the ability to close Discussions, or maybe it was always there but I never noticed it, but I prefer to keep discussions open so that other people searching for similar problems can find the discussions easier. It's one of the advantages of Discussions over Issues, in my opinion for things like this, because the default search under Issues ignores closed Issues, and many people don't bother doing the extra search.

[runger1101001]

Perhaps something like this?

MyClass.h

#include <Arduino.h>
#include <AceButton.h>


class MyClass : public ace_button::IEventHandler {
protected:
  ace_button::AceButton myButton;
  void handleEvent(ace_button::AceButton* button, uint8_t eventType, uint8_t buttonState) override;
public:
  MyClass(uint8_t button_pin);
};

MyClass.cpp

#include "./MyClass.h"

using namespace ace_button;

MyClass::MyClass(uint8_t button_pin) : myButton(button_pin) {
  myButton.getButtonConfig()->setIEventHandler(this);
};

void MyClass::handleEvent(AceButton* button, uint8_t eventType, uint8_t buttonState) {
  // handle event here
}

[bxparks]

(Doing some spring cleaning of my inbox...)
@runger1101001: Other than some minor typos, your code should work. I normally try to avoid cyclic dependencies, but your MyClass allows encapsulation of AceButton so it could be worth it in this case. Anyway, this being C++, there are lots of different ways to "skin the cat" as they say.