Button improvements

[vortigont]

Hi @rwmingis, a bit of improvements to your lib if you mind to check it.

Button timing changes:

• longKeyPress now triggers right after m_longKeyPressMS interval, not waiting for button release. It looks more comfortable to get a feedback once long_press interval reached. Otherwise it is not always clear how long I need to keep the button pressed to trigger long_press
• if none of the double_click callbacks defined, than KeyPress() triggers right after button release, no need to wait for DoubleClick timeout

Implemented 'repeat' action Repeat action triggers if button is kept pressed longer than longKeyPress interval. It could be enabled/disabled on a per-instance basis. Also repeat interval could be changed. Both types of async and sync callbacks could be assigned for repeat action. If none of the repeat-specific callback's assigned, than a simple keyPress action repeated

I'm kind of like this lib :) Looks almost the way I wanted to implement something of a kind on my own :) Have some other ideas to add later, if you like. Might worth opening a discussion section in your repo.

Cheers!

[rwmingis]

Hi @vortigont,

All good tips, thanks for the feedback,

For the long keypress, are you suggesting that it should fire once the long keypress timeout is reached rather than wait for the timeout and then button release? Makes sense, I just need to think it through.

Agree on the not waiting for double click timeout if doubleclick not defined. Good idea.

The repeat function is a good idea, I just need to think about how this works in consideration with the long keypress as they are similar. I suppose I could allow an either/or selection for longkeypress option or repeat function configuration. Asynchronous operation would be easy and logical to implement, though synchronous may be clunky depending on loop timing. Would need to think through.

I am new to github, so need to learn how to sort out versioning with some of these changes, particularly with changes to function. Also will open a discussion session. Cheers!

Rob

[vortigont]

For the long keypress, are you suggesting that it should fire once the long keypress timeout is reached rather than wait for the timeout and then button release? Makes sense, I just need to think it through.

exactly! I faced this when testing simple LED blinking - sometimes it worked as longPress and some times I released button too early and it triggered a simple keyPress with different action. So I had to tell my self "keep holding it :)"... much more easy to get feedback right after you kept it long enough to recognize a longHold. Well, that was my idea.

I just need to think about how this works in consideration with the long keypress as they are similar. I suppose I could allow an either/or selection for longkeypress option or repeat function configuration.

yep, these could be different ways also. For now I've made it this way:

• repeat is disabled by default, you need to specifically enable it via repeatOnHold(bool state) call.
• longPress is not affected in any way. If callback has been defined - it will fire
• if both repeat and logPress defined/enabled it will go this way: 1) Press Down T+0 2) lognPress T+ longpress_interval 3) first repeat action T+ longpress_timeout + repeat_interval 4) repeat every repeat_interval

I've split the changes into two different commits - one for longpress and timers and another for repeat, as repeat depends on changes for longPress. Creating some 'experimental' branch could be a good idea to start with, so no to mess with current lib features. Would be happy to discuss other options. Cheers!

[rwmingis]

Hi @vortigont, starting to look at this again after a busy week. I am new to Github, pull requests, commits, and versioning so please bear with me. Given that the logic does get surprisingly complicate around the state machine, debouncing, and various sync and async events, associated with ISR's, I have created a flow diagram, see it in the code section. Figure it's best to get this right before updating the code. Incorporates your suggestion of immediate longpress events, and the introduction of a rapidfire/repeat feature.

Have a look and see what you think.

CHeers

Rob

[vortigont]

Hi @rwmingis that's a good idea with chart! A bit of corrections from my side - for the "State: pressed" a first step I'd set

• check long_press time expired
• if yes, than:
  • run async log_press acton, and schedule sync
  • if repeat is enabled and repeat timer is not running yet - start it
  • if no:
  • go to the "state: waiting for release"

something like this. I'll go trough it again and let you know if I'm missed something

Cheers!

[rwmingis]

Hi @vortigont,

Right, have had another update. Bit of a distraction here today, some poor soul got eaten by a shark down the road (here in australia) but at least this takes my mind off things.

I have added a lot of your suggestions, and I think we came to the same conclusion independently on some of them.

Have reused the longpress timer as the repeat timer as they are similar in function and never used at same time.

Have also made much of the library static class members to minimise code repetition amongs individual instances to save memory. Still need to do dynamic memory allocation for more or less pages of menu levels. Maybe i'll use std::vector.

Still have to figure out out to make it platform independent without making it too complicated and still servicing all vesrion s of the ESP32 and also the single version of the ESP8266.

Bit of tidying to do still, but the function is there now.

I need to learn how to do a release as well, just so much to do so little time.

Cheers and let me know how you go with testing.

Rob

[vortigont]

Hello @rwmingis! Had quite a busy week here.

some poor soul got eaten by a shark down the road (here in australia)

Ugh! What a chance to be eaten by a shark! Must be real hungry one, sigh! Quite funny to watch 'em while diving, but better not to face the one that hunts for a prey.

Have also made much of the library static class members to minimise code repetition amongs individual instances to save memory.

I like the changes, will give it a try next week! I've made one of my light controllers with four optocoupled 'buttons' hooked to the wall switches. Work pretty good with this lib! Just the right thing for me :) So I'll keep working on improving it.

Still need to do dynamic memory allocation for more or less pages of menu levels. Maybe i'll use std::vector.

Last time I've checked std::vector appeared pretty hungry for memory on esp platforms. Mostly due to preallocating more memory in advance that it is required, although pretty easy to use. I ended up using LinkedLists in my projects. A bit updated ivanseidel's lib.

Still have to figure out out to make it platform independent without making it too complicated and still servicing all vesrion s of the ESP32 and also the single version of the ESP8266.

My 5 cent's to this - trying to make it platform independent would ruin most yummy features if this lib. esp8266 is old and lacks RTOS/IDF support. Stick it to Arduino API and it will become just another button lib. I would love to see this lib strictly using IDF API. This way it will handle all esp32 family chips, s2, c2, s3, h and whatever other chips are coming... It will work on esp32-arduino automatically. If you drop the Arduino API the next evolution step becomes obvious - get rid of sync/async modes. Use some RTOS queue or event loop and make all button events async! Get rid of the ugly loop() hook processSyncEvents(). Quite a roadmap, huh? :)

I need to learn how to do a release as well, just so much to do so little time.

it's pretty easy here. Release is nothing but milestone with a read-me here :) I usually do a git tagged commit with README relnotes and version mark, than spawn a release out of this tag. It's just a convenient way to tie to specific lib version in Platformio or what is your favorite build env.

Will update you later with testing results for dynamic button objects. Still need this feature :) Cheers!

[vortigont]

OK, tried your changes lately. There are two things missing in auto repeat: 1) need a method to change repeat timer somehow. The one provided in a constructor might need an adjustment later. 2) if no specific callback provided it would be handy to repeat a plain button press (kind of like it works with volume button on a TV remote :) )

2.5) I was thinking that maybe we can use m_autoRepeatMS variable as a "repeat" enabler. By default it is set to "0", so means "no autorepeat". If set to non zero value, than autorepeat is enabled. How about it?

[rwmingis]

Hi @vortigont, thanks for the feedback again, it really is improving the library.

I have implemented most of your suggestions, but need to do a bit more testing and tidying before uploading the new code. (in the past, my uploads have broken your pull requests, may try to learn to do it properly or just charge on like a bull in a chinashop 😂)

Have not tested the dynamic button objects yet (but some of the changes may have fixed that by default), and also need to start making it arduino independan. I have not used the ESP32 IDF, RTOS, or anything like that aside from stealing a timer or ADC calibration function or two, so need to do a bit more research.

Will upload in the next few days.

Cheers

Rob

[rwmingis]

Hi @vortigont,

I have done a big update to the whole library as a one off. I haven't done any versioning yet, just copying and pasting the new code, so this one may break a few member interfaces for you, but being a small library, it's pretty straight forward and logical. See the example, it covers most things aside from unbinding actions and modifying special press timings.

I feel like it is becoming a good functional library now (with a lot of input from you which has been great). Now just starting to look at optimisations and getting rid of the Arduino API so it suits ESP_IDF and Arduino. I will close out this pull request, and you can start another for further features to do it in a more streamlined fashion.

To Summarise some of the changes on based on this pull request, see below:

1. Long key presses occur immediately after time out occurs. Done by seperate timer process rather than flags and timekeeping (as you still need a timer to timeout). Only difference is where the action code is called either from timer or from state machine.

2. . Repeat pressess occur after the longpress time out if enabled. As you suggested, it looks for an autopress specific action defined. If not, it uses the regular keypress action. THis behaviour is stopped if the autopress function is specifically disabled.

3. Special presses are initialised to disabled (long, double, and autorepeat), but once a special press is defined using bind, it is automtatically enabled. In general, I am explicitly enabling or disabling these actions this way as it is more intuitive, and better to not undefine or re-define an action just because one wants to temporarily disable it.

4. Have tidied the event action pointers. Now a 2 dimensional dynamic array that is "m_menuLevel" rows long, and "number of event types" columns wide.. The number of menus can be set once before the first button is defined, so we are no longer limited to ust 5 menus. Can't think of a time when you want to dynamically increase menu length during runtime, but I suppose could be done, just didn't want to deal with memory reallocation atm, and keeping a list of all buttons (remembering that all buttons have their own event action array and they all must be the same number of menuLevels) A single function (bind) is now used to bind a specific external action to a specific button event event using the menuLevel, the enumerated event, and the external action. Actions can also be 'unBind'ed but not sure if there is a lot of use there. No vectors as you suggested.

5. Timing variables for longPress, AutoRepeat Press, and Double Clicks all have their own getters and setters to give the user the ability to change these. Debounce does not, can't see that needing to change.

6. I do intend on droping the Arduino API, maybe as my next task, though I don't have much experience with the ESPIDF, so any code snippets you wish to share with repect to setting up onChange interrupt, setting buttons as inputs, and reading a button's state would be appreciated. I have figured out how to do ESP_IDF timers and clear stale interrupt flags as you can see in the code, but did take some time.

7. As much as the code that can be part of the static class has been made so to limit code repetition among different instances.

8. Have given timers a timestamp and a rough calling name (based on press type). Haven't put much thought into it or tidied, so a bit rough (ie they don't need to be using 'Strings" and they will have to go, along with 'Serial' calls when turfing arduino)

Have a look at the new code and let me know what you think. Cheers!

[vortigont]

Hi @rwmingis! That's awesome job, thanks! Give me a day or two to go through it and test, but looks very promising! Sure, I can suggest code changes to deArduino pin's settings and interrupts methods. You might wanna create some 'experimental' branch and I can make a small atomic PRs for those features for you to check it one bite at a time.

Regards!