I did a little bit of testing. duty_u16 is fine. The duty_ns seems fine so far, a little bit too short.
20ns -> 12ns
100ns -> 87ns
1000ns -> 987ns
10000ns -> 9987 ns
duty_ns stays when the frequency is changed. That's fine. But it might have introduced a bug for duty() and freq(). The duty cycle changes if you change the frequency. If you have set e.g. duty(512) and freq(10000), and then go to freq(1000), the duty cycle is 32 of 1024. If you do the opposite, starting with freq(1000), duty(512), and switch to freq(10000), the output is off.
Interestingly that works fine, if you switch between lower frequencies, like 1000 and 500 Hz.

b.t.w.: I would suggest that you get yourself a simple logic analyzer probe, like the usual 10$ items sold at Amazon etc. The have a sampling rate of 25Mhz/8 channel, which is good enough for low frequency up to ~6MHz. At e.g. AliExpress, you can get 200MHz/16channel (e.g. KingstVIS) device for about 80$. When developing code like this PWM or Encoder, you need independent testing tools.

This phenomenon of changing duty when changing the frequency also exists in the master branch, but at different frequencies. Like when going form 100000 down to 10000 and 1000Hz. So to be safe, the duty cycle should be set again after changing the frequency.

The last commit makes the PWM more accurate.
It is still not ideal, but I think it's the best of possibles.

Could natives English speakers review/correct the docs/esp32/quickref.rst?

Please report about the good well-tested results.
These will be the arguments to merge into the main branch.
Thanks.

you really should consider adding a property for the maximum resolution. It is something that would be super helpful due to the resolution changes that can occur based on the frequency. something like this should also be available for duty_u16 and duty_ns

STATIC mp_obj_t machine_pwm_maximum_duty_get(mp_obj_t self_in) {
    machine_pwm_obj_t *self = MP_OBJ_TO_PTR(self_in);
    ledc_timer_config_t timer = timers[TIMER_IDX(self->mode, self->timer)];
    return MP_OBJ_NEW_SMALL_INT((1 << timer ->duty_resolution) - 1);
}

STATIC MP_DEFINE_CONST_FUN_OBJ_1(machine_pwm_maximum_duty_get_obj, machine_pwm_maximum_duty_get);

STATIC const mp_obj_property_t machine_pwm_maximum_duty_obj= {
    .base.type = &mp_type_property,
    .proxy = {(mp_obj_t)&machine_pwm_maximum_duty_get_obj,
              (mp_obj_t)mp_const_none,
              (mp_obj_t)mp_const_none},
};

@kdschlosser I agree about the actual PWM resolution, but just now
I do not want to get involved in changing the API, this will slow down the adoption of the PR.

@IhorNehrutsa Some quick checks:

When duty() is set e.g. to 512, it stays like that when the frequency is changed, e.g. i steps 100-1000-10000-100000 up and down.

The problem with changing duty comes back when using duty_u16.
Step 1: Setting duty_u16(32768) as 10 kHz -> ok.
Step 2: Changing the frequency to 1 kHz -> duty_u16() reports 2048, which matches the signal.
In the opposite direction 1kHz -> 10 kHz going then from 10k to 100k duty_u16() drops again to 2048. You do not need an oscilloscope to see the errors here, since duty_us16() reports the wrong values.

What also not works well is using duty_ns. At 1 kHz it's fine, at higher frequencies it fails. Some results:

Freq   set_duty_ns signal_duty_ns  duty_ns()
1k     100         100             107
1k     1000        999.98          1007
1k     10000       9974            9995
10k    100         no signal       0
10k    1000        37.5            781
10k    10000       599             9766

@IhorNehrutsa

I understand. It is best for another PR once things are functioning properly.

It is still not working well. Three examples:
a)

from machine import Pin, PWM
p=PWM(Pin(5, Pin.OUT), 10000)
p.duty_ns(10000)

results in a duty_ns value of 600ns instead of 10000 ns.
b)

from machine import Pin, PWM
p=PWM(Pin(5, Pin.OUT), 1000)  # start at 1000Hz
p.freq(10000)  # switch to 10 kHz
p.freq(100000)  # switch to 100 kHz

After the last command, there is no signal anymore, instead of the expected 100 kHz signal. You can only set 100kHz then after rebooting.
c)
Similar, when starting at 100kHz:

from machine import Pin, PWM
p=PWM(Pin(5, Pin.OUT), 100000, duty=512)  # start at 100 kHz -> duty 512
p.freq(10000)  # switch to 10 kHz, duty should stay at 512

The duty is set to 512 = half of the period. Which is 5µs correctly at 100kHz.
Going down, the high cycle changes to 6.25µs (or 64 of 1024) instead of 50 µs (or 512 = half period).

Some improvement about duty_ns staying stable when the frequency changes, but these sequences still fails:
a)

from machine import Pin, PWM
p=PWM(Pin(5, Pin.OUT), 1000, duty=512)  # start at 1000Hz, duty 512 (default)
p.freq(10000)  # switch to 10 kHz

When trying to change freq to 10kHz, the signal disappears. Calling p.duty(512) explicitly brings back the signal. But on the next step to 100k the signal disappears again. When stepping the frequency down it works as expected.

b)

from machine import Pin, PWM
p=PWM(Pin(5, Pin.OUT), 1000, duty_ns=5000)

This sequence locks up the PWM machine completely. To restart it, a hard reset is required.

@robert-hh
Which version are you using?

idf.py --version

ESP-IDF v4.4-dev-3235-g3e370c4296-dirty

With V4.3 other things happen:

from machine import Pin, PWM
p=PWM(Pin(5, Pin.OUT), 1000, duty_ns=5000)  # works here
p=PWM(Pin(5, Pin.OUT), 1000, duty_ns=5000)  # locks up

@robert-hh
Pin.OUT reconfigure pin as output mode.
Use simply Pin(5)

Configuring Pin as Output twice in a row is generally not a problem. If you do that outside the PWM call it works well. I see that it is a problem here, which is strange. It looks like a bug. Do you reconfigure the Pin object in your code?

This commit looks good. The tests from my various post pass now.

I agree that it is not the common case to reconfigure PWM and the Pin when changing the frequency or duty cycle. So a glitch should not harm. If re-configuring is a problem, then a hint in the docs may be helpful, hoping that someone reads it.

Two files for tests...
With pwm_test_duty.py you can see artefacts caused by reconfig the channel.
Uncomment line 465 // if ((chans[channel_idx].pin == -1) || (chans[channel_idx].timer_idx != timer_idx))
to have right form of PWM.

# pwm_test_freq.py

from utime import sleep
from machine import Pin, PWM

f_max = 1000
f_min = f_max // 5

f = f_min
delta_f = 1

pin = Pin(23)
p=PWM(pin, f, duty=512//2)
print(p)
while True:
    #p=PWM(Pin(23), f)  # ValueError: out of PWM timers
    #p=PWM(pin, f)  # ValueError: out of PWM timers
    p.freq(f)
    #print(p)
    
    sleep(10 / f)
    f += delta_f
    if f > f_max:
        delta_f = -1
    elif f < f_min:
        delta_f = 1

# pwm_test_duty.py

from utime import sleep
from machine import Pin, PWM

f = 1000
duty_min = 0
pin = Pin(23)

if 1:
    duty_max = 2**16 - 1
    d = duty_max // 4
    delta_d = 16

    p=PWM(pin, f, duty_u16=d)
    print(p)
    while True:
        #p=PWM(Pin(23), f, duty_u16=d)
        #p=PWM(pin, f, duty_u16=d)
        #p.init(duty_u16=d)
        p.duty_u16(d)
        
        sleep(1 / f)
        d += delta_d
        if d > duty_max:
            d = duty_max
            delta_d = -delta_d
        elif d < duty_min:
            d = duty_min
            delta_d = -delta_d
else:
    duty_max = 2**10 - 1
    d = duty_max // 4
    delta_d = 1

    #p=PWM(Pin(23), f, duty=d)
    p=PWM(pin, f, duty=d)
    print(p)
    while True:
        #p=PWM(Pin(23), f, duty=d)
        #p=PWM(pin, f, duty=d)
        #p.init(duty=d)
        p.duty(d)
        
        #print(p)
        sleep(2 / f)
        d += delta_d
        if d > duty_max:
            d = duty_max
            delta_d = -delta_d
        elif d < duty_min:
            d = duty_min
            delta_d = -delta_d

Pin(5) is not ideal for tests.
Pin(5) outputs PWM signal at boot.
See https://randomnerdtutorials.com/esp32-pinout-reference-gpios/

@robert-hh Do you have time to test pwm_test_freq.py and pwm_test_duty.py ?

I can do that. What do you expect from the test?

What I see is glitches when changing the frequency in pwm_test_freq.py. The driver does not wait for a full cycles to complete when changing the frequency. The trace below shows such an event. The pulse on the green track is created immediately before the frequency change. The trigger was set for a pulse <= 240µs

Changing the duty cycle looks fine. Since the code varies between the extremes, it is hard to check for gflitches. But the duty cycle distribution looks ok, and there are no frequency glitches. The one in the diagram is a test artifact.

Next, I limited the duty range to 16384 - 49151 (25% - 75%) to allow some space to glitches. But there were none.

I noticed that on soft reset PWM will not be disabled. Maybe you could add that.

I noticed that on soft reset PWM will not be disabled. Maybe you could add that.

From https://docs.micropython.org/en/latest/wipy/tutorial/reset.html?highlight=unaffected
A soft reset simply clears the state of the MicroPython virtual machine, but leaves hardware peripherals unaffected.

A solution is maybe to use try-except-finally in main.py

try:
    ...
    pwm = PWM(Pin(5))
    ...
except Exception as e:
    print(e)
    ...
finally:
    pwm.deinit()
    ...

The driver does not wait for a full cycles to complete when changing the frequency.

Changing the frequency is done in ESP-IDF ledc_timer_config().
I cann't change this.

The code below shows what is done on ESP32 soft reset. It's a little bit more than just the MP virtual machine. It does indeed not deinit UART or USB or the RAM.

soft_reset_exit:

    #if MICROPY_BLUETOOTH_NIMBLE
    mp_bluetooth_deinit();
    #endif

    machine_timer_deinit_all();

    #if MICROPY_PY_THREAD
    mp_thread_deinit();
    #endif

    gc_sweep_all();

    mp_hal_stdout_tx_str("MPY: soft reboot\r\n");

    // deinitialise peripherals
    machine_pins_deinit();
    machine_deinit();
    usocket_events_deinit();

    mp_deinit();
    fflush(stdout);
    goto soft_reset;

I have a question regarding the changes made. I feel it needs to be mentioned if how I think this things works is correct.
How many ISRs are available to be used with the PWM channels? I thought there are only 4 ISRs and if that is the case then only 4 frequencies can be used at any given time. Even tho there are 16 PWM channels you cannot have 16 different PWM frequencies going at the same time. there is a limit to only 4.

Would it be useful to provide some kind of a class/static method to return the status of the ISR's. and maybe be able to optionally pass the frequency to the method to see if there is an ISR that is running for that frequency. If no frequency is supplied then it would return the number if ISRs that are currently running.

This way if for some reason a user is dynamically creating PWM's and the frequencies are also being set dynamically an easy check can be done to make sure there is an available ISR for 1 and if all ISRs are being used to be able to know if they are able to create a PWM at a given frequency because there is an ISR already running for that frequency. While I know that exception catching can be used to handle it it's more expensive to run then simply asking if it can be made. And asking lends to cleaner code and is easier to follow.

I could be wrong in my understanding of how it works and if my understanding is not correct if there is someone willing to edjumicate (intentional) me on it I would appreciate it.

@kdschlosser
At the page PWM (pulse width modulation) pay attention to the line:

Generic ESP32 can generate 8 different PWM frequencies at the same time, ESP32-S2 and ESP32-C3 - 4 different PWM frequencies at the same time.
See example at https://docs.micropython.org/en/latest/esp32/tutorial/pwm.html

Note: PWM does not use ISR, you may use word "handler" to designate it hardware.

Please open new issue to discuss new PWM API methods, like actual PWM resolution and available handlers. This affects other hardware ports.
Thanks.

@robert-hh
Thanks for the soft reset PWM.deinit() hint. Is useful.

@dpgeorge
Hello Damien.
I think this PR is ready for merge. It is tested on ESP32.
I think that no problems with ESP32-S2 and ESP32-C3 also.
Should I squash commits into two commits (code and doc)?
Thanks.

P.S. #7806 and #7887 and #7909 may be closed.

I think this PR is ready for merge. It is tested on ESP32.
I think that no problems with ESP32-S2 and ESP32-C3 also.
Should I squash commits into two commits (code and doc)?

Thanks for the work on this! No need to squash, I'll do it during merge. Will do a review first.

Codecov Report

Merging #7907 (9ef2085) into master (4c9e17e) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7907      +/-   ##
==========================================
- Coverage   98.24%   98.23%   -0.01%     
==========================================
  Files         154      154              
  Lines       20007    20014       +7     
==========================================
+ Hits        19656    19661       +5     
- Misses        351      353       +2     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c9e17e...9ef2085. Read the comment docs.

@dpgeorge
Dear Damien.
Is it possible to reduce time of ESP32 PR's review?
May somebody else do this?
Thanks.

I've squashed this and merged it in b491967

Thank you @IhorNehrutsa for the hard work to get this done and in mainline!