Arduino Protothreads seem to be shared by two buttons

Question

I ran into a problem regarding the Protothreading library in Arduino. I have created a Button class, which represents a hardware button. Now the idea is that you can attach a ButtonListener to it, which listens to the button. If a button is pressed, then the clicked() function is called.

#include <Arduino.h>
#include <pt.h>

class ButtonListener {
    public:
        virtual void clicked() = 0;
        virtual void longClicked() = 0;
        virtual void tapped(int) = 0;
};

class Button {

    static const int RECOIL_TIME = 200;
    static const int LONG_CLICK_LENGTH = 1000;

    private:
        int _pin;
        ButtonListener *_listener;
        struct pt _thread;
        unsigned long _timestamp = 0;

        int listenerHook(struct pt *pt) {
            PT_BEGIN(pt);
            this->_timestamp = 0;
            while (true) {
                PT_WAIT_UNTIL(pt, millis() - _timestamp > 1);
                _timestamp = millis();
                if (&this->_listener != NULL) {
                    this->listenForClick();
                }
            }
            PT_END(pt);
        }

        void listenForClick() {
            boolean longClicked = true;
            int state = digitalRead(this->_pin);
            if (state == HIGH) {
                unsigned long timestamp = millis();
                while (true) {
                    longClicked = millis() - timestamp > LONG_CLICK_LENGTH;
                    state = digitalRead(this->_pin);
                    if (state == LOW) {
                        break;
                    }
                }
                if (&this->_listener != NULL) {
                    if (longClicked) {
                        (*this->_listener).longClicked();
                    }
                    else {
                        (*this->_listener).clicked();
                    }
                }
            }
        }

    public:
        Button(int pin) {
            this->_pin = pin;
        }

        void init() {
            pinMode(this->_pin, OUTPUT);
            PT_INIT(&this->_thread);
        }

        void setListener(ButtonListener *listener) {
            this->_listener = listener;
        }

        void listen() {
            this->listenerHook(&this->_thread);
        }
};

Now I've created two implementations of ButtonListener:

class Button12Listener : public ButtonListener {
    public:
        void clicked() {
            Serial.println("Button 12 clicked!");
        }
}

The other implementation is a Button13Listener and prints "Button 13 clicked!"

Then let's run the code:

// Instantiate the buttons
Button button12(12);
Button button13(13);

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

    button12.init();
    button13.init();

    // Add listeners to the buttons
    button12.setListener(new Button12Listener());
    button13.setListener(new Button13Listener());
}

void loop() {
    while (true) {
        // Listen for button clicks
        button12.listen();
        button13.listen();
    }
    Serial.println("Loop ended.");
    delay(60000);
}

I expect "Button 12 clicked!" when I click the button on pin 12, and "Button 13 clicked!" when I click the button on pin 13.

But when I try to click on any of the buttons, it is randomly printing "Button 12 clicked!" or "Button 13 clicked!" no matter what button I press.

It look like the protothreads are shared among the buttons or something.

If I check in which order the buttons are called, like this:

button12.listen();
Serial.println("listen12");
button13.listen();
Serial.println("listen13");

then the following outputs:

12
13
12
13
12
12

Thát seems okay.

So what's the problem? What have I missed?

Answer 1

You are completely eliminating the whole point of protothreads by having that while(true) loop in listenForClick. I would do it like this:

PT_BEGIN(thr); 
while(1){
  // ensure that the pin is low when you start
  PT_WAIT_UNTIL(thr, digitalRead(pin) == LOW); 
  // wait until pin goes high
  PT_WAIT_UNTIL(thr, digitalRead(pin) == HIGH); 
  // insert delay here for minimum time the pin must be high
  this->timeout = millis() + 20; // 20 ms
  // wait until the delay has expired
  PT_WAIT_UNTIL(thr, this->timeout - millis() > 0); 
  // wait until the pin goes low again
  PT_WAIT_UNTIL(thr, digitalRead(pin) == LOW); 
  // call the click callback
  this->clicked(); 
}
PT_END(thr); 

Then just call this thread repeatedly.

NOTE: when you have buttons connected, you would usually have pullup on the pin and have button connected between the pin and ground - so the pin is LOW when the button is down and high when it is not being pressed. This would certainly be the case on an arduino. So you would have to change the code above to wait for a negative pulse instead of a positive one. :)