PWM: Reduce inconsistencies between ports 2. #12084

IhorNehrutsa · 2023-07-24T16:03:35Z

Let me continue the discussion started here: PWM: Reduce inconsistencies between ports.#10817
I express my deepest respect for all who have contributed to bringing the API to a unified form, especially @robert-hh.
I consider this discussion unfinished.

EDITED 4 November 2024:
Only 3 states are possible for the PWM pin output: LOW(duty=0%), HIGH(duty=100%), and WAVE(0%<duty<100%). WAVE is the more likely state of the PWM pin that the user wants to see. WAVE is described by two parameters: frequency and duty. Both are required for WAVE!

Several unresolved questions remain in the PWM API. I suggest discussing them one by one and fixing the result at the top of the thread.

Question 1):

Most of the ports have
Constructor:

PWM(dest, *, freq, duty_u16, duty_ns, invert)

Method:

init(*, freq, duty_u16, duty_ns)

The difference between ports is that when executed

pwm = PWM(Pin(PIN))

Variant 1) Unless freq and duty is chosen by the user there must be no signal on the pin output.
Variant 2) PWM WAVE starts with some predefined frequency and duty immediately.

Variant 1)
I see disadvantages in two hypothetical "bad" codes:

case 1) Reset MCU.

pin = Pin(2, Pin.OUT, value=1) # output is HIGH
pwm = PWM(Pin(2)) # output save previous state HIGH
pwm.init(duty=0) # output save previous state HIGH, but user expects LOW 
pwm.init(duty_u16=0) # output save previous state HIGH, but user expects LOW 
pwm.init(duty_ns=0) # output save previous state HIGH, but user expects LOW 
pwm.init(freq=1000) # output is LOW as the user expects

The user expects LOW because 0% duty means LOW for any frequency in the electronic world.

Unless freq and duty is chosen by the user there must be no signal.

BUT THERE IS HIGH SIGNAL during 4 lines of the "bad" code.
Output becomes LOW as the user expects after 5 lines of the "bad" code.
"Good" code is:

pin = Pin(2, Pin.OUT, value=1) 
pwm = PWM(Pin(2), freq=ANY, duty_u16=0)

case 2) Reset MCU.

pin = Pin(2, Pin.OUT, value=0) # output is LOW
pwm = PWM(Pin(2)) # output save previous state LOW
pwm.init(duty=2**10) # output save previous state LOW, but user expects HIGH 
pwm.init(duty_u16=2**16) # output save previous state LOW, but user expects HIGH
pwm.init(duty_ns=XXXX) # the user can't set 100% duty without a certain frequency
pwm.init(freq=1000) # output is HIGH as the user expects

The user expects HIGH because 100% duty means HIGH for any frequency.
THERE IS LOW SIGNAL during 4 lines of the "bad" code.
Output becomes HIGH as the user expects after 5 lines of the "bad" code.
pwm.init(duty_ns=) - can't set 100% duty without the frequency.
"Good" code is:

pin = Pin(2, Pin.OUT, value=0) 
pwm = PWM(Pin(2), freq=ANY, duty_u16=2**16)

Variant 2)

case 1) Reset MCU.

pin = Pin(2, Pin.OUT, value=1) # output is HIGH
pwm = PWM(Pin(2)) # output WAVE that is native and usual to PWM
pwm.init(duty=0) # output is LOW as user expects
pwm.init(duty_u16=0) # output is LOW as user expects
pwm.init(duty_ns=0) # output is LOW as user expects

There is a glitch in line 2.
"Good" code is:

pin = Pin(2, Pin.OUT, value=1) 
pwm = PWM(Pin(2), duty_u16=0)

case 2) Reset MCU.

pin = Pin(2, Pin.OUT, value=0) # output is LOW
pwm = PWM(Pin(2)) # output WAVE that is native and usual to PWM
pwm.init(duty=2**10) # output is HIGH as user expects
pwm.init(duty_u16=2**16) # output is HIGH as user expects
pwm.init(duty_ns=XXXX) # output is HIGH as user expects

There is a glitch in line 2.
"Good" code is:

pin = Pin(2, Pin.OUT, value=0) 
pwm = PWM(Pin(2), duty_u16=2**16)

Let's consider new user:

Variant 1)
pwm = PWM(Pin(2)) - output pin save previous state, a user is confused
print(pwm) - the user try to find out the reason
output is >>> PWM(Pin(2)) - nothing else new, the user is confused still
pwm.freq() or pwm.duty() - raise an exception, the user is confused again
one of pwm.int(freq=123) or pwm.freq(123) or pwm.int(duty=456) or pwm.duty(456) - output pin save previous state, the user is angry
when both freq and duty is chosen by one of pwm.int(duty=456) or pwm.duty(456) or pwm.int(freq=123) or pwm.freq(123) - resolved, user is happy

Variant 2)
pwm = PWM(Pin(2)) - output WAVE with unexpected freq and duty, a user is confused
print(pwm) - the user try to find out the reason
output is >>> PWM(Pin(2), freq=5000, duty_u16=32768) - Ok, according to an oscillograph
pwm.init(freq=my_freq, duty=my_duty) - resolved, user is happy

Conclusion:

PWM(Pin(2))

Variant 1) allows surprises of unexpectedness to the user
The user sees PWM statement in code and the user sees the previous state on the pin.
Variant 2) has the glitch in "bad" code, but works as expected in all lines of the code,
The user sees PWM statement in code and the user sees PWM WAVE on the pin.

I vote for Variant 2).

Question 2):

duty_u16 0-65535 vs 0-65536
please after a solution to Question 1)

Dear maintainers, please make the volitional decision.

To the attention of @dpgeorge @projectgus @robert-hh @jonathanhogg @yoann-darche @Ayush1325 @rkompass @karlp
@andrewleech @pdg137
and all interested parties.

The text was updated successfully, but these errors were encountered:

IhorNehrutsa · 2023-07-24T16:10:52Z

TODO:

Correct common machine.PWM documentation.

TODO in ports:

ESP2866:

Add invert arg by pwm_set_channel_invert()
Correct duty cycle range to 0-65536

robert-hh · 2023-07-24T19:00:51Z

Thanks for the pointer. Indeed, the value range could be limited. ATM it accepts any value, but out-of-band values work as one could assume. Values < 0 behave like 0, and values > 65536 like 65536.
I did intentionally not add the invert method. Could be done. But the esp8266's PWM is anyhow very limited, so it seemed not worth the effort.

rkompass · 2023-07-24T20:48:28Z

I did intentionally not add the invert method. Could be done. But the esp8266's PWM is anyhow very limited, so it seemed not worth the effort.

It would allow a motor driver to be board-agnostic if it was there. But if it's too complicated: One could try that argument and catch the exception..

IhorNehrutsa · 2024-11-04T11:34:02Z

The start message is changed, please reread.
Please discuss only Question 1).

robert-hh · 2024-11-04T11:44:22Z

Sorry. I'm not willing to continue this discussion and testing PR 10854 over and over again. Besides the start-up behavior, it is an excellent contribution and compatible with the other ports.

IhorNehrutsa · 2024-11-04T12:06:53Z

@robert-hh Sorry, I didn't mean to offend you.
I see disadvantages in variant 1), but I don't know the advantages which overload these disadvantages.

yoann-darche · 2024-11-04T15:33:54Z

From my perspective, after working with the ESP32 port, here is my position:

When the user writes p = PWM(Pin(2)), he expects to see some activity on Pin 2 that resembles a PWM signal. Since neither frequency nor duty cycle was specified, MicroPython assigns default values for these parameters.

If the user wants to update the parameters, he can use p = PWM(Pin(2), freq=500), which sets the frequency to 500 Hz on Pin 2.

This works because the GPIO pin is the ineternal ID, not the Python object itself. Reusing the same pin in code simply updates the parameters associated with that pin.

robert-hh · 2024-11-04T16:06:21Z

Sorry, I didn't mean to offend you.

Do not worry. I'm just tired about this discussion. I think all is said, but maybe not by everyone. It's up to Damien to decide, which way to go.
Besides that, Damien (kind of) asked me to change back the duty_u16() value, at which the output is at constant high level to 65535. That is in #15153. You may adapt the code in your PR #10854. Maybe it's just setting the cap value. Since ATM it generates proper pulses for duty_16(65535), duty_u16(1) and duty_u16(32768), the setting for duty_u16(65535) will be lost.
Actually this device has the best resolution for PWM with just 500ps difference between high and low pulse at duty_u16(32768) and 1 kHz frequency.

IhorNehrutsa · 2024-11-04T17:09:55Z

@robert-hh Ok. I have something to say about the duty_u16 0-65535 vs 0-65536, but after decision Question 1).

IhorNehrutsa · 2024-11-04T17:14:47Z

@yoann-darche What ESP32 board are you using? May you test #10854 ?

robert-hh · 2024-11-07T11:36:45Z

Tests of this PR against the script from PR #16147 improve. With the most recent push the special if.. for ESP32 is not needed any more. It is now possible to specify freq and duty_u16 separately. Intermittent problems exist still with duty_16 == 0 and 65535. With duty_u16 == 65536 the result is even worse.

Test platform: MicroPython v1.25.0-preview.20.g9d9471f65 on 2024-11-07; Generic ESP32 module with ESP32

Max duty 65535:

freq=50    duty_u16=0     : True True : 0 True
freq=50    duty_u16=6553  : True True : 0 True 1 True
freq=50    duty_u16=16383 : True True : 0 True 1 True
freq=50    duty_u16=32767 : True True : 0 True 1 True
freq=50    duty_u16=49151 : True True : 0 True 1 True
freq=50    duty_u16=58981 : True True : 0 True 1 True
freq=50    duty_u16=65535 : True True :        1 False
freq=100   duty_u16=0     : True True : 0 True
freq=100   duty_u16=6553  : True True : 0 True 1 True
freq=100   duty_u16=16383 : True True : 0 True 1 True
freq=100   duty_u16=32767 : True True : 0 True 1 True
freq=100   duty_u16=49151 : True True : 0 True 1 True
freq=100   duty_u16=58981 : True True : 0 True 1 True
freq=100   duty_u16=65535 : True True :        1 False
freq=500   duty_u16=0     : True True : 0 True
freq=500   duty_u16=6553  : True True : 0 True 1 True
freq=500   duty_u16=16383 : True True : 0 True 1 True
freq=500   duty_u16=32767 : True True : 0 True 1 True
freq=500   duty_u16=49151 : True True : 0 True 1 True
freq=500   duty_u16=58981 : True True : 0 True 1 True
freq=500   duty_u16=65535 : True True :        1 True
freq=1000  duty_u16=0     : True True : 0 True
freq=1000  duty_u16=6553  : True True : 0 True 1 True
freq=1000  duty_u16=16383 : True True : 0 True 1 True
freq=1000  duty_u16=32767 : True True : 0 True 1 True
freq=1000  duty_u16=49151 : True True : 0 True 1 True
freq=1000  duty_u16=58981 : True True : 0 True 1 True
freq=1000  duty_u16=65535 : True True :        1 True
freq=2000  duty_u16=0     : True True : 0 True
freq=2000  duty_u16=6553  : True True : 0 True 1 True
freq=2000  duty_u16=16383 : True True : 0 True 1 True
freq=2000  duty_u16=32767 : True True : 0 True 1 True
freq=2000  duty_u16=49151 : True True : 0 True 1 True
freq=2000  duty_u16=58981 : True True : 0 True 1 True
freq=2000  duty_u16=65535 : True True :        1 True
freq=5000  duty_u16=0     : True True : 0 True
freq=5000  duty_u16=6553  : True True : 0 True 1 True
freq=5000  duty_u16=16383 : True True : 0 True 1 True
freq=5000  duty_u16=32767 : True True : 0 True 1 True
freq=5000  duty_u16=49151 : True True : 0 True 1 True
freq=5000  duty_u16=58981 : True True : 0 True 1 True
freq=5000  duty_u16=65535 : True True :        1 True
freq=10000 duty_u16=0     : True (0, 65520) : 0 False
freq=10000 duty_u16=6553  : True True : 0 True 1 True
freq=10000 duty_u16=16383 : True True : 0 True 1 True
freq=10000 duty_u16=32767 : True True : 0 True 1 True
freq=10000 duty_u16=49151 : True True : 0 True 1 True
freq=10000 duty_u16=58981 : True True : 0 True 1 True
freq=10000 duty_u16=65535 : True True :        1 True

Max duty 65536:

freq=50    duty_u16=0     : True True : 0 True
freq=50    duty_u16=6553  : True True : 0 True 1 True
freq=50    duty_u16=16384 : True True : 0 True 1 True
freq=50    duty_u16=32768 : True True : 0 True 1 True
freq=50    duty_u16=49152 : True True : 0 True 1 True
freq=50    duty_u16=58982 : True True : 0 True 1 True
freq=50    duty_u16=65536 : True True : 0 True 1 (20000, -2)
freq=100   duty_u16=0     : True True : 0 True
freq=100   duty_u16=6553  : True True : 0 True 1 True
freq=100   duty_u16=16384 : True True : 0 True 1 True
freq=100   duty_u16=32768 : True True : 0 True 1 True
freq=100   duty_u16=49152 : True True : 0 True 1 True
freq=100   duty_u16=58982 : True True : 0 True 1 True
freq=100   duty_u16=65536 : True True : 0 True 1 (10000, -1)
freq=500   duty_u16=0     : True True : 0 True
freq=500   duty_u16=6553  : True True : 0 True 1 True
freq=500   duty_u16=16384 : True True : 0 True 1 True
freq=500   duty_u16=32768 : True True : 0 True 1 True
freq=500   duty_u16=49152 : True True : 0 True 1 True
freq=500   duty_u16=58982 : True True : 0 True 1 True
freq=500   duty_u16=65536 : True True : 0 True 1 (2000, -1)
freq=1000  duty_u16=0     : True True : 0 True
freq=1000  duty_u16=6553  : True True : 0 True 1 True
freq=1000  duty_u16=16384 : True True : 0 True 1 True
freq=1000  duty_u16=32768 : True True : 0 True 1 True
freq=1000  duty_u16=49152 : True True : 0 True 1 True
freq=1000  duty_u16=58982 : True True : 0 True 1 True
freq=1000  duty_u16=65536 : True True : 0 True 1 (1000, -1)
freq=2000  duty_u16=0     : True True : 0 True
freq=2000  duty_u16=6553  : True True : 0 True 1 True
freq=2000  duty_u16=16384 : True True : 0 True 1 True
freq=2000  duty_u16=32768 : True True : 0 True 1 True
freq=2000  duty_u16=49152 : True True : 0 True 1 True
freq=2000  duty_u16=58982 : True True : 0 True 1 True
freq=2000  duty_u16=65536 : True True : 0 True 1 (500, -1)
freq=5000  duty_u16=0     : True True : 0 True
freq=5000  duty_u16=6553  : True True : 0 True 1 True
freq=5000  duty_u16=16384 : True True : 0 True 1 True
freq=5000  duty_u16=32768 : True True : 0 True 1 True
freq=5000  duty_u16=49152 : True True : 0 True 1 True
freq=5000  duty_u16=58982 : True True : 0 True 1 True
freq=5000  duty_u16=65536 : True True : 0 True 1 (200, -1)
freq=10000 duty_u16=0     : True True : 0 True
freq=10000 duty_u16=6553  : True True : 0 True 1 True
freq=10000 duty_u16=16384 : True True : 0 True 1 True
freq=10000 duty_u16=32768 : True True : 0 True 1 True
freq=10000 duty_u16=49152 : True True : 0 True 1 True
freq=10000 duty_u16=58982 : True True : 0 True 1 True
freq=10000 duty_u16=65536 : True True : 0 True 1 (100, -1)

IhorNehrutsa · 2024-11-07T11:51:39Z

@robert-hh

Tests of this PR against

Do you mean PR 10845?

IhorNehrutsa · 2024-11-07T12:29:28Z

Try to use

    time.sleep(1 / freq)

after setting PWM duty.

Note: New PWM parameters take effect in the next PWM cycle.

    pwm = PWM(2, duty=512)
    print(pwm)
    >>>PWM(Pin(2), freq=5000, duty=1024)  # the duty is not relevant
    pwm.init(freq=2, duty=64)
    print(pwm)
    >>>PWM(Pin(2), freq=2, duty=16)  # the duty is not relevant
    time.sleep(1 / 2)                # wait one PWM period
    print(pwm)
    >>>PWM(Pin(2), freq=2, duty=64)  # the duty is actual

robert-hh · 2024-11-07T12:44:34Z

Indeed PR #10854.

robert-hh · 2024-11-07T12:49:38Z

New PWM parameters take effect in the next PWM cycle.

That is a common behavior but adding this sleep does not change the results. Even time.sleep(10/freq) does not pass all test cases. You can run the test yourself.

IhorNehrutsa · 2025-02-01T20:21:04Z

Closed in favor esp32/PWM: Reduce inconsitencies between ports. #10854

IhorNehrutsa added the bug label Jul 24, 2023

IhorNehrutsa changed the title ~~PWM: Reduce inconsistencies between ports.~~ PWM: Reduce inconsistencies between ports 2. Jul 25, 2023

IhorNehrutsa mentioned this issue Nov 4, 2024

esp32/PWM: Reduce inconsitencies between ports. #10854

Open

IhorNehrutsa closed this as completed Feb 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PWM: Reduce inconsistencies between ports 2. #12084

PWM: Reduce inconsistencies between ports 2. #12084

IhorNehrutsa commented Jul 24, 2023 •

edited

Loading

IhorNehrutsa commented Jul 24, 2023 •

edited

Loading

robert-hh commented Jul 24, 2023

rkompass commented Jul 24, 2023 •

edited

Loading

IhorNehrutsa commented Nov 4, 2024 •

edited

Loading

robert-hh commented Nov 4, 2024

IhorNehrutsa commented Nov 4, 2024 •

edited

Loading

yoann-darche commented Nov 4, 2024

robert-hh commented Nov 4, 2024

IhorNehrutsa commented Nov 4, 2024

IhorNehrutsa commented Nov 4, 2024

robert-hh commented Nov 7, 2024

IhorNehrutsa commented Nov 7, 2024

IhorNehrutsa commented Nov 7, 2024

robert-hh commented Nov 7, 2024

robert-hh commented Nov 7, 2024

IhorNehrutsa commented Feb 1, 2025

PWM: Reduce inconsistencies between ports 2. #12084

PWM: Reduce inconsistencies between ports 2. #12084

Comments

IhorNehrutsa commented Jul 24, 2023 • edited Loading

Question 1):

Question 2):

IhorNehrutsa commented Jul 24, 2023 • edited Loading

robert-hh commented Jul 24, 2023

rkompass commented Jul 24, 2023 • edited Loading

IhorNehrutsa commented Nov 4, 2024 • edited Loading

robert-hh commented Nov 4, 2024

IhorNehrutsa commented Nov 4, 2024 • edited Loading

yoann-darche commented Nov 4, 2024

robert-hh commented Nov 4, 2024

IhorNehrutsa commented Nov 4, 2024

IhorNehrutsa commented Nov 4, 2024

robert-hh commented Nov 7, 2024

IhorNehrutsa commented Nov 7, 2024

IhorNehrutsa commented Nov 7, 2024

robert-hh commented Nov 7, 2024

robert-hh commented Nov 7, 2024

IhorNehrutsa commented Feb 1, 2025

IhorNehrutsa commented Jul 24, 2023 •

edited

Loading

IhorNehrutsa commented Jul 24, 2023 •

edited

Loading

rkompass commented Jul 24, 2023 •

edited

Loading

IhorNehrutsa commented Nov 4, 2024 •

edited

Loading

IhorNehrutsa commented Nov 4, 2024 •

edited

Loading