Detecting the duration of a button press in a microcontroller project

Question

I'm a C++ newbie and so far have written my code mostly in an not OO way. Since this is getting confusing I'd like to start creating libraries for some of the functionalities that I've implemented. The following program basically issues callback functions depending on a counter. It's made to detect the duration of a button press in a microcontroller project and issues certain functions accordingly.

What I'd like to achieve is that I can define an array or list of key values somewhere in the program and call the stateEval function from within main().

const action action_table[] =
    {
        { 4, state1 },
        { 8, state4 },
        { 11, state8 },
    };

So far I've tried to wrap the stateEval function into a class but then I simply don't know how to pass the action_table data into the class. The code below is the current working implementation without my desperate try to refactor it. I can't use STL libraries due to the uC environment that I am using. cout would be replaced.

#include <iostream>

struct action
{
    int counter_limit;
    void (*transition)(void);
};

void state1() {
    std::cout << __func__;
}
void state4() {
    std::cout << __func__;
}
void state8() {
    std::cout << __func__;
}

const action action_table[] =
{
    { 4, state1 },
    { 8, state4 },
    { 11, state8 },
};

int STATE =0;

void stateEval(int counter)
{
    size_t size = sizeof(action_table) / sizeof(action_table[0]);
    for (size_t i=0; i < size; i++)
    {
        const action &a = action_table[i];
        if (counter < a.counter_limit)
        {
            a.transition();
            return;
        }
    }
}

int main()
{
  for (int counter = 0 ; counter < 11; ++counter)
        stateEval(counter);

    return 0;
}

This is how I would like to use my class:

const action action_table[] =
{
    { 4, state1 },
    { 8, state4 },
    { 11, state8 },
};

int main()
{
  State_Caller sc;
  sc.attach(action_table);

  for (int counter = 0 ; counter < 11; ++counter)
        sc.stateEval(counter);

    return 0;
}

This is the mentioned (in the comment) "ticker" logic that shall go into the class as well which I initially left out to keep it simpler:

Ticker ticker;
InterruptIn pb(p17);
// Global count variable
int volatile counter = 0;
int volatile STATE = 0;

void countCallback(void) {
    counter = counter + 1;
}

// pb Interrupt routine - is interrupt activated by a falling edge of pb input
void pb_hit_interrupt (void) {
    ticker.attach(countCallback, 1);
    counter = 0;         
}

void pb_release_interrupt (void) {
    ticker.detach();
    stateEval();
    counter = 0;
    STATE = 0;
   // pc.printf("Counter: %d \n", counter);
}

int main() {
    // Use internal pullup for pushbutton
    pb.mode(PullUp);
    // Delay for initial pullup to take effect
    wait(.01);
    // Attach the address of the interrupt handler routine for pushbutton
    pb.rise(&pb_release_interrupt);
    pb.fall(&pb_hit_interrupt);
}

This is the code merging the sections "If you can't use C++11" and "working with interrupts"

    #include "mbed.h"

Serial pc(USBTX, USBRX);

struct action
{
    int counter_limit;
    void (*transition)(void);
};

struct action_list
{
    size_t size;
    const action *table;
};

class ButtonHandler {
public:
/*    ButtonHandler(PinName pin, float seconds, std::initializer_list<action> table)
    : counter{0},
      buttonPin{pin},
      intervalInSeconds{seconds},
      mTable{table.size(), table.begin()},
      ticker{}
    { }
*/
    ButtonHandler(std::size_t table_size, const action* begin, PinName pin, float seconds ) {
        mTable.size = table_size;
        mTable.table = begin;
        buttonPin = pin;
        intervalInSeconds = seconds;
    }

    void react(int counter) const {
        const action *a = mTable.table;
        for (std::size_t i=mTable.size; i; --i, ++a) {
            if (counter < a->counter_limit) {
                a->transition();
                return;
            }
        }
    }
   void enable() {
        buttonPin.mode(PullUp);
        wait(0.01);
        buttonPin.rise(this, &ButtonHandler::release);
        buttonPin.fall(this, &ButtonHandler::press);
        buttonPin.enable_irq();
    }
    void disable() {
        buttonPin.disable_irq();
        ticker.detach();
    }
    virtual ~ButtonHandler() {
        disable();
    }
protected:

    void press() {
        counter = 0;
        ticker.attach(this, &ButtonHandler::secondsCount, intervalInSeconds);
    }
    void secondsCount() {
        ++counter;
    }
    void release() {
        ticker.detach();
        react(counter);
        counter = 0;
    }
private:
    volatile unsigned counter;
    InterruptIn buttonPin;
    float intervalInSeconds;
    action_list mTable;
    Ticker ticker;
};

void state1() {
    //std::cout << __func__;
    pc.printf("state1");
}
void state4() {
    //std::cout << __func__;
    pc.printf("state4");
}
void state8() {
    //std::cout << __func__;
    pc.printf("state8");
}

PinName pin(p17);
//const int pb17 = 17;

//Action Table specifiying push duration and callback function
const action action_table[] =
{
    { 4, state1 },
    { 8, state4 },
    { 11, state8 },
};

int main()
{
    const ButtonHandler green(
            sizeof(action_table)/sizeof(action_table[0]),
            action_table,
            pin,
            1
    );

    green.enable();
}

There're still two compiler errors:

Error: No default constructor exists for class "mbed::InterruptIn" in "main.cpp", Line: 32, Col: 94

Error: The object has type qualifiers that are not compatible with the member function in "main.cpp", Line: 117, Col: 6

Answer 1

The code is not bad as it is, but here are some ideas that may help you improve your code.

Use an object

This is an idea you've already had, but this will show you one way of doing that, which I'll then add to for the next points. First, I'm going to assume that you're using a recent enough gcc toolchain that it support C++11. That can be enabled by using -std=c++11 on the command line. There are some important things in C++11 that are useful here, the first of which is constexpr which enforces the idea that something is created at compile-time:

constexpr action action_table[] =
{
    { 4, state1 },
    { 8, state4 },
    { 11, state8 },
};

The simplest way to wrap this in an object would be to provide the table as a constructor argument:

class StateMachine {
public:
    StateMachine(size_t size, const action* act_table) 
    : mTable{act_table}, mSize{size}
    { }

Rather than give it a separate named function, this overload allows us to call the object directly with an int argument, as though it were a function.

    void operator()(int counter) const {
        const auto *a = mTable;
        for (auto i=mSize; i; --i, ++a) {
            if (counter < a->counter_limit) {
                a->transition();
                return;
            }
        }
    }
private:
    const action* mTable;
    size_t mSize;
};

Note here that while the StateMachine object may be dynamically created, it's only taking up enough RAM to store the mTable pointer and the mSize. Here's how it might be used:

int main()
{
    const StateMachine sm{sizeof(action_table)/sizeof(action), action_table};
    for (int counter = 0 ; counter < 11; ++counter)
        sm(counter);
}

Note that there are some problems with this approach. First, there's no enforcement that the action_table is immutable. If it were dynamically created and then destroyed after the creation of the StateMachine, it would be attempting to use an invalid pointer. We can do better.

Use std::initializer_list

It's possible to create the ation_table array simply by directly listing the various actions. What would be nice would be able to declare it directly like this:

constexpr StateMachine sm{
    { 4, state1 },
    { 8, state4 },
    { 11, state8 },
};

In fact, we can with another C++11 innovation called the std::initializer_list. It allows us to write a constexpr constructor very similar to the previous

constexpr StateMachine(std::initializer_list<action> table)
: mTable{table.size(), table.begin()} 
{ }

The data is now simply the table:

private:
    action_list mTable; 

The result is extremely efficient. Omitting the existing definitions for your action structure and the statex() functions, we have this:

buttonpress.cpp

#include <iostream>
#include <initializer_list>

// previous action struct and statex() functions here

struct action_list 
{
    size_t size;
    const action *table;
};

class StateMachine {
public:
    constexpr StateMachine(std::initializer_list<action> table)
    : mTable{table.size(), table.begin()} 
    { }
    void operator()(int counter) const {
        const auto *a = mTable.table;
        for (auto i=mTable.size; i; --i, ++a) {
            if (counter < a->counter_limit) {
                a->transition();
                return;
            }
        }
    }
private:
    action_list mTable; 
};

int main()
{
  constexpr StateMachine sm{
    { 4, state1 },
    { 8, state4 },
    { 11, state8 },
  };
  for (int counter = 0 ; counter < 11; ++counter)
        sm(counter);
}

For example, compiling for a Cortex-M3 using gcc 5.2.0 (gcc-arm-none-eabi), with this command line:

arm-none-eabi-g++ -c -mthumb -mcpu=cortex-m3 -std=c++11 -O2 buttonpress.cpp -o buttonpress.o

And then disassmbling the results with this one:

arm-none-eabi-objdump -d buttonpress.o |c++filt >buttonpress.sa

We get the following for main:

00000000 <main>:
   0:   b570        push    {r4, r5, r6, lr}
   2:   4b09        ldr r3, [pc, #36]   ; (28 <main+0x28>)
   4:   2400        movs    r4, #0
   6:   4e09        ldr r6, [pc, #36]   ; (2c <main+0x2c>)
   8:   4d09        ldr r5, [pc, #36]   ; (30 <main+0x30>)
   a:   e005        b.n 18 <main+0x18>
   c:   2c03        cmp r4, #3
   e:   dd09        ble.n   24 <main+0x24>
  10:   2c07        cmp r4, #7
  12:   bfcc        ite gt
  14:   4633        movgt   r3, r6
  16:   462b        movle   r3, r5
  18:   3401        adds    r4, #1
  1a:   4798        blx r3
  1c:   2c0b        cmp r4, #11
  1e:   d1f5        bne.n   c <main+0xc>
  20:   2000        movs    r0, #0
  22:   bd70        pop {r4, r5, r6, pc}
  24:   4b00        ldr r3, [pc, #0]    ; (28 <main+0x28>)
  26:   e7f7        b.n 18 <main+0x18>
        ...
                        28: R_ARM_ABS32 state1()
                        2c: R_ARM_ABS32 state8()
                        30: R_ARM_ABS32 state4()

If you're familiar with ARM assembly language, you'll see that not only did the compiler generate efficient code, but it actually incorporated the counter_limit arguments for each action into the code rather than maintaining it as part of a table. One of the significant advantage to using constexpr is to enable exactly this kind of compiler optimization.

If you can't use C++11

As I've mentioned, C++11 makes a lot of things easier and more directly expressible. However, if you don't yet have such a compiler available, here's an alternative version of the code:

#include <iostream>

// action, action_table and states as in the original
struct action_list 
{
    size_t size;
    const action *table;
};

const int pb17 = 17;

class ButtonHandler {    
public:
    ButtonHandler(std::size_t table_size, const action* begin) {
        mTable.size = table_size;
        mTable.table = begin; 
    }

    void operator()(int counter) const {
        const action *a = mTable.table;
        for (std::size_t i=mTable.size; i; --i, ++a) {
            if (counter < a->counter_limit) {
                a->transition();
                return;
            }
        }
    }
private:
    action_list mTable; 
};

int main()
{
    const ButtonHandler green(
            sizeof(action_table)/sizeof(action_table[0]),
            action_table
    );

    for (int counter = 0 ; counter < 11; ++counter)
        green(counter);
}

Work with interrupts

As I understand it, the idea here is essentially this:

1. wait for a button press event
2. when button is pressed, start timer/counter
3. when button released, stop timer/counter
4. call appropriate action based on table counter_limit

If that's correct, here's how I might want to use it if I wanted to assign a button handler to handle a button. Specific values chosen are arbitrary, but based somewhat on your code.

int main()
{
    const ButtonHandler green{pb17, 1, {
        { 4, state1 },
        { 8, state4 },
        { 11, state8 },
    }};
    // ...
    green.enable();
    // ...
    green.disable();
}

I don't use mBed, but based on their documentation, I imagine something like this might work:

Warning! untested code~

class ButtonHandler {
public:
    ButtonHandler(PinName pin, float seconds, std::initializer_list<action> table)
    : counter{0},
      buttonPin{pin}, 
      intervalInSeconds{seconds},
      mTable{table.size(), table.begin()},
      ticker{} 
    { }
    void enable() {
        buttonPin.mode(PullUp);
        wait(0.01);
        buttonPin.rise(this, &ButtonHandler::release);
        buttonPin.fall(this, &ButtonHandler::press);
        buttonPin.enable_irq();
    }
    void disable() {
        buttonPin.disable_irq();
        ticker.detach();
    }
    virtual ~ButtonHandler() {
        disable();
    }
protected:
    void react(int counter) const {
        const auto *a = mTable.table;
        for (auto i=mTable.size; i; --i, ++a) {
            if (counter < a->counter_limit) {
                a->transition();
                return;
            }
        }
    }
    void press() {
        counter = 0;
        ticker.attach(this, &ButtonHandler::secondsCounter, intervalInSeconds);
    }
    void secondsCount() {
        ++counter;
    }
    void release() {
        ticker.detach();
        react(counter);
        counter = 0;
    }
private:
    volatile unsigned counter;
    InterruptIn buttonPin;
    float intervalInSeconds;
    action_list mTable; 
    Ticker ticker;
};