Pointers... again

Status
Not open for further replies.
Thanks but I want to remove the operations of the callback outside of my button class. I think the button class should be responsible for simply calling something else. In this first case it will be to write a CANbus message but later on it may be to do something totally different. Instead on having a button class with CANbus functionality and other unrelated functions it might be best to put all the other stuff outside this class.
 
Thanks but I want to remove the operations of the callback outside of my button class. I think the button class should be responsible for simply calling something else. In this first case it will be to write a CANbus message but later on it may be to do something totally different. Instead on having a button class with CANbus functionality and other unrelated functions it might be best to put all the other stuff outside this class.

Adding a callback mechanism to gfvalvo's code would be quite easy. You could then attach a callback from outside. I'm just wondering if the effort is worth it just to avoid a simple cast from void* to int* as is done in #12.
 
Adding a callback mechanism to gfvalvo's code would be quite easy. You could then attach a callback from outside. I'm just wondering if the effort is worth it just to avoid a simple cast from void* to int* as is done in #12.
I am happy with the simple 'cast to whatever pointer type I'm using' approach. Thanks for all the input though!
 
Here you go:

Code:
#include "OneButton.h"

using usrCallback_t = void (*)(uint8_t);

class NewButton : public OneButton
{
 public:
    NewButton(uint8_t p) : OneButton(p, true, true), pin(p)
    {
        OneButton::attachClick(callBack, this);
    }
    static void attachClick(usrCallback_t cb) { usrCallback = cb; }

 private:
    const uint8_t pin;

    static usrCallback_t usrCallback;
    static void callBack(void* p)
    {
        usrCallback(((NewButton*)p)->pin);
    }
};

usrCallback_t NewButton::usrCallback = nullptr;

//=================================================================

NewButton buttons[] = {0, 1, 2};
constexpr uint8_t numButtons = sizeof(buttons) / sizeof(buttons[0]);

void buttonCallback(uint8_t pin)
{
    Serial.printf("Called with button %u\n", pin);
}

void setup()
{
    NewButton::attachClick(buttonCallback);
}

void loop()
{
    for (uint8_t i = 0; i < numButtons; i++)
    {
        buttons[i].tick();
    }
}

or, if you need the possibility to attach dedicated callbacks to each button:

Code:
#include "OneButton.h"

using usrCallback_t = void (*)(uint8_t);

class NewButton : public OneButton
{
 public:
    NewButton(uint8_t p) : OneButton(p, true, true), pin(p)
    {
        OneButton::attachClick(callBack, this);
    }
    void attachClick(usrCallback_t cb) { usrCallback = cb; }

 private:
    const uint8_t pin;

    usrCallback_t usrCallback;
    static void callBack(void* p)
    {
        ((NewButton*)p)->usrCallback(((NewButton*)p)->pin);
    }
};

//usrCallback_t NewButton::usrCallback = nullptr;

//=================================================================

NewButton buttons[] = {0, 1, 2};
constexpr uint8_t numButtons = sizeof(buttons) / sizeof(buttons[0]);

void buttonCallback(uint8_t pin)
{
    Serial.printf("Called with button %u\n", pin);
}

void setup()
{
    for (int i = 0; i < numButtons; i++)
    {
        buttons[i].attachClick(buttonCallback);
    }
}

void loop()
{
    for (uint8_t i = 0; i < numButtons; i++)
    {
        buttons[i].tick();
    }
}

But again, quite some effort to avoid a cast :)
 
Last edited:
Since you are interested in options, here another one:

  • No need to subclass OneButton
  • No need for any changes to OneButton
  • Works with all the attachXXX functions out of the box
  • Clean code
  • Works nicely on an Uno :)
(I obviously like that one :) )

Code:
#include "OneButton.h"

OneButton button[]{{0, true}, {1, true}, {2, true}};
constexpr uint8_t nrOfButtons = sizeof(button) / sizeof(button[0]);

void myIntCallback(int pin)
{
    Serial.print("called with paramter");
    Serial.println(pin);
}

void setup()
{
    Serial.begin(115200);

    for (int i = 0; i < nrOfButtons; i++)
    {
        button[i].attachClick([](void* p) { myIntCallback((int)p); }, (void*)i);
    }
}

void loop()
{
    for (unsigned i = 0; i < nrOfButtons; i++)
    {
        button[i].tick();
    }
}
 
Last edited:
Since you are interested in options, here another one:

  • No need to subclass OneButton
  • No need for any changes to OneButton
  • Works with all the attachXXX functions out of the box
  • Clean code
  • Works nicely on an Uno :)
(I obviously like that one :) )

Code:
#include "OneButton.h"

OneButton button[]{{0, true}, {1, true}, {2, true}};
constexpr uint8_t nrOfButtons = sizeof(button) / sizeof(button[0]);

void myIntCallback(int pin)
{
    Serial.print("called with paramter");
    Serial.println(pin);
}

void setup()
{
    Serial.begin(115200);

    for (int i = 0; i < nrOfButtons; i++)
    {
        button[i].attachClick([](void* p) { myIntCallback((int)p); }, (void*)i);
    }
}

void loop()
{
    for (unsigned i = 0; i < nrOfButtons; i++)
    {
        button[i].tick();
    }
}

So this one is another Lambda function being passed to attachClick?
 
Yes, but this version doesn't have the capture issue and it compiles for an UNO as well. The callback extensions of gfvalvos code are in #30
 
Yes, but this version doesn't have the capture issue and it compiles for an UNO as well. The callback extensions of gfvalvos code are in #30

I have gone and hit a wall again.... these ^&%*&& pointers

The code in #31 was so elegant and compiled and worked perfectly on the ATMega328 as well as the Teensy 4.0. I then tried to extend it again to work with the MCP23008 port expander.

The Adafruit_MCP23008 library is probably the most widely used library for this expander and I had it working separately without issue.

What I have been trying to do from the onset is use the OneButton library to debounce the outputs from the MCP23008 IC.

I have a class, mcpButtons that extends the Adafruit_MCP23008 class. I create an array of OneButton objects (untouched from https://github.com/mathertel/OneButton) and initialise with only begin() as I don't care about hardware pins on the AtMega. Later on I call the OneButton::tick(bool level) method and pass it the output from the MCP23008.

My mcpButtons class compiled fine until I attempted to pass in the attachClick callback....again. I feel like I am just going around in circles. One minute I think I understand function pointers and Lambdas, the next minute I feel like an idiot again.

What I'm trying to achieve is to have 8 of these MCP23008 IC's on a custom PCB that has a plug in Teensy 4.0 - I need to pass in callback functions (may have different callbacks for different pins) for click, double click, hold. I am just trying to get the click callback working in the code below.

I honestly don't know what I'm doing anymore

Code:
#include <Wire.h>
#include "Adafruit_MCP23008.h"
#include "OneButton.h"

#define ACTIVELOW true

class mcpButtons : public Adafruit_MCP23008 {
  public:
    mcpButtons() : Adafruit_MCP23008() {
    }

    void begin(uint8_t ic2Addr) {
      _ic2Addr = ic2Addr;
      Adafruit_MCP23008::begin(_ic2Addr);

      // initialise all 8 GPIO on MCP23008
      for (int i = 0; i < 8; i++) {
        Adafruit_MCP23008::pinMode(i, INPUT);
        Adafruit_MCP23008::pullUp(i, HIGH);

        _buttons[i].setPressTicks(2000);
        _buttons[i].setClickTicks(200);
        _buttons[i].setDebounceTicks(20);  // 20ms for bounce

        
      }
    }

    void attachClick(uint8_t id, callback ???????????????) {  // <=========== PROBLEM IS HERE
      _buttons[id].attachClick([]((???????????????), void* p) { f((void*)p); }, (void*)id);  // <=========== and HERE
    }

    void attachDoubleClick() {

    }

    void attachHold() {

    }

    void tick() {
      _pins = Adafruit_MCP23008::readGPIO();  // read 8-bits from the MCP23008
      for (uint8_t i = 0; i < 8; i++) {
        if (ACTIVELOW) {
          _buttons[i].tick(~_pins & 0x1);  // invert if we are using ACTIVELOW and call the OneButton state machine
        } else {
          _buttons[i].tick(_pins & 0x1);
        }
        _pins >>= 1;
      }
    }

  private:
    OneButton _buttons[8];  //  array of 8 OneButton objects
    uint8_t _ic2Addr;
    uint8_t _pins;
  };

mcpButtons mcp_0;

void setup()
{
  Serial.begin(115200);

  mcp_0.begin(0x00);

  // callbacks for 2 pins as example
  mcp_0.attachClick(0, clickCallback);  // <=========== and HERE
  mcp_0.attachClick(1, clickCallback);  // <=========== and HERE
}

void loop()
{
  mcp_0.tick();
}

void clickCallback(int pin)  // <=========== and HERE
{
  Serial.print("Click pin: ");
  Serial.println(pin);
}
 
Last edited:
What you want to achieve is a bit tricky. First the usual solution using the given void(callback*)(void*) interface of the OneButton library. You simply pass it the address of your external callback function and your id (cast to void*) to the attachClick function. The attached callback function then simply casts the void* parameter back to int before using it:

Code:
#include "Adafruit_MCP23008.h"
#include "OneButton.h"
#include <Wire.h>

//#define ACTIVELOW true       this hurts my eyes :-)
constexpr bool ACTIVELOW = true;

void clickCallback(void* pinParam) // <=========== and HERE
{
    unsigned pin = (unsigned)pinParam;
    Serial.print("Click pin: ");
    Serial.println(pin);
}

using callback_t = void (*)(void*); // function pointer stuff gets much easier using type aliases (or typedefs)

class mcpButtons : public Adafruit_MCP23008
{
 public:
    mcpButtons() : Adafruit_MCP23008()
    {
    }

    void begin(uint8_t ic2Addr)
    {
        _ic2Addr = ic2Addr;
        Adafruit_MCP23008::begin(_ic2Addr);

        // initialise all 8 GPIO on MCP23008
        for (int i = 0; i < 8; i++)
        {
            Adafruit_MCP23008::pinMode(i, INPUT);
            Adafruit_MCP23008::pullUp(i, HIGH);

            _buttons[i].setPressTicks(2000);
            _buttons[i].setClickTicks(200);
            _buttons[i].setDebounceTicks(20); // 20ms for bounce
        }
    }

    void attachClick(unsigned id, callback_t cb)
    {
        _buttons[id].attachClick(cb, (void*)id);
    }

    void attachDoubleClick()
    {
    }

    void attachHold()
    {
    }

    void tick()
    {
        _pins = Adafruit_MCP23008::readGPIO(); // read 8-bits from the MCP23008
        for (uint8_t i = 0; i < 8; i++)
        {
            if (ACTIVELOW)
            {
                _buttons[i].tick(~_pins & 0x1); // invert if we are using ACTIVELOW and call the OneButton state machine
            } else
            {
                _buttons[i].tick(_pins & 0x1);
            }
            _pins >>= 1;
        }
    }

 private:
    OneButton _buttons[8];   //  array of 8 OneButton objects
    uint8_t _ic2Addr;
    uint8_t _pins;
};

mcpButtons mcp_0;

void setup()
{
    Serial.begin(115200);

    mcp_0.begin(0x00);

    // callbacks for 2 pins as example
    mcp_0.attachClick(0, clickCallback); // <=========== and HERE
    mcp_0.attachClick(1, clickCallback); // <=========== and HERE
}

void loop()
{
    mcp_0.tick();
}
This compiles and should work (can't test it because I don't have your hardware)

If I understand you correctly you would prefer if the external callback would get an int instead of the void* parameter. This would be easy to achieve if the OneButton library would provide a std::function interface instead of the void(callback*)(void*) interface. (Changing that would be easy as shown a few posts above but you'd loose compatibility to your legacy boards.)

You can however pass the call through a helper which will do the cast and invoke the callback with the integer. I would't do such stuff without real need (e.g. need for a super simple interface) but for the records:

Code:
#include "Adafruit_MCP23008.h"
#include "OneButton.h"
#include <Wire.h>

//#define ACTIVELOW true       this hurts my eyes :-)
constexpr bool ACTIVELOW = true;

void clickCallback(int pin) // <=========== and HERE
{
    Serial.print("Click pin: ");
    Serial.println(pin);
}

using callback_t = void (*)(int); // function pointer stuff gets much easier using type aliases (or typedefs)

class mcpButtons : public Adafruit_MCP23008
{
 public:
    mcpButtons() : Adafruit_MCP23008()
    {
    }

    void begin(uint8_t ic2Addr)
    {
        _ic2Addr = ic2Addr;
        Adafruit_MCP23008::begin(_ic2Addr);

        // initialise all 8 GPIO on MCP23008
        for (int i = 0; i < 8; i++)
        {
            Adafruit_MCP23008::pinMode(i, INPUT);
            Adafruit_MCP23008::pullUp(i, HIGH);

            _buttons[i].setPressTicks(2000);
            _buttons[i].setClickTicks(200);
            _buttons[i].setDebounceTicks(20); // 20ms for bounce
        }
    }

    // use this for version b

    // static void helper(void* p)
    // {
    //     unsigned pin = (unsigned)p;
    //     (callbacks[pin])(pin);
    // }

    void attachClick(unsigned id, callback_t cb)
    {
        callbacks[id] = cb; // you need to store the callback function somewhere

        // Version a) using a lambda to do everything inline
        _buttons[id].attachClick([](void* p) // this attaches a relay function which does the cast and calling your callback with an int.
        {
            unsigned pin = (unsigned)p;
            (callbacks[pin])(pin);
        },(void*)id);

        // Version b) doing exactly the same using a static helper function
        // _buttons[id].attachClick(helper, (void*)id);
    }

    void attachDoubleClick()
    {
    }

    void attachHold()
    {
    }

    void tick()
    {
        _pins = Adafruit_MCP23008::readGPIO(); // read 8-bits from the MCP23008
        for (uint8_t i = 0; i < 8; i++)
        {
            if (ACTIVELOW)
            {
                _buttons[i].tick(~_pins & 0x1); // invert if we are using ACTIVELOW and call the OneButton state machine
            } else
            {
                _buttons[i].tick(_pins & 0x1);
            }
            _pins >>= 1;
        }
    }

 private:
    OneButton _buttons[8];          //  array of 8 OneButton objects
    static callback_t callbacks[8]; // need to store the callbacks
    uint8_t _ic2Addr;
    uint8_t _pins;
};

callback_t mcpButtons::callbacks[8]; // need to define static members

mcpButtons mcp_0;

void setup()
{
    Serial.begin(115200);

    mcp_0.begin(0x00);

    // callbacks for 2 pins as example
    mcp_0.attachClick(0, clickCallback); // <=========== and HERE
    mcp_0.attachClick(1, clickCallback); // <=========== and HERE
}

void loop()
{
    mcp_0.tick();
}
 
Status
Not open for further replies.
Back
Top