3
\$\begingroup\$

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
\$\endgroup\$
3
  • \$\begingroup\$ Is the action_table dynamically created or is always in ROM? Also, could you show how you'd like to be able to use a class for this? That is, ideally what would code look like that uses it? \$\endgroup\$ – Edward Mar 24 '16 at 21:13
  • \$\begingroup\$ The action_table would be always in ROM. It is hardcoded. I've edited the post to show how I'd like to use the class. The environment I am using (mbed) is offering a ticker interrupt class, which I use as timer to measure the duration of the button press. This logic, that I've left out initially to not complicate things should ideally go into that class too. Obviously that would change the way how I use the class hence I decided to add the relevant code. \$\endgroup\$ – jens strümper Mar 25 '16 at 10:02
  • \$\begingroup\$ So much meta programming! When you have written so much overhead code for something so trivial, surely your de-bouncing algorithm must be state of the art! And you have an edge-triggered interrupt connected to a button pin? How do you handle the spurious interrupts caused by that? (You can do that, but it is not trivial). My advise to you is to completely forget about C++ (for now). It is directly harmful. Focus on the basics of embedded programming and electronics instead. \$\endgroup\$ – Lundin Apr 20 '16 at 12:47
1
\$\begingroup\$

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;
};
\$\endgroup\$
10
  • \$\begingroup\$ I've studied your examples for several hours but I'm still a bit puzzled. Functionality wise It is exactly what I was looking for but unfortunately mbed doesn't support C++11 yet. From what I understood from the forums their IDE is still using an "old" ARM 5 based compiler. So why not using an alternative toolchain? I am using the Nordic nrf51 chip and mbed has build a nice library for the nrf51's bluetooth LE softdevice - hence I can't easily switch to a different compiler. So I tried to use your example "1" without the constexpr but then how do I know the datatype within the class? \$\endgroup\$ – jens strümper Mar 29 '16 at 15:30
  • \$\begingroup\$ It might not be an easy switch, but once I switched to using C++11 for embedded work, I found that a lot of things got much easier. If you think you might want to investigate that, you might find this series of articles on the nrf51 useful. Good luck! \$\endgroup\$ – Edward Mar 29 '16 at 15:35
  • \$\begingroup\$ The keyword "auto" doesn't exist in non C++11/14 compilers or has a different meaning. \$\endgroup\$ – jens strümper Mar 29 '16 at 15:38
  • \$\begingroup\$ Yes, that's correct. auto used to be a storage specifier but since C++11 is now a way to tell the compiler "you can figure out what type this is." In the code I provided, you can simply substitute the actual types which are void (*)(void) and size_t respectively. \$\endgroup\$ – Edward Mar 29 '16 at 15:43
  • \$\begingroup\$ The compiler tells me that list initialization is a c++11 feature so I assume that I can't pass the table as a constructor argument in my environment. I had a look at the nrf51 gcc example and further reading on the original nordic bluetooth api. It would be nice to switch to a more modern compiler but I guess I would run into even bigger problems considering my current skill level. Is there no "other" way to pass the array to the class that doesn't required C++11 features? Anyways, I really appreciate all your efforts in providing an solution for this problem! \$\endgroup\$ – jens strümper Mar 29 '16 at 16:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.