Short and Long button press

Development
1

To continue a discussion from chat:

// example irq callback handler, copy for your own uses
void button_irq_callback(uint gpio, uint32_t events){
    gpio_acknowledge_irq(gpio, events);   
    gpio_set_irq_enabled(gpio, events, true);
    button_pressed=true;
}
// enable the irq for button button_id
void button_irq_enable(uint8_t button_id, void *callback){
    button_pressed=false;
    gpio_set_irq_enabled_with_callback(EXT1, GPIO_IRQ_EDGE_RISE, true, callback);
} 
// disable the irq for button button_id
void button_irq_disable(uint8_t button_id){
    gpio_set_irq_enabled(EXT1, GPIO_IRQ_EDGE_RISE, false);
    button_pressed=false;
}

To add a timer for short/long presses, Iā€™d start something like this:

// enable the irq for button button_id
void button_irq_enable(uint8_t button_id, void *callback){
    button_pressed=false;
    gpio_set_irq_enabled_with_callback(EXT1, GPIO_IRQ_EDGE_RISE|GPIO_IRQ_EDGE_FALL, true, callback);
}

Adding GPIO_IRQ_EDGE_FALL gives us interrupts on rise and falling edges now.

// example irq callback handler, copy for your own uses
void button_irq_callback(uint gpio, uint32_t events){

//is button pressed? rising edge i guess
if(events&GPIO_IRQ_EDGE_RISE){
//start a timer here
}else if (events&GPIO_IRQ_EDGE_FALL){
  //falling edge, button released. Stop timer, determine short or long
   if(elapsed_time<=BP_BUTTON_SHORT_PRESS_MS){
      button_pressed=BP_BUTTON_SHORT;
   }else{
      button_pressed=BP_BUTTON_LONG;
   }
}
//clear interrupts
    gpio_acknowledge_irq(gpio, events);   
    gpio_set_irq_enabled(gpio, events, true);

}

Iā€™m not sure this is exactly how the events variable is evaluated, but something in the neighborhood of this should work.

2

Thank you for the guidance. I have successfully made the separate rise and fall IRQā€™s work in button.c, with current src printing debug information for testing:

#include <stdio.h>
#include "pico/stdlib.h"
#include "pirate.h"

#define BP_BUTTON_SHORT_PRESS_MS 500 // Threshold for short button press

static bool button_pressed = false;
static absolute_time_t press_start_time;

// poll the value of button button_id
bool button_get(uint8_t button_id){
    return gpio_get(EXT1);
} 
bool button_check_irq(uint8_t button_id){
    if(button_pressed){
        button_pressed=false;
        return true;
    }
    return false;
}
// example irq callback handler, copy for your own uses
void button_irq_callback(uint gpio, uint32_t events){
    // Check if button was pressed (rising edge)
    if (events & GPIO_IRQ_EDGE_RISE) {
        printf("Button pressed (rising edge detected)... ");
        press_start_time =  get_absolute_time();
    }

    // Check if button was released (falling edge)
    if (events & GPIO_IRQ_EDGE_FALL) {
        absolute_time_t press_end_time = get_absolute_time();
        int64_t duration_ms = absolute_time_diff_us(press_start_time, press_end_time) / 1000;
        printf("Button released (falling edge detected) ");
        printf("Press duration: %lld ms ", duration_ms);
    
    if (duration_ms <= BP_BUTTON_SHORT_PRESS_MS) {
        printf("Short press detected\r\n");
    } else {
        printf("Long press detected\r\n");
    }
    
    button_pressed = true;
}

    gpio_acknowledge_irq(gpio, events);   
    gpio_set_irq_enabled(gpio, events, true);
//    button_pressed=true;
}

// enable the irq for button button_id
void button_irq_enable(uint8_t button_id, void *callback){
    button_pressed=false;
    gpio_set_irq_enabled_with_callback(EXT1, GPIO_IRQ_EDGE_RISE|GPIO_IRQ_EDGE_FALL, true, callback);
} 
// disable the irq for button button_id
void button_irq_disable(uint8_t button_id){
    gpio_set_irq_enabled(EXT1, GPIO_IRQ_EDGE_RISE|GPIO_IRQ_EDGE_FALL, false);
    button_pressed=false;
}
// initialize all buttons
void button_init(void){
    gpio_set_function(EXT1, GPIO_FUNC_SIO);
    gpio_set_dir(EXT1, GPIO_IN);
    gpio_pull_down(EXT1);
}

ā€”>
I have it print when the RISE IRQ is triggered, and then on release print when the FALL IRQ is triggered. i then have to print out the duration of the hold, and whether it was detected as a short of long press
ā€”>

HiZ> Button pressed (rising edge detected)... Button released (falling edge detected) Press duration: 1190 ms Long press detected
Using default 'button.scr'

HiZ> 

HiZ> Button pressed (rising edge detected)... Button released (falling edge detected) Press duration: 92 ms Short press detected

# reset power
HiZ> w
Command has no effect in HiZ mode, press 'm' to choose a mode
HiZ> W 3.3 20
Command has no effect in HiZ mode, press 'm' to choose a mode

HiZ>
1 Like
3

Hi Ian,

Do you have a hardware-based debounce circuit (e.g., RC filter, ) for the BP5 physical button?

If not, there may need to be additional complexity wrapped into the button handling. Iā€™ve not put a scope to my REV10 board, and wonā€™t have time for at least a couple weeks, so thought I might raise the question early, in case it saves later frustration.

P.S. ā€“ I realize this type of issue is well-known to many; For readers new to this, here is one decent write-up with scope showing visually whatā€™s happening.

1 Like
4

Iā€™m interested to see Ianā€™s respond on henrygabs comment. very interesting.
i have a branch with working long press logic:

can you let me know if you are interested in merging this?

1 Like
5

Nice work. Letā€™s get this integrated.

To future proof this, I feel like maybe it should return an value from an enumerated list? Also to keep pirate.c clean:

                if(button_check_irq(0, &long_press)){
                    button_irq_disable(0);
                    if (long_press) {
                        button_long_exec(); // over long press threshold execute bulong.scr
                    } else {
                        button_exec(); // short press execute button.scr
                    }
                    bp_state=BP_SM_COMMAND_PROMPT;  //return to command prompt
                }

How about moving this logic into the button.c lib and the button_scr.c commands?

                button_code=button_check_irq(0);
               if(button_code) {
                    button_irq_disable(0);
                    button_exec(button_code);
                    bp_state=BP_SM_COMMAND_PROMPT;  //return to command prompt
                }

Could even move the irq disable to button_exec.
With an enum of codes:

enum button_codes{
  BP_BUTT_NO_PRESS=0,
 BP_BUTT_SHORT_PRESS,
BP_BUTT_LONG_PRESS,
BP_BUTT_DOUBLE_TAP,
};

This gives the flexibility to add more button presses (BP_BUTT_DOUBLE_TAP) in the future without expanding on the code in pirate.c.

No, there is no hardware debugging, but the RP2040 pins have a schmitt trigger, and it seems to do ok along with the lazy servicing in the shared multitasking loop.

6

ok i think i accomplished what you wanted:

Added enum button_codes to button.h to define button press types.
Declared button_check_press function in button.h.
Added button press detection logic to button_irq_callback.
Implemented button_check_press to return the button press type.
Used a static variable button_code to track the current press type.
Moved button script execution logic to button_scr.c.
Added button_exec function in button_scr.c to handle short and long press script execution.
Integrated button_check_press and button_exec(press)code) into the main loop in pirate.c.

7

T he long/short functionality works well. Your changes are spot on.

In button_scr->button_script_handler:

In this function the user can assign a custom script to the button. It seems with the latest changes that assignment is tested, but not copied to the button file variable. Ideally, we need some way to assign both the long and short button scripts, with a verb or flag:

button short filename.scr
button long filename.scr

I believe the constant char file names will need to go back to, letā€™s say char[13] (8.3 file name, no folders).

Iā€™m going to roll it back for now to restore the original functionality. I can make some changes, but want to discuss with you first.

8

i literally woke up this morning and saw you merged and started thinking about stuff. i didnt catch the button assignment, but i was pondering

a) the help message needs to be updated
b) w eshould comment out double tab option for now.
enum button_codes {
BP_BUTT_NO_PRESS = 0,
BP_BUTT_SHORT_PRESS,
BP_BUTT_LONG_PRESS,
BP_BUTT_DOUBLE_TAP,
};
c) sidenote: the first time you do a short or long press, it checks if LONG_BUTTON_FLAG_FILE_CONFIGURED flag is NOT set in button_flags with the if (!(button_flags & BUTTON_FLAG_FILE_CONFIGURED))
, and if not copies it sets the flag. on subsequent long presses it would not print this message. it does the same for short. these 2 things could happen at different times though. i feel like maybe both variables should be set at the time either one is run.
d) and now you bring up the point about the assignment command

sorry i just woke up, but im brainstorming this now. it definitely needs some more minor considerations for the overall usability. we need to make sure it is clear inside of the software what is going on. not everyone is going to read new docs or diffs. we may also want to raise the long press threshold to 1000ms.

you are welcome to make changes or we can talk this out and i can make a new push sometime today.

i assigned these as const in button_scr.c so that will need to be changed

static const char *button_script_file = "button.scr";
static const char *button_long_script_file = "bulong.scr";

one confusion i have is where is 8.3 being enforced? we have static char button_script_file[32]; which should be able to hold a much larger filename. but oddly enough i tried a longer filename at first and had errors. i changed to bulong.scr because i figured thats how you wanted it, but didnt actually see why longer filenames didnt work.

EDIT:
and to be clear you had it this way at first, just not const

static char button_script_file[32];

you are saying now to change to char[13] for 8.3

ā€“

seriously though if you know exactly how we should add the new button assignment, feel free

9

I just need to come up with a game plan. maybe you can help me get my head on straight
1.) Do we want the new script to be named bulong.scr?
2.) how should we setup the strings to hold the script name? (im a bit confused if we are doing this correct with char[32] and we definitely cant have it defined as a const)
3.) do we add a new command, identical to button command but for bulong?
4.) i guess we can just leave how it sets the flags and is verbose on first press of each one. that will be fine as it will indicate wat script is being used if they changed it

EDIT: just let me know when you can talk in realtime. im thinking we should have a separate bulong_scr.c now?

my problem was i got focused on the changes i needed to make in button_exec in button_scr.c, but didnt consider how intertwined button_scr.c is with the button command

10

One of my biggest questions and where I got confused is i havenā€™t dealt much with the CLI commands. i did read your thread the other day on how to add one, and iā€™ve dug through the source today.

But here are some points:
commands.c includes button_scr.c and defines the button command of course:

{"button", true, &button_scr_handler, 0x00},

using the button_scr_handler which resides in button_scr.c
this all makes sense now. the button_scr.c is FOR the cli button command.

But where my confusion comes in, is this is intertwined with the actual handling of button presses by including button_exec. The original:

bool button_exec(void){

    if(!(button_flags & BUTTON_FLAG_FILE_CONFIGURED)){
        memcpy(button_script_file, "button.scr", sizeof("button.scr"));
        button_flags|=BUTTON_FLAG_FILE_CONFIGURED;
        printf("Using default '%s'\r\n", button_script_file); 
    }

    printf("\r\n");

    if(script_exec(button_script_file, false, !(button_flags & BUTTON_FLAG_HIDE_COMMENTS), false, (button_flags & BUTTON_FLAG_EXIT_ON_ERROR))){
        printf("\r\nError in script file '%s'. Try button -h for help\r\n", button_script_file);
        return true;
    }

}

which ultimately calls script_exec.

pirate.c original logic

                if(button_check_irq(0)){
                    button_irq_disable(0);
                    button_exec(); //if button pressed, run button.scr script
                    bp_state=BP_SM_COMMAND_PROMPT;  //return to command prompt
                }
                break;

yes button_exec is what sets the script files, but its not necessarily related to the CLI button command. see what im getting at? shouldnt we move button_exec to button.c and only use button_scr.c (and bulong_scr.c) for the command line handling?

11

The flash media is using the FAT file system, and to save RAM, has explicitly disabled the long file name (LFN) functionality. Therefore, you are limited to filenames that would be accepted by pre-Windows95 FAT file system.

Those filenames are limited to eight characters + . + three letter file extension. The period character cannot be in any other location.

12

ahh ok. something i did figure out today while digging thru the src, the button_scr_handler uses cmdln_args_string_by_position(uint32_t pos, uint32_t max_len, char *str) , called with sizeof(script_file) as the max_len param

void button_scr_handler(struct command_result *res){
    button_flags=0;
    char script_file[32];
    if(!cmdln_args_string_by_position(1, sizeof(script_file), script_file)){
        printf("No script file specified. Try button -h for help\r\n");
        res->error=true;
        return;
.
.

its in charge of extracting the string from the command line args

bool cmdln_args_string_by_position(uint32_t pos, uint32_t max_len, char *str){
    char c;
    uint32_t rptr=0;
    memset(str, 0x00, max_len);
    for(uint32_t i=0; i<pos+1; i++){
        if(!cmdln_consume_white_space(&rptr, false)){
            return false;
        }
        if(i!=pos){
            if(!cmdln_consume_white_space(&rptr, true)){
                return false;
            }
        } else {
            struct prompt_result result;
            if(cmdln_args_get_string(rptr, max_len, str)){
                return true;
            } else {
                return false;
            }
        }
    }
    return false;
}

come to think of it, if what you said is true we should just declare it as char[13] to begin with like ian said

1 Like
13

Of course, what you meant was to define the maximum length as a constant, and then use that, such as:

#define BP5_MAXIMUM_FILENAME_CHARACTER_COUNT 12

// +1 for trailing NULL
char script_file[BP5_MAXIMUM_FILENAME_CHARACTER_COUNT+1];
2 Likes
14

Ok guys. this one is a doozy but is working good:

I Added bulong command with bulong_scr.{h,c}
Moved button_exec to button.c as this would be used by both so didnt make sense to be in button_scr.c

I defined BP_FILENAME_MAX 12 in button.h and started using it as henrygab suggested.
I defined many other things in button.h to consolidate.

button_check_irq was replaced with button_check_press (using the new enum button_codes method)

i think i cleaned up pretty good, but i had to get rid of some warnings and make sure declarations were in the right place, and adding some includes to button.cā€¦ so yeh please check

here is the diff. i didnt put in a pull request yet:

let me know if any changes needed, but i think overall this accomplished some better structure for the button stuff, and the long press functionality is working. soā€¦ its getting there

i can put in pull request if you give me go ahead

15

I like the idea of using a define for the file length. Sometimes I add some extra (32 bytes) thinking ā€œit might be in a folderā€, but thatā€™s just wasted memory for a minor convenience. maybe 8.3 + max 8 char folder + /?> 21 bytes? or just 12 is probably fine 99% of the time.

16

This should probably go in pirate.h, as that is included in everything

17

Pushed some proposed changes here:

18

For those who havenā€™t used an analog scope to look at what really happens when a button if pushed, the following is an easy to read primer, with graphs showing what an oscilliscope sees for a random selection of buttons ā€¦ very enlightening!

The second page (linked at bottom of above) includes both hardware and software debounce options ā€¦ including the simplest timer-based debouncing function (that works well) Iā€™ve seen.

19

And what is (imho) the gold-standard arduino library bounce2 is described in another good article:

20

nice. i actually wrote up some code the other day for button_irq_callback to handle debouncing with with ā€œtime since last pressā€ checks of 20ms or so. I think Ian said the react time in the loop naturally debounces it thoughtā€¦
I assume in a lot of hardware with more complicated considerations like your articles, you would want to actually measure with a scope and even build a large dataset to see if it changes over time. If you were in a lab, measuring this for a large companies projects, you might want to see if it changes after 10,000 presses. or different environmental situations even

thanks for the infos. this skillbank article is super interesting.

ā€“
Ian: it took me a good few minutes to wrap my head around how you were using the button_script_files and button_flags arrays, but it makes perfect sense now.
I tried to clean up, and after some research, i reduced the long press threshold to 550ms. this seems to match the consensus of other projects.
I left all your comments in. There was a multiline one i think you probably dont intend to leave, but you can just remove on next commit if you merge.

I think im close! take a look