-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix #3504: Don't use time module in pew.tick() #3536
Conversation
The time.sleep() and time.monotonic() functions break the timer interrupt on which PewPew10 display relies, so we can't use them anymore. Instead I'm adding a time-keeping function to the display code itself, which then can be used in pew.tick() internally.
This is a proof of concept for the fix. If we agree to it, I will also need to add a corresponding code change to the frozen |
I replied on the issue. I suspect we won't need this. |
Fixed by #3546 |
I would like to reopen this pull request. If I avoid relying on the time.monotonic() entirely, there is zero flicker in PewPew, and I don't run into problems with lowering accuracy over time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is OK by me, since it doesn't change anything else except your own library.
If we had some kind of |
No, an important feature here is that it doesn't use the timekeeping/sleep machinery, and so doesn't trigger the flickering. |
I don't think it's the timekeeping/sleep machinery causing you problems. All of those things are still happening. I'd blame monotonic's degrading accuracy if anything. |
In any case, this solves the problem and makes it unlikely to ever come back, which is important for me from the maintenance point of view. |
The time.sleep() and time.monotonic() functions break the timer
interrupt on which PewPew10 display relies, so we can't use them
anymore. Instead I'm adding a time-keeping function to the display
code itself, which then can be used in pew.tick() internally.