Performance and multiple buttons on one analog pin

[Note: topic renamed, original title: "Performance: incrementing a variable VS using millis()"]

Hi everyone! :slight_smile:

I've been using Arduino for the past week and loving it so far!

I've got a couple questions regarding the chip performance:

1) How do you measure how long things take inside the loop?

I tried the general approach of taking note of time before starting the operation and when its finished, then just subtracting the former from the latter and outputting result to the Serial screen.
However I find that the output to the Serial itself is taking a very long time (much longer then the operation that I am trying to measure) and that skews the results.

2) What is more preferable for timing things - incrementing a variable (e.g. when doing a software debounce) OR using millis() ?

In my first project - I want to make a small "Simon says" type of game. I have 3 LEDs and 3 push buttons. All of them use software debouncing for timing events (LEDs blink, buttons listen for input) There is also the main loop which needs to time things. So I've got a total of 7 entities which need to time things. What is better - incrementing 7 variables every cycle, or calling the millis() function 7 times?

In my day to day life I am mostly dealing with dynamic languages, so in my mind an increment is more performant then a function call (in most cases anyway) However I don't know how true this is in the Arduino environment.

I know that millis() are tied to real time, unlike the variable increment, however in my project this is not critical. I just want to know what is better in terms of performance

Any advice on this is much appreciated!

Serial is buffered. It shouldn't be affecting results too much. Please post your code which demonstrates this.

Thanks for a quick response!
Here is the simplified version of the code which demonstrates the slow down when using Serial port (uncomment the first line inside the loop() )

In this version - I have 3 buttons hooked up to one analog pin, listening for user input. They each turn the corresponding LED on/off when pushed.

PS sorry about the code not being super nice, it is just an experiment at this point

// PINS
const int buttonPin = 0;
const int ledPin1 =  11; 
const int ledPin2 =  10; 
const int ledPin3 =  9; 

// GLOBAL
int currentReading; // current reading from the buttons analog pin
int currentLed = LOW; // current LED state

// BUTTON
class Button {
	int pin;
	int triggerValue;
	int triggerFlac = 40; // trigger flactuation
	long inputDelay = 100; // debounce delay

	long debounce = 0; // debounce counter

	int buttonState;
	int buttonPressed;

	public:
		Button(int p, int tV) { // pin number, trigger value
			pin = p;
			triggerValue = tV;
			buttonState = 0;
			buttonPressed = 0;
		}

		void update() {

			// All buttons are on the same analog input pin
			// Check current reading
			if (currentReading > triggerValue - triggerFlac && currentReading < triggerValue + triggerFlac) {

				debounce++; // THIS IS THE VARIABLE I AM INCREMENTING TO CHECK THE TIMING

			} else {
				debounce = 0;
				buttonState = 0;
				buttonPressed = 0;
			}

			// The button state is changed if the reading is within
			// the trigger value boundaries for a certain amount of time
			if (debounce > inputDelay) {
				buttonState = 1;
			}

			if (buttonPressed == 0 && buttonState == 1) {
				currentLed = !currentLed;
				digitalWrite(pin, currentLed);
				buttonPressed = 1;
			}
		}
};

Button but1(ledPin1, 845);
Button but2(ledPin2, 922);
Button but3(ledPin3, 1013);

void setup() {
	pinMode(buttonPin, INPUT);
	pinMode(ledPin1, OUTPUT);
	pinMode(ledPin2, OUTPUT);
	pinMode(ledPin3, OUTPUT);

	Serial.begin(9600);
}


void loop() {
	currentReading = analogRead(buttonPin);

	// Serial.println(currentReading); // <-- UNCOMMENT this to check if its taking longer

	but1.update();
	but2.update();
	but3.update();
}

The simple way to time some code is to record millis() immediately before the start of the test and immediately after the end of it. Then the difference is the elapsed time.

Have the dode in the test repeat often enough (1,000 or 10,000 times) to get a good average.

Something like

startMillis = millis();
for (int n = 0; n <= 10000; n++) {
   // stuff to test
}
endMillis = millis();

I can't make any sense of your own code. It seems a complicated way to deal with buttons.

...R

Have you tried running Serial at a higher baud rate ? 9600 is pretty pathetic.

Robin2:
The simple way to time some code is to record millis() immediately before the start of the test and immediately after the end of it.

Yep, that's what I'm doing, only trying to get a better reolution with micros() instead of millis()

Robin2:
I can't make any sense of your own code. It seems a complicated way to deal with buttons.

Lol, I was actually trying to make it simpler :slight_smile:
Is there a better general approach to this? (i.e, listening to multiple buttons at the same time)

UKHeliBob:
Have you tried running Serial at a higher baud rate ? 9600 is pretty pathetic.

Cheers for the suggestion!
I'll look into this now. Any suggestions on what would be a better value?

Any suggestions on what would be a better value?

Any suggestions on what you are talking to?

Serial data is buffered, as Nick said. What he assumed, that is not true in your case, is that there would be sufficient time to clock the data out before you filled the buffer again.

When the buffer gets full, the Serial.print() function becomes blocking. That is, it has to wait for there to be room in the buffer to write the data before it can return. Of course that is going to have an impact on your performance.

Any suggestions on what you are talking to?

I have an Arduino Uno board (with a 328p) running off USB, it has 3 LEDs connected to separate pins and 3 push buttons on a single analog 0 pin.
I am watching the output in the Serial monitor in Arduino IDE.

Serial data is buffered, as Nick said. What he assumed, that is not true in your case, is that there would be sufficient time to clock the data out before you filled the buffer again.

When the buffer gets full, the Serial.print() function becomes blocking. That is, it has to wait for there to be room in the buffer to write the data before it can return. Of course that is going to have an impact on your performance.

OK, I see, thanks for the explanation!
Is there any way to 'debug' it in a non-blocking/semi-non-blocking fasion?

PS
While I do care about measuring performance, at the moment I am really more interested in my second question.
Any light on this one?

"What is more preferable for timing things - incrementing a variable every cycle (e.g. when doing a software debounce) OR checking the elapsed time using the millis() function?"

I am watching the output in the Serial monitor

The Serial Monitor application has a drop down list that lets you set the baud rate. The values in the list are the only speeds that it supports. I think that it goes without saying that you want to pick the fastest one.

Is there any way to 'debug' it in a non-blocking/semi-non-blocking fasion?

No. You live with the limits of the debugging technique while you are debugging.

"What is more preferable for timing things - incrementing a variable every cycle (e.g. when doing a software debounce) OR checking the elapsed time using the millis() function?"

There is no simple answer to that. If the loop() function iterates at a known rate (extremely rare), then you could use a counter. For most things, millis(), micros(), and real time clocks are preferred.

Thanks for your help!
This really cleared things up a lot.

I'll stick with using millis() for now and will experiment with various Serial speed rates.

YemSalat:
Is there a better general approach to this? (i.e, listening to multiple buttons at the same time)

Something simple like

void readButtons() {
   for (n = 0; n < numButtons; n++) {
      btnState[n] = digitalRead(buttonPin[n]);
   }
}

...R

Robin2:
Something simple like

void readButtons() {

for (n = 0; n < numButtons; n++) {
      btnState[n] = digitalRead(buttonPin[n]);
  }
}

Keep in mind - I have multiple buttons on one pin, and I am also trying to do software debouncing for more precise push detection.

If you have several buttons on one pin I presume it is an analog pin and the different button combinations give different voltages through the use of resistors.

That makes the code much simpler

void readButtons() {
   buttonsVal = analogRead(buttonsAnalogPin);
}

You then need some IF statements or a SWITCH/CASE to figure out what buttons were pressed to give you buttonsVal.

I would be rather surprised if you have a bounce problem with analogRead(). analogRead() is not nearly as fast as digitalRead(). If there is a bounce issue just leave a reasonable time between reads.

...R

Robin2:
If you have several buttons on one pin I presume it is an analog pin and the different button combinations give different voltages through the use of resistors.

That makes the code much simpler

void readButtons() {

buttonsVal = analogRead(buttonsAnalogPin);
}
You then need some IF statements or a SWITCH/CASE to figure out what buttons were pressed to give you buttonsVal.

I think if you go with this approach and then add all the other logic - you will actually end up with more code then me and it will be harder to maintain.

Robin2:
I would be rather surprised if you have a bounce problem with analogRead(). analogRead() is not nearly as fast as digitalRead(). If there is a bounce issue just leave a reasonable time between reads.

...R

Sorry, I might be using the term "debounce" inccorrectly - essentially what I'm doing is just checking if the analog reading stays within certain boundaries for a number of clock cycles.

Right now I have the following code (compiles to 1,522 bytes). It makes it so that when each button is pressed - a corresponding LED blinks for 125 milliseconds and does not turn on unless the button is released and pressed again.

Any suggestions on how this can be improved / simplified are very welcome (PS I don't have much experience with C++, so please let me know if I am doing something stupid here):

const unsigned char BUTTON_PIN = 0;
const unsigned char LED_PIN_1 =  11; 
const unsigned char LED_PIN_2 =  10; 
const unsigned char LED_PIN_3 =  9; 
const int BUTTON_VALUE_1 = 845;
const int BUTTON_VALUE_2 = 925;
const int BUTTON_VALUE_3 = 1015;

// GLOBAL
int currentReading;
unsigned long currentTimer;

// LED Light
class Light {
    unsigned char pin;
    unsigned long timerCheck;

    unsigned char ledState;

    public:
        Light() = default;
        Light(unsigned char p) { // pin number
            pin = p;
            ledState = 0; // off by default

            timerCheck = 0;

            pinMode(pin, OUTPUT);
        }

        unsigned char getPin() {
            return pin;
        }

        void blink(unsigned long onTime) {
            timerCheck = currentTimer + onTime;
            ledState = 1;
            digitalWrite(pin, HIGH);
        }

        void update() {
            if (ledState == 1) {
                if (timerCheck < currentTimer) {
                    digitalWrite(pin, LOW);
                    ledState = 0;
                }
            }
        }
};

// BUTTON
class Button {
    int triggerValue;
    int triggerFlac; // trigger flactuation
    int inputDelay; // debounce delay
    
    Light * const ledLight; // led light

    unsigned char debounce;
    unsigned char buttonState;
    unsigned char buttonPressed;

    public:
        Button() = default;
        Button(int tV, Light & l) :
        triggerValue{tV},
        ledLight{&l}        
        { // pin number, trigger value
            debounce = 0;
            buttonState = 0;
            buttonPressed = 0;

            triggerFlac = 40;
            inputDelay = 150;
        }

        void update() {

            // All buttons are on the same analog input pin
            // Check current reading
            if (currentReading > triggerValue - triggerFlac && currentReading < triggerValue + triggerFlac) {

                debounce++; // THIS IS THE VARIABLE I AM INCREMENTING TO CHECK THE TIMING

            } else {
                debounce = 0;
                buttonState = 0;
                buttonPressed = 0;
            }

            // The button state is changed if the reading is within
            // the trigger value boundaries for a certain amount of time
            if (debounce > inputDelay) {
                buttonState = 1;
            }

            if (buttonPressed == 0 && buttonState == 1) {
                ledLight->blink(150);
                buttonPressed = 1;
            }

            // update LED
            ledLight->update();
        }
};

Light led1(LED_PIN_1);
Light led2(LED_PIN_2);
Light led3(LED_PIN_3);

Button but1(BUTTON_VALUE_1, led1);
Button but2(BUTTON_VALUE_2, led2);
Button but3(BUTTON_VALUE_3, led3);

void setup() {
    pinMode(BUTTON_PIN, INPUT);
}


void loop() {
    currentReading = analogRead(BUTTON_PIN);
    currentTimer = millis();

    // update buttons
    but1.update();
    but2.update();
    but3.update();
}

YemSalat:
I think if you go with this approach and then add all the other logic - you will actually end up with more code then me and it will be harder to maintain.

I'm prepared to invest some time in trying to prove you wrong.

But before I can do that I need to know how the buttons are connected to the pin. Can you make a pencil drawing of the circuit and post a photo of the drawing. Include the various resistor values.

...R

Thanks for that!
I will try to make a drawing, but will also take a picture of my current breadboard in case I miss something.

Also, as I mentiond in the first post - in the end this is going to be a "Simon Says" kind of game (where the LEDs blink in a certain sequence and the player needs to repeat it), so I am structuring my code in that direction.

I forgot to ask why are you using an analog pin for the buttons ?

If you have used up all your digital pins are you aware that you can use the analog pins as digital pins ?

...R

OK, I did the best I could to not be confusing :slight_smile: I admit I am terrible at schematics and/or diagrams, so please let me know if I need to explain anything better.

The wiring diagram:

And breadboard top-down view (ugh!):

Robin2:
I forgot to ask why are you using an analog pin for the buttons ?

If you have used up all your digital pins are you aware that you can use the analog pins as digital pins ?

...R

I am planning to transfer this program to an Attiny45 later (that's also why I want to keep the code small)
It only has 5 IO pins, so I had to put all the buttons on the same analog pin and use resistors to distinguish between them (as far as I understand this is quite a popular approach when the amount of pins is limited).

By outputting a number every time through loop you are adding an overhead of:

(1/9600) * 10 * 8 = 0.0083 seconds

So that's 8 ms slowdown (assuming a 6-digit number, plus cr/lf) per iteration.

That's enough to make it look slower. If you want to take timings, you might output the results after 1000 iterations, for example.

To time things I usually call micros() at the start of whatever-it-is, and then again afterwards, subtract one from the other and display the result.