[Question] How to pass object method as an event handler? about acebutton HOT 11 CLOSED

Ok, I have implemented an object-based EventHandler on the ieventhandler branch. Do you want to check it out?

There 2 new things:

• an IEventHandler interface, and
• ButtonConfig::setIEventHandler(IEventHandler*) method

See examples/SingleButtonUsingIEventHandler/ for sample code.

This increases the library code size by 34 bytes of flash on an Arduino Nano, but no additional RAM (because I was able to reuse a spare bit in an uint16_t flag field). I think 34 bytes is worth paying for this additional flexibility.

from acebutton.

I'm not sure exactly how your buttons differ from each other, but don't forget that even if you use the same ButtonConfig for a collection of different buttons, your event handler can be written to ignore the LongPressed and RepeatPressed events for some buttons and not for other buttons. About the only time that you must use separate ButtonConfig is when the timing parameters are different (e.g. some buttons have a 100ms delay, other buttons have a 200ms delay). Using multiple ButtonConfig instances is fine, but I find that many people get confused by them, so I recommend people to use a single ButtonConfig (in fact, just the System ButtonConfig) if they can get away with it.

With regards to reconfiguring the ButtonConfig at runtime, one of the overloadedinit() methods takes a ButtonConfig*, and there is also the setButtonConfig() method, so you should be able to do that. I have never needed to do this myself, but I don't see a reason why that shouldn't work. Just want to point out again that if you are able to use just a single ButtonConfig, then it might be possible to do the reconfiguring by remapping the getId() of the various buttons (using the init()).

With regards to overloading the setEventHandler(), I think this may be possible, let me play around with it. Instead of a EventHandler typedef, I would need to define an interface EventHandlerClass with a pure virtual function. Then instead of storing a function pointer, I would store a void* and an uint8_t status discriminator to figure out which callback method was being used. Then cast the void* to the right type, before calling the callback. The issue is that this will cause the code for EventHandlerClass to be pulled in, even if it wasn't being used, making the library bigger for 8-bit AVR environment. I'm going to guess about 50-100 bytes. Allowing the EventHandler to be an object is definitely cleaner OOP for more complex programs, so I think this might be worth it.

With regards to lambdas, I'm not sure if that will work, since the lambda will be created on the stack, and the EventHandler on the ButtonConfig has a lifetime longer than the code that will configure it.

from acebutton.

Thanks for reminding me about writing a test. It was on my mind, then I forgot. Will be in the upcoming merge.

BTW, one more technique that may help you. It is possible to create your FrontPanel and all of its wrapped AceButtons and ButtonConfigs on the heap. Then you can delete the whole thing and recreate it with different configurations. Just watch out for heap fragmentation if you do this too often. Oh, and also, the virtual destructor on the ButtonConfig is activated only on the ESP8266 and ESP32, because activating it on an AVR causes 600 bytes to be pulled in, which is too much.

from acebutton.

I'm not familiar with that board, how is it identified?
In the meantime, the virtual destructor is not critical, there's no memory leak. It's needed just to keep the compiler quiet.

from acebutton.

I inverted the #if logic around the virtual destructor:

-    #if defined(ESP8266) || defined(ESP32)
+    #if ! defined(ARDUINO_ARCH_AVR)

It is now enabled for all architectures except 8-bit AVR chips. Which means that your ARDUINO_ARCH_NRF52 will pick up the virtual destructor, and keep the compiler happy.

This is checked in to develop branch. I'll push a 1.6.1 release today or tomorrow.

from acebutton.

Done!

from acebutton.

Hi, Thanks for the interest and kind words. You raise an interesting design question, and the answer depends on the scale of the problem that you are trying to solve. The basic problem is that ButtonConfig::setEventHandler() takes a C-function, not an object. I can think of several ways around the problem.

But first some background comments:

1. AceButton does not use pure OOP, because I felt that OOP requires too much overhead and boilerplate code in a constrained microcontroller environment with only 2kB of RAM. Pure OOP would have made the EventHandler an interface, with a pure virtual function, instead of just a typedef for a function pointer. But I didn't want the end-user to suffer the cognitive overhead of dealing with yet another class hierarchy, along with an extra 2 or 4 bytes of memory for the v-table pointer, and the boilerplate code needed to define the subclass of the EventHandler and its overridden method to handle the event. For simple programs, I think it is too much abstraction, too much code.

2. I assumed that in most cases, multiple AceButton objects would be associated with a single ButtonConfig. FrontPanel assume a one-to-one association between AceButton and ButtonConfig (although you may have simplified the code when writing up this question). Notice that unlike most other button libraries, the event dispatcher lives on the ButtonConfig object, not the AceButton object. You can think of the ButtonConfig as the objectification of the AceButton class itself, as a workaround for the fact C++ classes are not first class objects.

   If you use only a single ButtonConfig in your app, don't forget that the library automatically provides a "System ButtonConfig" that is bound to all AceButton instances by default.

3. I'm not sure it is worth creating a facade object to manage a ButtonConfig and AceButton together, although this might make sense if you have lots of AceButton objects, and lots of different types of AceButton objects. Recall that the AceButton object must called in the global loop() method, so if you layer the FrontPanel on top of it, you have to either reach into the AceButton object, or provide a pass-through method like FrontPanel::check() method which delegates into the AceButton::check() method. That seems like a lot of overhead.

   Also note that when a button event is detected, the AceButton object dispatches through the EventHandler on the ButtonConfig, to the FrontPanel::handleShiftButtonEvent(). It seems awkward to have a contained child object sending messages to the containing parent object.

Taking these things into consideration, I recommend a looser coupling between your FrontPanel object and the AceButton object. The FrontPanel only cares about the events generated by the AceButton object. It's not clear to me that there is much to be gained from having the FrontPanel "own" the AceButton object. The one benefit I can see is better encapsulation, but you are kinda fighting the programming paradigm that I selected in for this library. I describe in the last solution below, how to mix the two, but I think that amount of overhead only makes sense if you have a very complex FrontPanel object, with large number of AceButton objects with independent sets of ButtonConfig objects.

Single AceButton If your FrontPanel actually needs only a single or (few AceButton objects), use a single EventHandler like this:

//----------------------------------------------------------------
// FrontPanel.h, FrontPanel.cpp
//----------------------------------------------------------------

class FrontPanel {
  public:
    void handleShiftButtonEvent(AceButton* /* button */, uint8_t eventType, uint8_t /* buttonState */) {
      switch (eventType) {
        case AceButton::kEventPressed:
          onShiftButtonPressed();
          break;
        case AceButton::kEventReleased:
          onShiftButtonReleased();
          break;
       }
     }
    void onShiftButtonPressed();
    void onShiftButtonReleased();
};

//----------------------------------------------------------------
// Program.ino
//----------------------------------------------------------------

// AceButton and FrontPanel are DECOUPLED
AceButton button(PIN);
FrontPanel panel;

// Binding between FrontPanel and AceButton happens here
void handleEvent(AceButton* button, uint8_t eventType, uint8_t buttonState) {
  panel.handleShiftButtonEvent(button, eventType, buttonState);
}

void setup() {
    ButtonConfig* config = ButtonConfig::getSystemButtonConfig();
    config->setEventHandler(eventHandler);
}

void loop() {
  button.check();
}

Multiple Buttons It seems likely that the FrontPanel has multiple buttons. Then we can embedded the discrimination logic into the FrontPanel::handleButtonEvent():

//----------------------------------------------------------------
// config.h
//----------------------------------------------------------------
#define BUTTON1_PIN 2
#define BUTTON2_PIN 3
#define BUTTON3_PIN 5

//----------------------------------------------------------------
// FrontPanel.h, FrontPanel.cpp
//----------------------------------------------------------------
#include <AceButton.h>
#include "configs.h"

class FrontPanel {
  public:
    void handleButtonEvent(AceButton* button, uint8_t eventType, uint8_t buttonState) {
       switch (button->getPin()) {
         case BUTTON1_PIN:
           handleButton1(eventType);
           break;
         case BUTTON2_PIN:
           handleButton2(eventType);
           break;
         case BUTTON3_PIN:
           handleButton3(eventType);
           break;
        }
    }
    void handleButton1(uint8_t eventType);
    void handleButton2(uint8_t eventType);
    void handleButton3(uint8_t eventType);

    void onShiftButtonPressed();
    void onShiftButtonReleased();
};

//----------------------------------------------------------------
// Program.ino
//----------------------------------------------------------------

#include <AceButton.h>
#include "config.h"
#include "FrontPanel.h"

// AceButtons and FrontPanel are DECOUPLED
AceButton button1(BUTTON1_PIN);
AceButton button2(BUTTON2_PIN);
AceButton button3(BUTTON3_PIN);
FrontPanel panel;

// Binding between FrontPanel and AceButton happens here
void handleEvent(AceButton* button, uint8_t eventType, uint8_t buttonState) {
  panel.handleButtonEvent(button, eventType, buttonState);
}

void setup() {
    ButtonConfig* config = ButtonConfig::getSystemButtonConfig();
    config->setEventHandler(eventHandler);
}

void loop() {
  button1.check();
  button2.check();
  button3.check();
}

Multiple FrontPanel Maybe you really want multiple instances of FrontPanel, where one AceButton is bound to one ButtonConfig, so that each button can be configured slightly differently from the others? You could use 3 different handleEvent() methods, but here's a clever solution that uses the AceButton::getId() field which was designed for problems like this. In fact, it's layer of indirection that's basically equivalent to making the EventHandler an interface, using the PANELS array as an array of pointers to EventHandler objects:

//----------------------------------------------------------------
// FrontPanel.h, FrontPanel.cpp
//----------------------------------------------------------------
#include <AceButton.h>

class FrontPanel {
  public:
    void handleEvent(AceButton* /* button */, uint8_t eventType, uint8_t /* buttonState */) {
      switch (eventType) {
        case AceButton::kEventPressed:
          onShiftButtonPressed();
          break;
        case AceButton::kEventReleased:
          onShiftButtonReleased();
          break;
       }
     }
    void onShiftButtonPressed();
    void onShiftButtonReleased();
};

//----------------------------------------------------------------
// Program.ino
//----------------------------------------------------------------

#include <AceButton.h>
#include "config.h"
#include "FrontPanel.h"

// AceButtons and FrontPanels are DECOUPLED
ButtonConfig config1;
ButtonConfig config2;
ButtonConfig config3;
AceButton button1(&config1);
AceButton button2(&config2);
AceButton button3(&config3);
FrontPanel panel1;
FrontPanel panel2;
FrontPanel panel3;

// A lookup table of FrontPanel based on the index of AceButton::getId()
FrontPanel* PANELS[] = { &panel1, &panel2, &panel3 };

// Bind the AceButton to its corresponding FrontPanel, using AceButton.getId()
void handleEvent(AceButton* button, uint8_t eventType, uint8_t buttonState) {
  FrontPanel* panel = PANELS[button->getId()];
  panel->handleEvent(button, eventType, buttonState);
}

void setup() {
  pinMode(BUTTON1_PIN, INPUT_PULLUP);
  pinMode(BUTTON1_PIN, INPUT_PULLUP);
  pinMode(BUTTON2_PIN, INPUT_PULLUP);

  button1.init(BUTTON1_PIN, HIGH, 0 /*id*/);
  button2.init(BUTTON2_PIN, HIGH, 1 /*id*/);
  button3.init(BUTTON3_PIN, HIGH, 2 /*id*/);

  buttonConfig1.setEventHandler(handleEvent);
  buttonConfig2.setEventHandler(handleEvent);
  buttonConfig3.setEventHandler(handleEvent);
}

void loop() {
  button1.check();
  button2.check();
  button3.check();
}

You Really Really Want a Facade Let's suppose you are still convinced that the FrontPanel should own and manage the AceButton and ButtonConfig objects for better encapsulation. The Multiple FrontPanel code above can be extended to do this as shown below:

//----------------------------------------------------------------
// FrontPanel.h, FrontPanel.cpp
//----------------------------------------------------------------
#include <AceButton.h>

class FrontPanel {
  public:
    void handleEvent(AceButton* /* button */, uint8_t eventType, uint8_t /* buttonState */) {
      switch (eventType) {
        case AceButton::kEventPressed:
          onShiftButtonPressed();
          break;
        case AceButton::kEventReleased:
          onShiftButtonReleased();
          break;
       }
     }
    void onShiftButtonPressed();
    void onShiftButtonReleased();

  // Public for illustration purposes.
  // Should be private in production with appropriate accessors.
  public: 
    AceButton button_;
    ButtonConfig buttonConfig_;
};

//----------------------------------------------------------------
// Program.ino
//----------------------------------------------------------------

#include <AceButton.h>
#include "config.h"
#include "FrontPanel.h"

FrontPanel panel1;
FrontPanel panel2;
FrontPanel panel3;

// A lookup table of FrontPanel based on the index of AceButton::getId()
FrontPanel* PANELS[] = { &panel1, &panel2, &panel3 };

// Bind the AceButton to its corresponding FrontPanel, using AceButton.getId()
// This is equivalent to putting this into the FrontPanel class, with a 'this' pointer to FrontPanel 
void handleEvent(AceButton* button, uint8_t eventType, uint8_t buttonState) {
  FrontPanel* panel = PANELS[button->getId()];
  panel->handleEvent(button, eventType, buttonState);
}

void setup() {
  pinMode(BUTTON1_PIN, INPUT_PULLUP);
  pinMode(BUTTON1_PIN, INPUT_PULLUP);
  pinMode(BUTTON2_PIN, INPUT_PULLUP);

  panel1.button_.init(panel1.buttonConfig_, BUTTON1_PIN, HIGH, 0 /*id*/);
  panel2.button_.init(panel2.buttonConfig_, BUTTON2_PIN, HIGH, 1 /*id*/);
  panel3.button_.init(panel3.buttonConfig_, BUTTON3_PIN, HIGH, 2 /*id*/);

  panel1.buttonConfig_.setEventHandler(handleEvent);
  panel2.buttonConfig_.setEventHandler(handleEvent);
  panel3.buttonConfig_.setEventHandler(handleEvent);
}

void loop() {
  panel1.button_.check();
  panel2.button_.check();
  panel3.button_.check();
}

NOTE: None of above code has been tested. There will most likely be typos and errors. I hope I have given enough options though.

from acebutton.

@bxparks huge thanks for a detailed answer! You've covered couple of my use cases and gave some new ideas about using your library. Thanks for your attention to my issue.

It's sad that event handler must be a plain function outside of FrontPanel class. AFAIK, it is possible to assign a static method as an event handler but this method must be public anyway (and this decision breaks encapsulation). Also lambda function may help in this case but I don’t know are they available in Arduino and how to create typed lambdas (my poor knowledge of C++).

I understand your point about library design, it really worth it. My MCU is 64MHz/256KB and memory consumption is not the case so I trade performance for better application design. The device I'm creating has an array of buttons with assignable functions + switchable left/right hand layout. Some buttons must support long press and click, but some of them - not (and are using "pressed" event only). So they have different configuration (and this configuration can be changed on-the-fly). I like your idea of detecting button by ID or pin number, it will help me with mapping of hardware buttons to features.

I have couple of short questions for you. Would be happy if you will find some time to respond. Thanks in advance.

Is it possible to reassign another ButtonConfig instance to a Button after init? This will allow me to reconfigure buttons on-the-fly after layout change.

Can we overload ButtonConfig::setEventHandler() method so it will accept some kind of typed lambda instead of a plain static function?

Cheers,
Denis.

from acebutton.

@bxparks everything looks great! I'm happy to use the IEventHandler in my project and to post back results. Do I need to make some tests before you release a new version?

from acebutton.

@bxparks thanks for the suggestion about a heap allocation, will check this tomorrow.

I'm using Arduino Nano BLE 33, it has enough Flash/RAM so maybe you'll want to add a virtual destructor for that CPU also.

from acebutton.

@bxparks it seems that you may check ARDUINO_ARCH_NRF52:

#if defined(ARDUINO_ARCH_NRF52)

from acebutton.