From 4fdf518251dd0751cf4e9b06b7ffb2b944e7d15c Mon Sep 17 00:00:00 2001
From: Thea Flowers <theaflowers@google.com>
Date: Tue, 22 Oct 2019 10:37:36 -0700
Subject: [PATCH] Track unadjusted PWM duty cycle to avoid accumulating
 conversion errors

Fixes #2086

When the frequency of a `PWMOut` is change it re-sets the PWM's duty cycle as
well, since the registers have to be re-calculated based on the new frequency.
Unfortunately, `common_hal_pulseio_pwmout_get_duty_cycle`
will return a value very close to, but not exactly, the value passed to `common_hal_pulseio_pwmout_set_duty_cycle`. If the frequency is modified
without the calling code also re-setting the duty cycle then the duty cycle
will decay over time. This fixes that problem by tracking the unadjusted duty
cycle and re-setting the duty cycle to that value when the frequency is changed.
---
 ports/atmel-samd/common-hal/pulseio/PWMOut.c | 11 +++++++++--
 ports/atmel-samd/common-hal/pulseio/PWMOut.h |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/ports/atmel-samd/common-hal/pulseio/PWMOut.c b/ports/atmel-samd/common-hal/pulseio/PWMOut.c
index 6dcace21fd..92305161f4 100644
--- a/ports/atmel-samd/common-hal/pulseio/PWMOut.c
+++ b/ports/atmel-samd/common-hal/pulseio/PWMOut.c
@@ -135,6 +135,7 @@ pwmout_result_t common_hal_pulseio_pwmout_construct(pulseio_pwmout_obj_t* self,
                                                     bool variable_frequency) {
     self->pin = pin;
     self->variable_frequency = variable_frequency;
+    self->duty_cycle = duty;
 
     if (pin->timer[0].index >= TC_INST_NUM &&
         pin->timer[1].index >= TCC_INST_NUM
@@ -322,6 +323,13 @@ void common_hal_pulseio_pwmout_deinit(pulseio_pwmout_obj_t* self) {
 }
 
 extern void common_hal_pulseio_pwmout_set_duty_cycle(pulseio_pwmout_obj_t* self, uint16_t duty) {
+    // Store the unadjusted duty cycle. It turns out the the process of adjusting and calucating
+    // the duty cycle here and reading it back is lossy - the value will decay over time.
+    // Track it here so that if frequency is changed we can use this value to recalcuate the
+    // proper duty cycle.
+    // See https://github.com/adafruit/circuitpython/issues/2086 for more details
+    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;
@@ -415,7 +423,6 @@ void common_hal_pulseio_pwmout_set_frequency(pulseio_pwmout_obj_t* self,
             break;
         }
     }
-    uint16_t old_duty = common_hal_pulseio_pwmout_get_duty_cycle(self);
     if (t->is_tc) {
         Tc* tc = tc_insts[t->index];
         uint8_t old_divisor = tc->COUNT16.CTRLA.bit.PRESCALER;
@@ -450,7 +457,7 @@ void common_hal_pulseio_pwmout_set_frequency(pulseio_pwmout_obj_t* self,
         #endif
     }
 
-    common_hal_pulseio_pwmout_set_duty_cycle(self, old_duty);
+    common_hal_pulseio_pwmout_set_duty_cycle(self, self->duty_cycle);
 }
 
 uint32_t common_hal_pulseio_pwmout_get_frequency(pulseio_pwmout_obj_t* self) {
diff --git a/ports/atmel-samd/common-hal/pulseio/PWMOut.h b/ports/atmel-samd/common-hal/pulseio/PWMOut.h
index eef653bcb2..09abda8196 100644
--- a/ports/atmel-samd/common-hal/pulseio/PWMOut.h
+++ b/ports/atmel-samd/common-hal/pulseio/PWMOut.h
@@ -36,6 +36,7 @@ typedef struct {
     const mcu_pin_obj_t *pin;
     const pin_timer_t* timer;
     bool variable_frequency;
+    uint16_t duty_cycle;
 } pulseio_pwmout_obj_t;
 
 void pwmout_reset(void);