Bug in indev_button_proc() misses buttons that are pressed for a single callback

Bug (archived)
1

The logic in indev_button_proc() has a flaw in that it assumes a button will always be pressed for at least two calls to read the button state.

(For an actual physical button, this is a reasonable assumption, but for things like faking a button press in development until the actual hardware is available, it will not necessarily be true.)

/*Still the same point is pressed*/
if(i->proc.types.pointer.last_point.x == i->proc.types.pointer.act_point.x &&
   i->proc.types.pointer.last_point.y == i->proc.types.pointer.act_point.y && data->state == LV_INDEV_STATE_PR) {
    indev_proc_press(&i->proc);
}
else {
    /*If a new point comes always make a release*/
    indev_proc_release(&i->proc);
}

The press is processed only after it is not considered a new point, so if the next time the button callback is sampled it must still be “down” to register.

Adding the following code after the call to indev_proc_release() appears to fix it. (I have not made an effort to optimize the code with this new logic, this was just a quick test.)

    if( data->state == LV_INDEV_STATE_PR )
    {
        indev_proc_press(&i->proc);
    }
2

Hi,

I’ve reviewed that function and made some adjustments. Could you try it works well?

static void indev_button_proc(lv_indev_t * i, lv_indev_data_t * data)
{
    /* Die gracefully if i->btn_points is NULL */
    if(i->btn_points == NULL) {
        LV_LOG_WARN("indev_button_proc: btn_points was  NULL");
        return;
    }

    lv_coord_t x = i->btn_points[data->btn_id].x;
    lv_coord_t y = i->btn_points[data->btn_id].y;

    /*If a new point comes always make a release*/
    if(data->state == LV_INDEV_STATE_PR) {
        if(i->proc.types.pointer.last_point.x != x ||
           i->proc.types.pointer.last_point.y != y) {
            indev_proc_release(&i->proc);
        }
    }

    if(indev_reset_check(&i->proc)) return;

    /*Save the new points*/
    i->proc.types.pointer.act_point.x = x;
    i->proc.types.pointer.act_point.y = y;

    if(data->state == LV_INDEV_STATE_PR) indev_proc_press(&i->proc);
    else indev_proc_release(&i->proc);

    if(indev_reset_check(&i->proc)) return;

    i->proc.types.pointer.last_point.x = i->proc.types.pointer.act_point.x;
    i->proc.types.pointer.last_point.y = i->proc.types.pointer.act_point.y;
}
3

Great, thanks!

Line #10 the “.x” needs to be “.y”.

And I think in this code:
if(data->state == LV_INDEV_STATE_PR) indev_proc_press(&i->proc);
else indev_proc_press(&i->proc);

The else is meant to be indev_proc_release()?

4

Both are correct, I’ve updated the code accordingly.

5

Thanks! Will let me remove my hacky work-around once it makes it into a release. :slight_smile:

6

I’ve added a fix in master.

1 Like
7

Great, thank you! I’ve tried the master branch and it works perfectly!

I know this was an edge case (using a character received from a serial port to set a one-sample “key down” state to simulate the future physical button), so I appreciate you fixing it! :slight_smile:

1 Like