JChristensen / JC_Button

Arduino library to debounce button switches, detect presses, releases, and long presses
GNU General Public License v3.0
425 stars 102 forks source link

Allow to use external i/o on read(), optimizes button bulk processing #27

Closed postpersonality closed 3 years ago

postpersonality commented 4 years ago

This modification allows to use direct io-pin read and single millis() call for several buttons at once. That allows to save up some cycles for speed-critical applications.

uint32_t ms = millis();
button1.read(ms, PINB & (1 << PINB1));
button2.read(ms, PINB & (1 << PINB2));
button3.read(ms, PINB & (1 << PINB3));
button4.read(ms, PINB & (1 << PINB4));
button5.read(ms, PINB & (1 << PINB5));
JChristensen commented 4 years ago

Interesting! I can see where that would be more efficient. Did you have an application where it made a difference?

postpersonality commented 4 years ago

Yes, I've used it in the https://github.com/postpersonality/led-matrix-drawing-board. There are two time-critical jobs: read encoders (pulses are very short) and LED-strip data send out.

https://github.com/postpersonality/led-matrix-drawing-board/blob/main/src/main.cpp#L77 You may see I process my rotary encoders in interrupts the similar way as I propose it in the PR. Originally the buttons readings were in the interruption handlers too. But I've moved them out since the button presses are much less critical in my case.

But I still want to contribute to your awesome lib. If you need me to add some examples or docs I will do that in a breeze.

JChristensen commented 4 years ago

I'm curious as to the thinking behind read(uint32_t ms), did you use that as well as read(uint32_t ms, bool pinVal) in your application? It seems that if I'm interested in optimizing the millis() calls, then I'd also want to optimize digitalRead() as well, since that incurs additional overhead too. OTOH there could also be a read(bool pinVal) function to allow the user to supply the pin value but not the ms value. I wonder if just having read() and read(uint32_t ms, bool pinVal) isn't a good enough approach. The user could code millis() and/or digitalRead() calls as arguments if desired, and this would avoid the overhead of an additional function call (which is probably not a huge thing and maybe the compiler might even optimize it away.) Just thinking out loud. Cheers ... jc

postpersonality commented 4 years ago

I see your point. No, I do not use read(uint32_t ms, bool pinVal) in my current application. And use only read(uint32_t ms) instead. Since I moved out the button processing out of the interruption. But if I would process the buttons inside the interruption handler I would use read(uint32_t ms, bool pinVal) providing both values without a single millis call actually. There is a possibility to count millis inside timer interruption. So when pin change interruption happpens there would be a ready for consumption a millis value somewhere in the static varible.

Having read(bool pinVal) is indeed a good idea. I can imagine a case for it. It is good for the remote events processing for a single source. Someone may require that as well.

Just to sum up