esp32: Always set the duty cycle when setting the frequency. #8361
Conversation
|
@IhorNehrutsa any comment? |
|
Ukraine and Odesa under Putin's attack. I am in readonly mode. |
|
@IhorNehrutsa That's what I assumed. Good luck. |
|
@IhorNehrutsa The change to set_freq() only fixes #8345. But I could not find the spot to addrss #8306. Therefore I fell back to changing set_freq in my previous PR. |
|
@IhorNehrutsa ah, my apologies, I did not match your name to a location (but I definitely should have, my wife is from Mariupol). We are all with you. |
ports/esp32/machine_pwm.c
Outdated
| @@ -71,28 +71,31 @@ STATIC ledc_timer_config_t timers[PWM_TIMER_MAX]; | |||
| #define TIMER_IDX_TO_MODE(timer_idx) (timer_idx / LEDC_TIMER_MAX) | |||
| #define TIMER_IDX_TO_TIMER(timer_idx) (timer_idx % LEDC_TIMER_MAX) | |||
|
|
|||
| // Params for PWM operation | |||
| // Params for PW operation | |||
There was a problem hiding this comment.
I think having "PWM" is better, because "PW" is not a common abbreviation and may confuse readers.
There was a problem hiding this comment.
That is something, that was fixed in the previous PR and now just returns, because my PR is based on an earlier state of the code.
ports/esp32/machine_pwm.c
Outdated
| #define PWFREQ (5000) | ||
|
|
||
| // 10-bit resolution (compatible with esp8266 PWM) | ||
| #define PWRES (LEDC_TIMER_10_BIT) |
There was a problem hiding this comment.
IMO it's best to leave these macros named as they were to minimise the diff. Also, as above, "PW" is confusing.
There was a problem hiding this comment.
As above. Just history.
|
@dpgeorge I've made the suggested changes. tested on a ES32 SPIRAM and UM_FEATHER_S2. Please have a look at the open MIMXRT PR's as well: |
If setting the frequency to a value used already by an existing timer, this timer will be used. But still, the duty cycle for that channel may have to be changed. Fixes micropython#8306 as well.
|
Squashed and merged in 55a0125 |
If setting the frequency to a value used already by an existing timer,
this timer will be used. But still, the duty cycle for that channel
may have to be changed, so it has to be set.
Fixes #8345 and by chance #8306 as well.