8000 ports/esp32: pwm: Prevent duty glitch on init. · micropython/micropython@26d4ebe · GitHub
[go: up one dir, main page]
Skip to content

Commit 26d4ebe

Browse files
karlpkarlp
committed
ports/esp32: pwm: Prevent duty glitch on init.
When a new channel was assigned, configure_channel would seet the duty to 50% before resetting to the actual user specified duty. This results in unintentional glitches. Example code ``` p1 = machine.PWM(self.pin1, self.pwm_freq, duty_u16=0) ``` This would brieflt glitch a pwm motor driver when it's created, instead of leaving it at duty=0 as intended. Instead, we calculate the duty into native units first, and apply it separately in a second step where needed. Signed-off-by: Karl Palsson <karlp@tweak.au>
1 parent e00a144 commit 26d4ebe

File tree

1 file changed

+30
-23
lines changed

1 file changed

+30
-23
lines changed

‎ports/esp32/machine_pwm.c

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ typedef struct _machine_pwm_obj_t {
118118
int duty_u10; // stored values from previous duty setters
119119
int duty_u16; // - / -
120120
int duty_ns; // - / -
121+
int channel_duty; // What we actually want to write, in esp32 units.
121122
} machine_pwm_obj_t;
122123

123124
STATIC bool is_timer_in_use(int current_channel_idx, int timer_idx);
@@ -196,7 +197,7 @@ void machine_pwm_deinit_all(void) {
196197
STATIC void configure_channel(machine_pwm_obj_t *self) {
197198
ledc_channel_config_t cfg = {
198199
.channel = self->channel,
199-
.duty = (1 << (timers[TIMER_IDX(self->mode, self->timer)].duty_resolution)) / 2,
200+
.duty = self->channel_duty,
200201
.gpio_num = self->pin,
201202
.intr_type = LEDC_INTR_DISABLE,
202203
.speed_mode = self->mode,
@@ -321,26 +322,28 @@ STATIC uint32_t get_duty_ns(machine_pwm_obj_t *self) {
321322
return duty_to_ns(self, get_duty_u16(self));
322323
}
323324

324-
STATIC void set_duty_u16(machine_pwm_obj_t *self, int duty) {
325+
STATIC void set_duty_apply(machine_pwm_obj_t *self) {
325326
pwm_is_active(self);
327+
check_esp_err(ledc_set_duty(self->mode, self->channel, self->channel_duty));
328+
check_esp_err(ledc_update_duty(self->mode, self->channel));
329+
}
330+
331+
STATIC void set_duty_u16(machine_pwm_obj_t *self, int duty) {
326332
if ((duty < 0) || (duty > UI_MAX_DUTY)) {
327333
mp_raise_msg_varg(&mp_type_ValueError, MP_ERROR_TEXT("duty_u16 must be from 0 to %d"), UI_MAX_DUTY);
328334
}
329335
ledc_timer_config_t timer = timers[TIMER_IDX(self->mode, self->timer)];
330-
int channel_duty;
331336
if (timer.duty_resolution <= UI_RES_16_BIT) {
332-
channel_duty = duty >> (UI_RES_16_BIT - timer.duty_resolution);
337+
self->channel_duty = duty >> (UI_RES_16_BIT - timer.duty_resolution);
333338
} else {
334-
channel_duty = duty << (timer.duty_resolution - UI_RES_16_BIT);
339+
self->channel_duty = duty << (timer.duty_resolution - UI_RES_16_BIT);
335340
}
336341
int max_duty = (1 << timer.duty_resolution) - 1;
337-
if (channel_duty < 0) {
338-
channel_duty = 0;
339-
} else if (channel_duty > max_duty) {
340-
channel_duty = max_duty;
342+
if (self->channel_duty < 0) {
343+
self->channel_duty = 0;
344+
} else if (self->channel_duty > max_duty) {
345+
self->channel_duty = max_duty;
341346
}
342-
check_esp_err(ledc_set_duty(self->mode, self->channel, channel_duty));
343-
check_esp_err(ledc_update_duty(self->mode, self->channel));
344347

345348
/*
346349
// Bug: Sometimes duty is not set right now.
@@ -361,7 +364,6 @@ STATIC void set_duty_u16(machine_pwm_obj_t *self, int duty) {
361364
}
362365

363366
STATIC void set_duty_u10(machine_pwm_obj_t *self, int duty) {
364-
pwm_is_active(self);
365367
if ((duty < 0) || (duty > MAX_DUTY_U10)) {
366368
mp_raise_msg_varg(&mp_type_ValueError, MP_ERROR_TEXT("duty must be from 0 to %u"), MAX_DUTY_U10);
367369
}
@@ -371,7 +373,6 @@ STATIC void set_duty_u10(machine_pwm_obj_t *self, int duty) {
371373
}
372374

373375
STATIC void set_duty_ns(machine_pwm_obj_t *self, int ns) {
374-
pwm_is_active(self);
375376
if ((ns < 0) || (ns > duty_to_ns(self, UI_MAX_DUTY))) {
376377
mp_raise_msg_varg(&mp_type_ValueError, MP_ERROR_TEXT("duty_ns must be from 0 to %d ns"), duty_to_ns(self, UI_MAX_DUTY));
377378
}
@@ -539,6 +540,17 @@ STATIC void mp_machine_pwm_init_helper(machine_pwm_obj_t *self,
539540
self->timer = TIMER_IDX_TO_TIMER(timer_idx);
540541
self->channel = CHANNEL_IDX_TO_CHANNEL(channel_idx);
541542

543+
// Note: doesn't set, just calculates...
544+
if (duty_u16 != -1) {
545+
set_duty_u16(self, duty_u16);
546+
} else if (duty_ns != -1) {
547+
set_duty_ns(self, duty_ns);
548+
} else if (duty != -1) {
549+
set_duty_u10(self, duty);
550+
} else if (self->duty_x == 0) {
551+
set_duty_u10(self, (1 << PWM_RES_10_BIT) / 2); // 50%
552+
}
553+
542554
// New PWM assignment
543555
if ((chans[channel_idx].pin == -1) || (chans[channel_idx].timer_idx != timer_idx)) {
544556
configure_channel(self);
@@ -549,17 +561,9 @@ STATIC void mp_machine_pwm_init_helper(machine_pwm_obj_t *self,
549561

550562
// Set timer frequency
551563
set_freq(self, freq, &timers[timer_idx]);
564+
// This will _re-apply_ duty, if it's a new channel, but that's ok....
565+
set_duty_apply(self);
552566

553-
// Set duty cycle?
554-
if (duty_u16 != -1) {
555-
set_duty_u16(self, duty_u16);
556-
} else if (duty_ns != -1) {
557-
set_duty_ns(self, duty_ns);
558-
} else if (duty != -1) {
559-
set_duty_u10(self, duty);
560-
} else if (self->duty_x == 0) {
561-
set_duty_u10(self, (1 << PWM_RES_10_BIT) / 2); // 50%
562-
}
563567
}
564568

565569
// This called from PWM() constructor
@@ -664,6 +668,7 @@ STATIC mp_obj_t mp_machine_pwm_duty_get(machine_pwm_obj_t *self) {
664668

665669
STATIC void mp_machine_pwm_duty_set(machine_pwm_obj_t *self, mp_int_t duty) {
666670
set_duty_u10(self, duty);
671+
set_duty_apply(self);
667672
}
668673

669674
STATIC mp_obj_t mp_machine_pwm_duty_get_u16(machine_pwm_obj_t *self) {
@@ -672,6 +677,7 @@ STATIC mp_obj_t mp_machine_pwm_duty_get_u16(machine_pwm_obj_t *self) {
672677

673678
STATIC void mp_machine_pwm_duty_set_u16(machine_pwm_obj_t *self, mp_int_t duty_u16) {
674679
set_duty_u16(self, duty_u16);
680+
set_duty_apply(self);
675681
}
676682

677683
STATIC mp_obj_t mp_machine_pwm_duty_get_ns(machine_pwm_obj_t *self) {
@@ -680,4 +686,5 @@ STATIC mp_obj_t mp_machine_pwm_duty_get_ns(machine_pwm_obj_t *self) {
680686

681687
STATIC void mp_machine_pwm_duty_set_ns(machine_pwm_obj_t *self, mp_int_t duty_ns) {
682688
set_duty_ns(self, duty_ns);
689+
set_duty_apply(self);
683690
}

0 commit comments

Comments
 (0)
0