8000 machine: Add wait_opposite param to time_pulse_us func. by IhorNehrutsa · Pull Request #16160 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

machine: Add wait_opposite param to time_pulse_us func. #16160

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

machine: Add wait_opposite param to time_pulse_us func. #16160

wants to merge 1 commit into from
+30 −20

Conversation

IhorNehrutsa
Copy link
Contributor
@IhorNehrutsa IhorNehrutsa commented Nov 5, 2024 •
edited
Loading

time_pulse_us(pin, pulse_level, timeout_us=1000000, wait_opposite=false, /)

If wait_opposite is true, if the pin is initially equal to pulse_level then first
waits until the pin input becomes different from pulse_level.
Then if the current input value of the pin is different to pulse_level,
the function first waits until the pin input becomes equal to pulse_level,
then times the duration that the pin is equal to pulse_level.

The advantage is that there is no need for additional synchronization before measuring the pulse duration.
A little bit longer, but with higher accuracy.

The jitter of the pulse measurement process is about 5-15 μs on the ESP32 board.

Idia from @robert-hh #16147 (comment)

This function is used in DRAFT PR for tests
PWM from 10845 and time_hardware_pulse_us() and test from 16147. #16161
The tests results show the functionality of the function over the entire ESP32 PWM frequency range from 1Hz to 40MHz.

@github-actions GitHub Actions
Copy link
github-actions bot commented Nov 5, 2024 •
edited
Loading

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:  +104 +0.012% standard
      stm32:   +68 +0.017% PYBV10
     mimxrt:   +72 +0.020% TEENSY40
        rp2:   +56 +0.006% RPI_PICO_W
       samd:   +68 +0.025% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@IhorNehrutsa IhorNehrutsa force-pushed the time_hardware_pulse_us branch from 7a9b45d to f59833b Compare November 5, 2024 15:42
@IhorNehrutsa IhorNehrutsa mentioned this pull request Nov 5, 2024
Closed
@robert-hh
Copy link
Contributor

Thanks for mentioning me. I considered creating that feature, but as optional argument to time_pulse_us() instead of a new method. That looks like a smaller change, even if the additional code size might be the same..

@IhorNehrutsa IhorNehrutsa force-pushed the time_hardware_pulse_us branch from f59833b to e29b760 Compare November 5, 2024 22:01
@codecov Codecov
Copy link
codecov bot commented Nov 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.53%. Comparing base (2264340) to head (27486f2).
Report is 317 commits behind head on master.

Files with missing lines Patch % Lines
extmod/machine_pulse.c 71.42% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #16160      +/-   ##
==========================================
- Coverage   98.54%   98.53%   -0.01%     
==========================================
  Files         169      169              
  Lines       21877    21882       +5     
==========================================
+ Hits        21558    21561       +3     
- Misses        319      321       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@IhorNehrutsa IhorNehrutsa force-pushed the time_hardware_pulse_us branch from f25ae18 to b9d6cf0 Compare November 7, 2024 06:22
@IhorNehrutsa IhorNehrutsa force-pushed the time_hardware_pulse_us branch from b9d6cf0 to aa1cba8 Compare December 5, 2024 22:55
@IhorNehrutsa IhorNehrutsa changed the title machine: Add time_hardware_pulse_us function. machine: Add wait_opposite param to time_pulse_us func. Dec 5, 2024
@IhorNehrutsa IhorNehrutsa force-pushed the time_hardware_pulse_us branch 3 times, most recently from 90f267b to 28906bc Compare December 5, 2024 23:24
@IhorNehrutsa IhorNehrutsa force-pushed the time_hardware_pulse_us branch 2 times, most recently from dd7b953 to b6bcf72 Compare December 19, 2024 12:50
@IhorNehrutsa
Copy link
Contributor Author

Remove additionnal synchronization time_pulse_us() from tests/extmod_hardware/machine_pwm.py

@IhorNehrutsa IhorNehrutsa force-pushed the time_hardware_pulse_us branch from b6bcf72 to e2a1c3a Compare December 19, 2024 13:08
@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label Jan 23, 2025
@thomasabbott thomasabbott mentioned this pull request Feb 6, 2025
Closed
@IhorNehrutsa @robert-hh
machine: Add wait_opposite to time_pulse_us func.
27486f2 
If the pin is initially equal to *pulse_level* then first waits until
the pin input becomes different from *pulse_level*.
Then the function waits until the pin input becomes equal to *pulse_level*,
then the func counts the duration that the pin is equal to *pulse_level*.

Signed-off-by: IhorNehrutsa <Ihor.Nehrutsa@gmail.com>
Co-Authored-By: Robert Hammelrath <12476868+robert-hh@users.noreply.github.com>
@IhorNehrutsa IhorNehrutsa force-pushed the time_hardware_pulse_us branch from e2a1c3a to 27486f2 Compare March 13, 2025 09:04
@dpgeorge dpgeorge mentioned this pull request May 23, 2025
Open
@dpgeorge
Copy link
Member

After discussing with @projectgus , we feel that this is a useful enhancement but maybe not worth the cost in code size.

I think it would be possible to make this PR a lot simpler and less code size. I opened #17346 as a basis for that.

If #17346 is merged, then the additional argument added here could be generalised to a counter, eg:

machine.time_pulse_us(pin, pulse_level, timeout_us=1000000, num_levels=2)

With num_levels=2 it's the same behaviour as before. With num_levels=3 it waits for 3 level changes and that implements the behaviour in this PR. Higher values are possible, which allow more synchronisation.

@robert-hh
Copy link
Contributor

then the additional argument added here

It's not (yet) visible in PR 17346.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
extmod Relates to extmod/ directory in source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@IhorNehrutsa @robert-hh @dpgeorge
0