-
Notifications
You must be signed in to change notification settings - Fork 15
Switch to time.monotonic_ns() when it's available #10
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
Conversation
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.
Looks awesome! Thank you!
The test will need to be moved to a test directory: https://travis-ci.com/adafruit/Adafruit_CircuitPython_Debouncer/builds/122915364#L362 Here is an example: https://github.com/adafruit/Adafruit_CircuitPython_Motor |
Sure! I'm traveling this weekend, but I will take a look when I get back.
|
Great! Thank you in advance! Hope you had a good weekend. |
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.
Thank you for your contribution! There are two Pylint complaints along with the need to move tests.py into a /tests/ directory. @tgs, if you're still available, please consider making these changes to this PR so we can get it added to the library.
…Python_Debouncer into use-monotonic-ns
Returning to this after a while, I notice that the I'm also changing a bunch of names of internal stuff to try to keep it clear what's measured in "whatever the monotonic clock gives us" (which I'll call ticks), and what's in seconds. I'm sure you'll tell me if that actually makes things more confusing! |
@tgs Thank you so much for getting this fixed up! I apologise for the delayed response - I managed to miss your updates. I'm going to have someone test this before merging, but we'll get it merged ASAP! |
No prob! Sounds good :) |
I've got two ItsyBitsy M0s set aside to run this test for a few days. I should be able to begin within the next day or two, I just need to get PCs set up to watch the log output from them during testing. |
Testing has begun, I'll report back in a few days the results. |
I do have this running side by side on two Itsy Bitsy M0s, but I am having a bit of trouble with them getting reset by static and by the PC they are plugged into. Which I assume is also resetting the "timer" for how long the issue takes to show itself. Since the PC seems to be a bit trigger happy about writing something, or accessing the drive in some way that is making the Circuit Py devices soft reboot as if a file has changed, today I am going to work on trying to set them up have write access from the python code. Then I will try to catch and save the exception message to a file so we can confirm after the fact that it's the same thing noticed originally for this issue. With that in place I will power them from the wall so it won't get interrupted by the PC again. |
Cool! My experience was that the counter would keep increasing as long as they were powered on - I didn't have to be in the python REPL or anything, and soft reboots didn't reset the counter. But that's my recollection from months ago. Just to clarify, you have two running so you can run the same test with the patched and unpatched versions of the library code? |
Ah, thank you. I'll leave them plugged in for now then rather than switch over then. That is correct, one running the patched lib and one running unpatched. |
Another update on this, I have yet to have either the master code, or the updated code actually crash. The only difference that I noticed is anecdotal only (don't have any measuring equipment hooked up or anything) but after several days of running I noticed that the master code device had to have the button pressed down for just a bit longer than the updated code in order for it to register the change from the debounce API. The PC eventually ended up resetting both devices again, and after the reset there is no more noticeable difference in the time it takes for the button press to register. I'm going to move the testing devices off the PC to power from the wall. I won't be able to see the stack trace if/when it crashes but it should not get reset nearly as aggressively. But I am curious to see if after a longer time the difference between the two will get more pronounced and if I can manage to see the master code one crash. |
I do still have both Itsy Bitsy M0s running. I did have a few more unrelated resets shortly after the last post and I moved them to a different outlet. It's run for about 4 weeks solid with no resets or any other issues. I've still never had either device (main branch / edited code) crash. It does seem like the edited code reacts a bit quicker to the button presses than the main branch code starting after a few days of running. But both have slowed over time as well. |
I resolved the merge conflict. However, this PR is now failing on the Black check. Running Black is trivial, but is required for this PR to pass. @tgs Are you available to update the PR? I'm happy to explain how to run Black. |
Hi @kattni, I will take a look at the formatting problem. Thanks for fixing the merge conflict! |
@tgs Great thanks! I'll get this merged! Thank you for your patience and perseverance! |
Thanks, that's great! I appreciate your help :) |
Updating https://github.com/adafruit/Adafruit_CircuitPython_RGB_Display to 3.9.2 from 3.9.1: > Merge pull request adafruit/Adafruit_CircuitPython_RGB_Display#75 from chfw/master Updating https://github.com/adafruit/Adafruit_CircuitPython_Debouncer to 1.3.2 from 1.3.1: > Merge pull request adafruit/Adafruit_CircuitPython_Debouncer#10 from tgs/use-monotonic-ns > Merge pull request adafruit/Adafruit_CircuitPython_Debouncer#18 from adafruit/setup-py-disabled
Hi again!
Here's the work I've done so far. I wrote tests to make sure I wasn't introducing breaking changes (that I could think of... do you see any?). There doesn't seem to be a CircuitPython standard testing library, so I just wrote some minimal version inside the
tests.py
file. If you don't want to merge in the tests, that would be OK.After 3 days of on-time, the current version of the debouncer code fails test_simple on line 25 - the time returned by monotonic() hasn't ticked yet in the .02 seconds of sleep. The new version passes all the tests. I've run it on an ItsyBitsy M0 Express at boot and at 2 and 3 days, and a Trinket M0 at boot (where it doesn't have
monotonic_ns
).Let me know what you think!
This should close issue #9.