Fix for issue #4937 - Implement minimum PulseOut time

[DavePutz]

Since PWM is being used for PulseOut, there can never be a pulseout pulse that is less than the 'high' portion of a PWM pulse. The hang reported in issue #4937 was due to the timer being set to less than the time required for a single PWM pulse. This fix calculates a minimum pulseout pulse based on the selected PWM frequency and enforces no timer delays less than that value.

Fixes #4937.

[dmcomm]

Hi, I guess you missed my update on the issue. And I posted another example just now.

[DavePutz]

@dmcomm - There was an issue when the timer interval being used to time the pulses was too short. I've made a modification to correct that. If you have a chance could you test again (with the fix I was successful running your newest test script).

[dmcomm]

Sounds good! I don't have the build system set up (my comment on this pull request was only based on the description).

[dhalbert]

@dmcomm - We'll wait for your testing before merging.

[dmcomm]

OK I'll get back to you on that.

[dhalbert]

@dmcomm you can get the already-built artifacts here: (got to here from the checks):
https://github.com/adafruit/circuitpython/pull/4964/checks?check_run_id=3002465675

Click on the Artifacts drop down which is toward the right:

[dmcomm]

• My example with the countdown now runs to completion.
• PulseOut crashes with the same symptoms if the first pulse is less than 40us, at both 38kHz and 500kHz.
• I retested the first example in PulseOut.send sometimes leaving the line high afterwards #4908 but changed to 500kHz, and that was OK for as long as I watched it for.
• If we want to guarantee that a pulse is output for too-small values, stretching to half a cycle doesn't achieve that. I expect a full cycle is needed.

[DavePutz]

@dmcomm, a PWM 'cycle' includes both high and low states, whereas a pulsein 'pulse' is only one or the other. That is the reason for using a half-cycle for PulseOut. I will check on the short initial pulse, I did not test for that.

…

On Thu, Jul 8, 2021 at 5:06 AM dmcomm ***@***.***> wrote: - My example with the countdown now runs to completion. - PulseOut crashes with the same symptoms if the first pulse is less than 40us, at both 38kHz and 500kHz. - I retested the first example in #4908 <#4908> but changed to 500kHz, and that was OK for as long as I watched it for. - If we want to guarantee that a pulse is output for too-small values, stretching to half a cycle doesn't achieve that. I expect a full cycle is needed. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4964 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFNJKESDJLUTVGPTE5TIY5LTWV2CRANCNFSM473BLUJQ> .

[dmcomm]

I meant that experimentally, a duration of 13us doesn't guarantee that a pulse will be output at 38kHz 50%. I thought that might be because the 13us window fell on the wrong phase of the PWM cycle, but PulseOut timing isn't that precise anyway, so maybe not.

[DavePutz]

Changes made to correct handling of short initial pulses as well as (hopefully) improve short pulse accuracy somewhat.

[dmcomm]

Great, I haven't managed to crash it!

A full PWM cycle still seems to better. Requesting 1ms pulses at 1kHz gives a good result:

Requesting 500us gives unwanted behaviour like this:

Similarly at 38kHz with 26/13us.

import array
import board
import time
import pulseio
import pwmio

pwm = pwmio.PWMOut(board.GP0, frequency=1000, duty_cycle=0x8000)
pulseOut = pulseio.PulseOut(pwm)
x = 500
pulses = array.array('H', [x,4*x] * 8)

while True:
    time.sleep(5)
    pulseOut.send(pulses)

[tannewt]

@DavePutz Any idea what could cause the second screen? Should we just merge this as-is?

[dmcomm]

I propose making the minimum pulse time a whole PWM cycle instead of half.

[DavePutz]

@tannewt - let's not merge this yet. I'll see if I can get things to have more consistent timing.

…

On Thu, Jul 15, 2021 at 2:37 PM dmcomm ***@***.***> wrote: I propose making the minimum pulse time a whole PWM cycle instead of half. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4964 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFNJKEVBLE5ES4OYMD3NGNLTX42H3ANCNFSM473BLUJQ> .

[DavePutz]

@dmcomm - I agree with your proposal; I changed the minimum pulse to be a whole PWM cycle and it helps. Also, I'm not sure if your excellent test scripts are just to show a condition; but if you want more accurate pulseout timings on the RP2040 you should consider changing the PWM duty_cycle value to 65535. That helps the pulse consistency, particularly for shorter pulses. @tannewt I think we can merge this one now.

[dmcomm]

@DavePutz thanks! Yes, these scripts are just for illustration. I did previously try to do non-modulated output with a high frequency and 100% duty cycle, but my signal was too fast for that, so using PIO now.

[tannewt]

Looks good! Thanks @DavePutz. CI issues are unrelated so I'll merge now.