ports/esp32/machine_pwm: Add support for all PWM timers and modes. #3608
Conversation
ESP32 has 16 PWM channels with 8 separate timers, but current PWM implementation
forces user to share single timer between all channels. This commit
allows to use all 8 timers and does not break backward compatibility.
API additions:
- PWM initializer takes two new optional keyword arguments:
`pwm.init(..., timer=0, speed_mode=PWM.HIGH_SPEED_MODE)`
- PWM class defines two new constants:
`PWM.HIGH_SPEED_MODE` and `PWM.LOW_SPEED_MODE`
| @@ -35,56 +35,52 @@ | |||
| // Forward dec'l | |||
| extern const mp_obj_type_t machine_pwm_type; | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Please don't add blank lines when not necessary.
| @@ -133,16 +132,31 @@ STATIC void esp32_pwm_init_helper(esp32_pwm_obj_t *self, | |||
| } | |||
| self->channel = channel; | |||
|
|
|||
| // set timer | |||
| if (0 <= args[ARG_timer].u_int && args[ARG_timer].u_int <= 3) { | |||
There was a problem hiding this comment.
Shouldn't the upper bound be < LEDC_TIMER_MAX?
| } | ||
|
|
||
| // set speed mode | ||
| if (0 <= args[ARG_mode].u_int && args[ARG_mode].u_int <= LEDC_SPEED_MODE_MAX) { |
There was a problem hiding this comment.
I think the upper bound here should be < LEDC_SPEED_MODE_MAX (note < not <=).
| int fval = args[ARG_freq].u_int; | ||
| if (fval != -1 && ledc_set_freq(self->mode, self->timer, fval) != ESP_OK) { | ||
| nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_ValueError, | ||
| "Bad frequency %d", fval)); |
There was a problem hiding this comment.
Please use lower-case at the start of error messages, for consistency with the rest of the code.
|
Thanks for the contribution! Can you please explain what the HIGH_SPEED and LOW_SPEED modes are and what their differences are? If there really are 4 high speed and 4 low speed timers then maybe it would be better to expose them as 8 timers. So get rid of the |
|
My esp32 bleeds pwm i to other pins. If i instantiate 4 pwm pins (pins: 5,18,19,21), sometimes pin5's pwm bleeds into the other pins. i changes the code to pin 4 and was getting the same results. i need to be able to have 4 separate pwm pins with control over each freq and duty cycle. Thanks for the contribution, i hope by switching to a different timer, this will solve the issue that im having. im going to test other pins today with an oscilloscope, and a different esp32 to make sure nothing is wrong on my end. Love this code base. i just have to have some workshops on micropython at CSUF next year for some other students. Let me know if im off on this assessment of this merge request. thanks. |
|
Is there any way to detach a gpio from PWM? consider an example with PWM on pin 12 and 13 that uses the same timer. |
You can try configuring that pin for GPIO, like |
|
thank you |
|
|
Happy to see this - hope it gets pulled in. I suggest keeping low speed and high speed distinction, with proper docs or checks. |
Update main with latest 6.0.x
|
This pull request is obsolete since |
|
I'll close this in favour of #7817 (and also because the feedback here was not addressed in this PR). |
This commit allows using all the available PWM timers (up to 8) and channels (up to 16), without affecting the PWM API. If a new frequency is set, first it checks if another timer is using the same frequency. If yes, then it uses this timer, otherwise, it creates a new one. If all timers are used, the user should set an already used frequency, or de-init a channel. This work is based on #6276 and #3608.
ESP32 has 16 PWM channels with 8 separate timers, but current PWM implementation
forces user to share single timer between all channels. This commit
allows to use all 8 timers and does not break backward compatibility.
API additions:
pwm.init(..., timer=0, speed_mode=PWM.HIGH_SPEED_MODE)PWM.HIGH_SPEED_MODEandPWM.LOW_SPEED_MODEExample:
There are 4 low speed timers and 4 high speed timers on ESP32.