search

adafruit / Adafruit_CircuitPython_RGB_Display

Drivers for RGB displays for Adafruit CircuitPython.
MIT License
129 stars 52 forks source link

Add delay to button presses in rgb_display_pillow_animated_gif.py #105

Closed nerdcorenet closed 2 years ago

nerdcorenet commented 2 years ago

This adds a slight delay to the looping in rgb_display_pillow_animated_gif.py when the user presses a back or advance button, because I found it difficult to use otherwise (skipped multiple files instead of one).

FoamyGuy commented 2 years ago

Ideally time.sleep() doesn't need to be used for button debouncing. I think it would be best to implement debouncing logic that checks the state of the button and only registered "new presses" after a release is detected (or after some timeout if you want press and hold to repeat functionality). There is a helper library for that here: https://github.com/adafruit/Adafruit_CircuitPython_debouncer but a basic version of the logic could be implemented directly in here instead of requiring the extra library as well.

I don't know for certain that it's worth making this change right away as it would be slightly more involved then just adding the sleeps like this. But worth considering IMO. With debouncing logic instead of sleeps there is no worry about the delays causing trouble for updating the display or anything else since it won't require the cpu to sleep.

nerdcorenet commented 2 years ago

I had tried to implement my own debouncing logic by tracking the previous state of the Advance and Back buttons as "AnimatedGif.advance_last" etc, but I wasn't able to easily implement it so I gave up and went with a sleep() instead.

This is by no means necessary and no hard feelings if you simply reject this PR. It's just a suggestion and @FoamyGuy has another wonderful approach to implement.

The example works okay the way it is, and it's not a button example but a RGB LCD example, so no need for any changes, really.

I chose 0.5s because some people with motor function disabilities may find a shorter delay to be too little, causing multiple images to load on each button press. I tried to find some reference to what an appropriate button delay would be for the broad public including those with motor function or other disabilities but I didn't find any simple answer or best practice; Am open to suggestions.

FoamyGuy commented 2 years ago

Closing this one for now. #107 contains a different approach to resolve the same issue using debouncing logic instead of the time sleeps.

This will prevent rapid changes to the file and not use sleep so it won't slow down the animation any.

Check it out if you have a chance @nerdcorenet and let us know if the new version is better with regards to this issue.