Skip to content

Add delay to button presses in rgb_display_pillow_animated_gif.py #105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

nerdcorenet
Copy link
Contributor

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).

Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you reduce the delay down to something like 0.2? A half second can seem like a lot when trying to scroll through multiple images quickly.

@FoamyGuy
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

FoamyGuy commented May 2, 2022

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.

@FoamyGuy FoamyGuy closed this May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants