Optimise machine.time_pulse_us implementation for code size, and switch esp8266 port to use it
#17346
Conversation
|
Code size report: |
|
It's really hard to pass CI these days 😅 |
e6464be to
04226d0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #17346 +/- ##
=======================================
Coverage 98.54% 98.54%
=======================================
Files 169 169
Lines 21898 21903 +5
=======================================
+ Hits 21579 21584 +5
Misses 319 319 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This implementation is based on the esp8266 custom implementation, and further optimised for size and accuracy. Testing on PYBD_SF2 and RPI_PICO2_W shows that it is at least as good as the original implementation in performance. Signed-off-by: Damien George <damien@micropython.org>
Testing shows that for frequencies which the esp8266 can handle -- up to about 1kHz -- `machine.time_pulse_us()` now gives more accurate results. Prior to this commit it would measure on average about 1us lower, but now the average is much closer to the true value. For example a pulse that is 1000us long, it would measure between 998 and 1000us. Now it measures between 999us and 1001us. Signed-off-by: Damien George <damien@micropython.org>
04226d0 to
c21708b
Compare
|
@robert-hh regarding your comment that I think you deleted: it should be possible to allow |
|
Yes, I delete it because at second though it was wrong. The |
Summary
The esp8266 has a custom implementation of
machine.time_pulse_us(), which is needed so it can feed the WDT during long waits for a pin change. It doesn't need to have a separate implementation, instead we can use the common one with an optional hook for a port to do work during the busy wait loops.This PR:
machine.time_pulse_us()to be more like the esp8266 version. That cuts down code size a little, and improves accuracy a little.Also, motivation comes from #16160: this PR should make that PR much simpler (and lower cost) to implement.
Testing
Tested with a range of square waves from 125Hz to 4kHz on:
Also tested the timeout argument, a value of 5000000 still takes 5s and doesn't lead to a WDT timeout on esp8266.
Trade-offs and Alternatives
This is an optimisation. There is no change in the API. As tested above, the accuracy is at least the same if not better due to better sampling of ticks and pin at the same point in the code.