adafruit / Adafruit_CircuitPython_Display_Button

Press it!
MIT License
4 stars 17 forks source link

Button Registering Twice #43

Open DJDevon3 opened 11 months ago

DJDevon3 commented 11 months ago

On button press it will always register 2 presses. Attempted to change the time.sleep(1). It will just wait 1 second and print a 2nd button press anyway. If you hold down on the button it will print button presses forever. Using SpriteButton simpletest but it happens with all examples using this library. Request adding debouncer like feature with rise/fall.

Adafruit ESP32-S3 Feather with 3.5" TFT Featherwing (with touch). I've done a few customizations to get it working with the TFT Featherwing without using the featherwing library.

from adafruit_hx8357 import HX8357
import adafruit_stmpe610
from adafruit_button.sprite_button import SpriteButton

# 3.5" TFT Featherwing is 480x320
displayio.release_displays()
DISPLAY_WIDTH = 480
DISPLAY_HEIGHT = 320

# Initialize TFT Display
spi = board.SPI()
tft_cs = board.D9
tft_dc = board.D10
display_bus = displayio.FourWire(spi, command=tft_dc, chip_select=tft_cs)
display = HX8357(display_bus, width=DISPLAY_WIDTH, height=DISPLAY_HEIGHT)
display.rotation = 0
_touch_flip = (False, True)

# Initialize 3.5" TFT Featherwing Touchscreen
ts_cs_pin = digitalio.DigitalInOut(board.D6)
touchscreen = adafruit_stmpe610.Adafruit_STMPE610_SPI(
    board.SPI(),
    ts_cs_pin,
    calibration=((231, 3703), (287, 3787)),
    size=(display.width, display.height),
    disp_rotation=display.rotation,
    touch_flip=_touch_flip,
)

TEXT_WHITE = 0xFFFFFF
small_font = bitmap_font.load_font("/fonts/GoodTimesRg-Regular-16.bdf")

# --| Button Config |--
BUTTON_WIDTH = 7 * 16
BUTTON_HEIGHT = 2 * 16
BUTTON_MARGIN = 5

# Defiine the button
button = SpriteButton(
    x=BUTTON_MARGIN,
    y=BUTTON_MARGIN,
    width=BUTTON_WIDTH,
    height=BUTTON_HEIGHT,
    label="MENU",
    label_font=small_font,
    label_color=TEXT_WHITE,
    bmp_path="icons/gradient_button_0.bmp",
    selected_bmp_path="icons/gradient_button_1.bmp",
    transparent_index=0,
)

while True:
            p = touchscreen.touch_point
            if p:
                if button.contains(p):
                    button.selected = True
                    print("Button Pressed")
                    time.sleep(0.25)  # Wait a bit so we can see the button color change
                else:
                    button.selected = False  # When touch moves outside of button
            else:
                button.selected = False  # When button is released

press the button on the screen once and registers twice no matter how fast I try to tap

Button Pressed
Button Pressed

This is a problem when used with GUI navigation as it skips pages. Next Page becomes page 3 instead of 2 for example. I've attempted to use debouncer and ticks to no avail. It's like button.selected = True is keeping the button selection in a memory buffer. No matter what I do it will press twice in most circumstances.

DJDevon3 commented 11 months ago

After more investigation I believe the 2nd button press is being kept in the STMPE610 buffer. I've tried everything I can think of to prevent the double press including attempting to manually change the point location being held, immediately setting the button.selected value to false, using time.sleep(), using debounce timer, removing the button layer with pop() and remove(). There's something about the STMPE610 with adafruit_buttons that does not work well together for pagination because it's buffering the selected state when button.contains(p) is called. It will double press in almost every circumstance. This is an issue that might be unique to the TFT Featherwings with the STMPE610 chip. Unsure if this also affects PyPortal touch displays.

The full code I'm using is available on my github as Feather Weather MQTT Touch

DJDevon3 commented 11 months ago

Think I figured out a work around. I underestimated how long the cooldown would be. I thought 2-3 seconds would be adequate, I was so wrong. It took about 7-10 seconds.

Loading into my main_group can take about 10-12 seconds (if everything works correctly). Loading the background image, about 50 labels, getting weather API, and publishing to MQTT. The LAST_PRESS_TIME needs to be much longer and the selected buffer WILL eventually release itself. Because my main page is so big it takes a while to load, about 12.5 seconds. Setting a cooldown timeout to 7-10 seconds with multiple time based checking works reliably.

while (time.monotonic() - last) <= sleep_time and display.root_group is main_group:
            p = touchscreen.touch_point
            _now = time.monotonic()
            if p:
                print(f"if p: {p[0]}")
                print(f"Loading Time: {_now - LAST_PRESS_TIME}")
                if _now - LAST_PRESS_TIME > 7:
                    print(f"Now - Last Press: {(p[0], p[1], p[2])}")
                    if next_button.contains(p):
                        next_button.selected = True
                        time.sleep(0.5)
                        print("Next Pressed")
                        hide_menu()
                        main_group.remove(menu_popout_group)
                        main_group.remove(splash)
                        main_group2.append(menu_popout_group)
                        main_group2.append(splash)
                        display.root_group = main_group2

Loading Time: 12.25

On pages 2 & 3 (which are currently blank) the cooldown can be as little as 0.1 seconds. LAST_PRESS_TIME can be very short.

while (time.monotonic() - last) <= sleep_time and display.root_group is main_group2:
            p = touchscreen.touch_point
            _now = time.monotonic()
            if p:
                print(f"if p: {p[0]}")
                print(f"Loading Time: {_now - LAST_PRESS_TIME}")
                if _now - LAST_PRESS_TIME > 1:
                    print(f"Now - Last Press: {(p[0], p[1], p[2])}")
                    if next_button.contains(p):
                        next_button.selected = True
                        time.sleep(0.1)
                        print("Next Pressed")
                        hide_menu()
                        main_group2.remove(menu_popout_group)
                        main_group2.remove(splash)
                        main_group3.append(menu_popout_group)
                        main_group3.append(splash)
                        display.root_group = main_group3

Loading Time: 0.25 The cooldown period on LAST_PRESS_TIME is completely dependent on the page load time. This is not an automated thing. This is something I had to learn with much trial, error, and days of printing everything.

while (time.monotonic() - last) <= sleep_time and display.root_group is main_group3:
            p = touchscreen.touch_point
            _now = time.monotonic()
            if p:
                print(f"if p: {p[0]}")
                print(f"Loading Time: {_now - LAST_PRESS_TIME}")
                if _now - LAST_PRESS_TIME > 1:
                    print(f"Now - Last Press: {(p[0], p[1], p[2])}")
                    if next_button.contains(p):
                        next_button.selected = True
                        time.sleep(0.5)
                        print("Next Pressed")
                        hide_menu()
                        print(f"After Hide Menu {p}")
                        main_group3.remove(menu_popout_group)
                        main_group3.remove(splash)
                        main_group.append(menu_popout_group)
                        main_group.append(splash)
                        display.root_group = main_group

Loading Time: 0.25

The problem is now there are different cool down times for each menu button which provides an inconsistent user experience. I could set the cooldown to 7 seconds for button consistency but that is too slow to be practical as a user interface.

I feel like this was not the intention of how the display button library was designed to function with a touchscreen. It took me 4 days of throwing stuff at it until settling on this combination of syntax that works as intended, much slower than desired but it works.

lcmcninch commented 11 months ago

I'm not sure about the long delays you're seeing, there might be something else going on causing that.

But I have a thought about your code in general. Going back to your original code in your first post, if you hold the button, do you get repeated button presses until you release? If so, (and assuming that's not desirable either) what if you did something like this:

while True:
    p = touchscreen.touch_point
    if p:
        if button.contains(p):
            if not button.selected:
                button.selected = True
                print("Button Pressed")
                time.sleep(0.25)  # Wait a bit so we can see the button color change
        else:
            button.selected = False  # When touch moves outside of button
    else:
        button.selected = False  # When button is released

I haven't tested this code, but the idea is that after initially touching the button, it will be held in the selected state for a minimum of 0.25 seconds or longer depending on how long it is touched, but "Button Pressed" will only be printed once. Then you don't have to do any other arbitrary delays. Could that work for you?

DJDevon3 commented 11 months ago

Will have to revisit this tomorrow. Tonight I took a couple people's suggestion and started trimming it down with functions. It grew to over 1700 lines at one point. Trimmed it down to about 1000 lines. Still a lot to do but it does all work currently even though some buttons take longer to respond than others. I will test your suggestion and get back to you.

DJDevon3 commented 11 months ago

That actually made a dramatic difference in how responsive the menu buttons are. While a SpriteButton is pressed there are two states. While pressed it shows an inverted bmp. With a small page it loads so quixckly you can't even tell it's there. For a page that takes 10 seconds that inverted state button stays inverted until the entire page loads. It's now actually working as intended.

def menu_switching(current_group, prev_target, next_target, item1_target, item2_target, item3_target, item4_target):
    if menu_button.contains(p):
        if not menu_button.selected:
            menu_button.selected = True
            time.sleep(0.25)
            print("Menu Pressed")
            show_menu()
    elif prev_button.contains(p):
        if not prev_button.selected:
            prev_button.selected = True
            time.sleep(0.1)
            print("Previous Pressed")
            group_switch(current_group, prev_target)
    elif next_button.contains(p):
        if not next_button.selected:
            next_button.selected = True
            time.sleep(0.5)
            print("Next Pressed")
            group_switch(current_group, next_target)
    elif item1_button.contains(p):
        if not item1_button.selected:
            item1_button.selected = True
            time.sleep(0.25)
            print("Item 1 Pressed")
            group_switch(current_group, item1_target)
    elif item2_button.contains(p):
        if not item2_button.selected:
            item2_button.selected = True
            time.sleep(0.25)
            print("Item 2 Pressed")
            group_switch(current_group, item2_target)
    elif item3_button.contains(p):
        if not item3_button.selected:
            item3_button.selected = True
            time.sleep(0.25)
            print("Item 3 Pressed")
            group_switch(current_group, item3_target)
    elif item4_button.contains(p):
        if not item4_button.selected:
            item4_button.selected = True
            time.sleep(0.25)
            print("Item 4 Pressed")
            group_switch(current_group, item4_target)
    else:
        group_cleanup()
        hide_menu()

while True:
# about 600 lines of code here...
# example of root group switching from page 2 to any other page
while display.root_group is main_group2:
        hello_label_page2.text = "Feather Weather Page 2"
        while (time.monotonic() - last) <= sleep_time and display.root_group is main_group2:
            p = touchscreen.touch_point
            if p:
                menu_switching(main_group2, main_group, main_group3, preferences_group, wifi_settings_group, rssi_group, sys_info_group)
            else:
                # Default state always running
                group_cleanup()
        last = time.monotonic()

Was able to remove a lot of the time related code. Refactored with a rootgroup page display switching function. This functions much much better. I believe this is how it's intended to behave.

I recommend all Adafruit_Button examples be updated with the nested if not button.selected: code. It likely won't make a difference for lightly coded (almost blank) pages because they load too quickly to notice but for pages that take about 5-15 seconds to load it makes a world of difference!

Thank you @lcmcninch 🤗

Keeping this issue open for a few more days as I run some more tests through it. I'm confident with what I've seen so far this can be closed after Adafruit_Button examples are updated to reflect the needed code change.

DJDevon3 commented 10 months ago

Using it for a couple weeks and the difference in behavior is night and day. Definitely fixed. However, I would like to add an advanced simnpletest example that includes if not button.selected: because without that you'll definitely get double presses with buttons and sprite_buttons.