Merge pull request #9343 from timchinowsky/fix-samd-pwm

Fix delays and rounding in samd PWM

Author: dhalbert

ports/atmel-samd/common-hal/pwmio/PWMOut.c

@@ -249,41 +249,34 @@ extern void common_hal_pwmio_pwmout_set_duty_cycle(pwmio_pwmout_obj_t *self, uin
     // Track it here so that if frequency is changed we can use this value to recalculate the
     // proper duty cycle.
     // See https://github.com/adafruit/circuitpython/issues/2086 for more details
-    self->duty_cycle = duty;
 
+    self->duty_cycle = duty;
     const pin_timer_t *t = self->timer;
     if (t->is_tc) {
         uint16_t adjusted_duty = tc_periods[t->index] * duty / 0xffff;
+        if (adjusted_duty == 0 && duty != 0) {
+            adjusted_duty = 1; // prevent rounding down to 0
+        }
         #ifdef SAMD21
         tc_insts[t->index]->COUNT16.CC[t->wave_output].reg = adjusted_duty;
         #endif
         #ifdef SAM_D5X_E5X
         Tc *tc = tc_insts[t->index];
-        while (tc->COUNT16.SYNCBUSY.bit.CC1 != 0) {
-        }
         tc->COUNT16.CCBUF[1].reg = adjusted_duty;
         #endif
     } else {
         uint32_t adjusted_duty = ((uint64_t)tcc_periods[t->index]) * duty / 0xffff;
+        if (adjusted_duty == 0 && duty != 0) {
+            adjusted_duty = 1; // prevent rounding down to 0
+        }
         uint8_t channel = tcc_channel(t);
         Tcc *tcc = tcc_insts[t->index];
-
-        // Write into the CC buffer register, which will be transferred to the
-        // CC register on an UPDATE (when period is finished).
-        // Do clock domain syncing as necessary.
-
-        while (tcc->SYNCBUSY.reg != 0) {
-        }
-
-        // Lock out double-buffering while updating the CCB value.
-        tcc->CTRLBSET.bit.LUPD = 1;
         #ifdef SAMD21
         tcc->CCB[channel].reg = adjusted_duty;
         #endif
         #ifdef SAM_D5X_E5X
         tcc->CCBUF[channel].reg = adjusted_duty;
         #endif
-        tcc->CTRLBCLR.bit.LUPD = 1;
     }
 }
 

tests/circuitpython-manual/pwmio/README.md

@@ -0,0 +1,39 @@
+# PWM testing
+
+This directory contains tools for testing CircuitPython's PWM API.  Running the tests involves three steps:
+
+1.  [CircuitPython PWM test code `code.py`](code.py) is run on the board to be tested.
+2.  As the code runs, the state of the PWM signal is logged by a logic analyzer (I used a Saleae Logic Pro 8).
+3.  Data collected by the logic analyzer is analyzed and plotted into a PNG image by [CPython code `duty.py`](duty.py).
+
+Here is a sample plot with key features annotated:
+
+<img src="pwm_plot_explainer.png">
+
+The CircuitPython code loops through a list of PWM frequencies ranging from 100 Hz to 10 MHz, staying at each frequency for one second.  At each frequency it repeatedly and randomly cycles through a list of duty cycles in a tight loop, updating the duty cycle as frequently as it is able.  The captured waveform is analyzed by `duty.py`, which calculates the duration and duty cycle of every observed PWM cycle and plots a point for each.
+
+## PWM expected behavior
+
+These tests can be used to assess how well the PWM API delivers expected behavior, as outlined below:
+
+1.  A PWM signal has a period (defined as the time between rising edges) and a duty cycle (defined as the ratio of the time between a rising edge and the next falling edge, divided by the period).  In a typical application the PWM period will be constant while the duty cycle changes frequently.
+
+2.  An exception to (1) is that CircuitPython explicitly supports duty cycles of 0% and 100%, where the signal stays constant at a low/high level.  In the CP API duty cycle is always specified as a 16-bit value, where 0 corresponds to 0%, 0xFFFF corresponds to 100%, and values in between scale accordingly.
+
+3.  As a corollary to (2), PWM settings of 0 and 0xFFFF should be the ONLY settings which result in always low/always high PWM.  Other settings should always result in an oscillating signal.
+
+4.  In the PWM API the duty cycle is specified as a 16-bit value and the period is specified by a 32-bit frequency value.  A given processor may not be able to provide a signal with that precision, but will do its best to approximate what is asked for.  The actual PWM duty and frequency settings resulting from the requested parameters can be obtained from the API.
+
+5.  The user can set the duty cycle and frequency (if initialized with `variable_frequency=True`) at any time.  Changes in duty cycle and frequency should appear in the PWM signal as soon as possible after the setting function is invoked.  The execution time of API calls for setting PWM frequency and duty cycle should be as short as possible and should not depend on the frequency and duty cycle parameters.
+
+6.  Changes in duty cycle should ideally never result in output glitches -- that is, the duty cycle of the PWM signal should never take on a value which has not been set by the user.
+
+7.  Changes in frequency may (and will usually) result in a transient glitch in frequency and duty cycle.  PWM hardware is generally not designed for glitch-free frequency transitions.
+
+8.  PWM frequency and duty cycle should be jitter-free.
+
+## Examples of PWM flaws
+
+The plot at the top of this page depicts data PWM gathered from a device with an API that displays all of the expected behavior listed above.  The plots below show how the tools reveal flaws in the behavior of PWM APIs that are not as complete.
+
+<img src="pwm_flaw_explainer.png">

tests/circuitpython-manual/pwmio/code.py

@@ -0,0 +1,102 @@
+import board
+import pwmio
+import random
+import time
+import microcontroller
+import os
+import sys
+import random
+
+exponents = [
+    2,
+    2.333,
+    2.667,
+    3,
+    3.333,
+    3.667,
+    4,
+    4.333,
+    4.667,
+    5,
+    5.333,
+    5.667,
+    6,
+    6.333,
+    6.667,
+    7,
+]
+
+freqs = [int(10**f) for f in exponents]
+
+top = 65536
+den = 10
+duties = [int(top * num / den) for num in range(1, den)]
+duties = [1, 65534, 1] + duties
+freq_duration = 1.0
+duty_duration = 0.000000001
+
+print("\n\n")
+board_name = sys.implementation[2]
+
+pins = {
+    "RP2040-Zero": (("GP15", ""),),
+    "Grand Central": (("D51", "TCC"), ("A15", "TC")),
+    "Metro M0": (("A2", "TC"), ("A3", "TCC")),
+    "ESP32-S3-DevKit": (("IO6", ""),),  # marked D5 on board for XIAO-ESP32-s3
+    "Feather ESP32-S2": (("D9", ""),),
+    "XIAO nRF52840": (("D9", ""),),
+}
+
+for board_key in pins:
+    if board_key in board_name:
+        pins_to_use = pins[board_key]
+        break
+
+while True:
+    for pin_name, pin_type in pins_to_use:
+        pin = getattr(board, pin_name)
+        print('title="', end="")
+        print(f"{board_name} at {microcontroller.cpu.frequency} Hz, pin {pin_name}", end="")
+        if len(pin_type) > 0:
+            print(f" ({pin_type})", end="")
+        print('",')
+        print(f'subtitle="{freq_duration:0.1f}s per frequency",')
+        print(f'version="{sys.version}",')
+        print("freq_calls=", end="")
+        pwm = pwmio.PWMOut(pin, variable_frequency=True)
+        t0 = time.monotonic()
+        duty_time = t0 + duty_duration
+        print("(", end="")
+        offset = 0
+        increment = 1
+        for freq in freqs:
+            i = 0
+            try:
+                pwm.frequency = freq
+            except ValueError:
+                break
+            freq_time = t0 + freq_duration
+            duty_time = t0 + duty_duration
+            while time.monotonic() < freq_time:
+                j = random.randrange(0, len(duties))
+                duty = duties[j]
+                if j > 1:
+                    duty = duties[j] + offset
+                    if duty > 65533:
+                        duty -= 65533
+                pwm.duty_cycle = duty
+                offset += increment
+                if offset > 65533:
+                    offset = 0
+                while time.monotonic() < duty_time and time.monotonic() < freq_time:
+                    pass
+                duty_time += duty_duration
+                i += 1
+                if time.monotonic() > freq_time:
+                    break
+            t0 = freq_time
+            print(f"({freq}, {i/freq_duration:.0f}), ", end="")
+        print(")")
+        print("done.")
+        pwm.deinit()
+        time.sleep(5)

tests/circuitpython-manual/pwmio/duty.py

@@ -0,0 +1,135 @@
+import math
+import matplotlib.pyplot as plt
+import numpy as np
+from PIL import Image
+from PIL import Image
+from PIL import ImageFont
+from PIL import ImageDraw
+
+
+def read(
+    filename,
+    image_filename=None,
+    height=480,
+    width=640,
+    f_min=10,
+    f_max=1e8,
+    title="",
+    subtitle="",
+    version="",
+    duty_labels=(0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9),
+    freq_calls=tuple(),
+    margin=0.01,
+    duty_color=(255, 0, 0),
+    freq_color=(0, 255, 0),
+    calls_color=(0, 255, 255),
+    title_color=(255, 255, 255),
+):
+    """Read a one channel logic analyzer raw csv data file and generate a plot visualizing the PWM signal
+    captured in the file.  Each line of the file is a <time, level> pair indicating the times (in seconds)
+    at which the signal transitioned to that level.  For example:
+                           1.726313020,0
+                           1.726313052,1
+                           1.726313068,0
+                           1.727328804,1
+    """
+    left = 80
+    right = 80
+    bottom = 20
+    top = 60
+    x0 = left
+    y0 = top
+    y1 = height - bottom
+    x1 = width - right
+    rising_edge = None
+    falling_edge = None
+    pixels = np.zeros((height, width, 3), dtype=np.uint8) * 255
+    t0 = None
+    t1 = None
+    val = None
+    with open(filename, "r") as f:
+        first = True
+        for line in f:  # find min and max t, excluding first and last values
+            if val is not None:
+                if not first:
+                    if t0 is None or t < t0:
+                        t0 = t
+                    if t1 is None or t > t1:
+                        t1 = t
+                else:
+                    first = False
+            t, val = line.split(",")
+            try:
+                t = float(t)
+                val = int(val)
+            except ValueError:
+                val = None
+    print("plotting", t1 - t0, "seconds")
+
+    with open(filename, "r") as f:
+        pts = 0
+        f_log_max = int(math.log10(f_max))
+        f_log_min = int(math.log10(f_min))
+        f_log_span = f_log_max - f_log_min
+        for line in f:
+            t, val = line.split(",")
+            try:
+                t = float(t)
+                val = int(val)
+            except ValueError:
+                val = None
+            if val == 1:
+                if falling_edge is not None and rising_edge is not None:
+                    period = t - rising_edge
+                    frequency = 1 / period
+                    duty_cycle = (falling_edge - rising_edge) / period
+                    x = int((x1 - x0) * (t - t0) / (t1 - t0)) + x0
+                    y_duty = int((1 - duty_cycle) * (y1 - y0)) + y0
+                    y_freq = (
+                        int((y1 - y0) * (1 - (math.log10(frequency) - f_log_min) / f_log_span))
+                        + y0
+                    )
+                    x = max(x0, min(x, x1 - 1))
+                    y_duty = max(y0, min(y_duty, y1 - 1))
+                    y_freq = max(y0, min(y_freq, y1 - 1))
+                    pixels[y_duty, x] = duty_color
+                    pixels[y_freq, x] = freq_color
+                    pts += 1
+                rising_edge = t
+            elif val == 0:
+                falling_edge = t
+        image = Image.fromarray(pixels)
+        draw = ImageDraw.Draw(image)
+        draw.text((left - 10, top), "Duty", duty_color, anchor="rt")
+        draw.text((0, top), "Calls", calls_color, anchor="lt")
+        draw.text((width - right / 2, top), "Freq", freq_color, anchor="mt")
+
+        for duty in duty_labels:
+            draw.text(
+                (left - 10, y0 + (y1 - y0) * (1 - duty)),
+                f"{int(100*duty):d}%",
+                duty_color,
+                anchor="rm",
+            )
+        for exponent in range(f_log_min + 1, f_log_max):
+            draw.text(
+                (width - right / 2, y0 + (y1 - y0) * (1 - (exponent - f_log_min) / (f_log_span))),
+                str(10**exponent) + " Hz",
+                freq_color,
+                anchor="mm",
+            )
+        for freq, count in freq_calls:
+            draw.text(
+                (0, y0 + (y1 - y0) * (1 - (math.log10(freq) - f_log_min) / (f_log_span))),
+                f"{count} Hz",
+                calls_color,
+                anchor="lm",
+            )
+        subtitle += f", showing {pts} PWM cycles"
+        draw.text((width * 0.5, height * margin), title, title_color, anchor="mm")
+        draw.text((width * 0.5, height * 4 * margin), version, title_color, anchor="mm")
+        draw.text((width * 0.5, height * 8 * margin), subtitle, title_color, anchor="mm")
+        image.show()
+        if image_filename is not None:
+            image.save(image_filename)
+    return image