Creating Custom Button Behaviour. Is This Wrong?

Using Arduino Programming Questions
1

I've only recently stumbled across function pointers and only partially understand them and their uses at this stage.

In trying to learn more, I wrote a super simple button class whereby on calling the execute() function of each button object, a piece of custom code is executed. A different behaviour for each instance of the button class is passed in through the constructor and executed.

Is this the correct way to be doing something like this? I mean, it works... So I guess if the shoe fits and that, but I was curious if there were any reasons for NOT doing things this way?

The only other way I can think of structuring this would be to have X number of classes inheriting from Button that all implement their own version of execute(); but if you had for example 50 buttons that seems a little excessive to have 50 different classes all only instantiated once.

P.S Excuse the C++ rather than Arduino code. You can run/test this here: GDB online Debugger | Code, Compile, Run, Debug online C, C++

#include<iostream>
#include<string>

class Button
{
     public:
     typedef void (*FuncPtr)();
     FuncPtr execute;
     Button(FuncPtr execute):execute(execute){}
};

void CustomCode1(){ std::cout << "Custom Code 1 Worked!" << '\n'; }
void CustomCode2(){ std::cout << "Custom Code 2 Worked!" << '\n'; }
void CustomCode3(){ std::cout << "Custom Code 3 Worked!" << '\n'; }
void CustomCode4(){ std::cout << "Custom Code 4 Worked!" << '\n'; }

Button buttons[4] = 
{
     {Button(&CustomCode1)},
     {Button(&CustomCode2)},
     {Button(&CustomCode3)},
     {Button(&CustomCode4)}
};

int main()
{
    for (int i=0; i<4; i++)
    {
         buttons[i].execute();
    }
    return 0;
}

Output:

Custom Code 1 Worked!
Custom Code 2 Worked!
Custom Code 3 Worked!
Custom Code 4 Worked!


...Program finished with exit code 0
Press ENTER to exit console.
2

i think there are better ways to do this. in C++ Programming Style cargill show examples of poorly designed classes and how to do them better.

i think your use of a class to exercise function pointers is unnecessary

consider how a single function could simply be passed a button id argument and how unique function can be invoked thru an array of function pointers.

byte pinButs [] = { A1, A2, A3 };
#define N_PINS   3

byte butState [N_PINS];

// -----------------------------------------------------------------------------
void butFunc1 (void)  { Serial.println (__func__); }
void butFunc2 (void)  { Serial.println (__func__); }
void butFunc3 (void)  { Serial.println (__func__); }

void (*butFunc [])(void) = {
    butFunc1,
    butFunc2,
    butFunc3,
};

// -----------------------------------------------------------------------------
void
genericButFunc (
    int  n )
{
    Serial.print   ("genericButFunc: ");
    Serial.println (n);
}

// -----------------------------------------------------------------------------
void loop ()
{
    for (unsigned n = 0; n < N_PINS; n++)  {
        byte but =  digitalRead (pinButs [n]);

        if (butState [n] != but)  {
            butState [n] = but;
            delay (10);
            if (LOW == but)  {
                genericButFunc (n);
                (*butFunc[n])();
            }
        }
    }
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < N_PINS; n++)  {
        pinMode (pinButs [n], INPUT_PULLUP);
        butState [n] = digitalRead (pinButs [n]);
    }
}

here's example code for a state machine that uses a table of function pointers

//
// example state machine
//

#include <stdio.h>

#include "stateMach.h"

// ------------------------------------------------
Status
a (void* a)
{
    printf ("%s:\n", __func__);
    return OK;
}

// ------------------------------------------------
Status
b (void* a)
{
    printf ("%s:\n", __func__);
    return OK;
}

// ------------------------------------------------
Status
c (void* a)
{
    printf ("%s:\n", __func__);
    return OK;
}

// ------------------------------------------------
Status
__ (void* a)
{
    printf ("%s:\n", __func__);
    return OK;
}

// --------------------------------------------------------------------

#define N_STATE     3
#define N_STIM      2

typedef enum { S0, S1, S2 } State_t;

typedef Status(*Action_t)(void*) ;

State_t smTransitionTbl [N_STATE] [N_STIM] = {
    {   S1, S2, },
    {   S0, S2, },
    {   S2, S1, },
};


Action_t smActionTbl [N_STATE] [N_STIM] = {
     {  a,  b },
     {  c, __ },
     { __,  a },
};

// ------------------------------------------------
char *msg1 [] = {
    "State machine has 3 states and 2 stimuli.",
    "It has the following state transistion and action routine tables.",
    "A __() represents do nothing and is easily identified in table.",
    "Each row is indexed by a state and each column a stimulus.",
    "Of course, meaningful action routine names are helpful.",
};

char *msg2 [] = {
    "Enter valid stimuli [0-1]:"
};

void
smHelp (void)
{
    for (int i = 0; i < sizeof(msg1)/sizeof(char*); i++)
        printf ("  %s\n", msg1 [i]);

    for (int state = 0; state < N_STATE; state++)  {
        printf ("%8s", "");
        for (int stim = 0; stim < N_STIM; stim++)
            printf (" %d", smTransitionTbl [state][stim]);

        printf ("%8s", "");
        for (int stim = 0; stim < N_STIM; stim++)  {
            if (a == smActionTbl [state][stim])
                printf ("  a()");
            else if (b == smActionTbl [state][stim])
                printf ("  b()");
            else if (c == smActionTbl [state][stim])
                printf ("  c()");
            else
                printf (" __()");
        }

        printf ("\n");
    }

    for (int i = 0; i < sizeof(msg2)/sizeof(char*); i++)
        printf ("  %s\n", msg2 [i]);

}

// ------------------------------------------------
static State_t _smState  = S0;

Status
smEngine (Stim_t stim)
{
    Action_t   func;
    State_t    last = _smState;

    if (N_STIM <= stim)
        return ERROR;

    func        = smActionTbl [_smState] [stim];
    _smState    = smTransitionTbl [_smState] [stim];

    printf ("    stim %d, transition from state %d to %d, exec ",
        stim, last, _smState);

    return (*func) (NULL);
}
3

In your button example, what's the advantage of having the array of function pointers outside of the button class?

If each button say 0-49 corresponds to one function inside the array of function pointers (also 0-49).... So buttons[35] goes with funcPtrs[35] etc. Then to my uninitiated mind it seems more logical to have the function pointer grouped in with each button object rather than referenced in an external array.

I'm in no way arguing that your code isn't better, just trying the understand the 'whats and whys' of this stuff so I can do things better/correctly.

Also, how are things like GUI menus/button set up then? Such as a File, Save, Open type of menu setup. Assuming that each button in a menu has to 'do something' do they all relate to a function pointer like in my/your code, or would each button/menu item inherit from a button class and override an execute function?

4

bear in mind i started working in 1985. C++ was just coming out in Bell Labs where i worked. i'm not a big fan of classes, as in everything is a class. but i'm a huge fan of object oriented design, which you can do in many languages and may not do correctly using classes.

why should all button functions be in the same class?

what if the functions triggered by different buttons are unrelated and in separate objects

if all the buttons more or less do the same things, then a single generic button function that takes a but-ID as an argument may make sense. that function could just translate the button ID to an ASCII char.

but more likely, if each button requires a unique type of processing, then i think it make much more sense to use an array of function pointers with an argument (e.g. void (*func) (int arg)), even if many of them are to a generic function, hence the arg.

i'll suggest that one reason a button class doesn't make sense is because the action for different buttons may be associated with different objects. (so now you're probably thinking have a register function in a button class with a callback for each button)

in "modular" programming (pre-dated OOD and just a different name for it), each module has a well defined purpose, maximizing "cohesion" and minimizing "coupling", dependencies on other modules (sound familiar)

if the private (i.e. static) and public variables and functions of an object are placed in a single separate files, then the various button functions are spread out.

of course there are many "right" ways to do this.

5

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.