From be007796fbd7cf81ec6c13d191190fa3ef2ff811 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Mon, 7 Apr 2025 09:45:12 -0300 Subject: [PATCH 1/5] feat(ledc): max resolution review --- cores/esp32/esp32-hal-ledc.c | 47 ++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index 0a3ec5a60c7..9e134902080 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -184,13 +184,6 @@ bool ledcWrite(uint8_t pin, uint32_t duty) { uint8_t group = (bus->channel / 8), channel = (bus->channel % 8); - //Fixing if all bits in resolution is set = LEDC FULL ON - uint32_t max_duty = (1 << bus->channel_resolution) - 1; - - if ((duty == max_duty) && (max_duty != 1)) { - duty = max_duty + 1; - } - ledc_set_duty(group, channel, duty); ledc_update_duty(group, channel); @@ -211,12 +204,6 @@ bool ledcWriteChannel(uint8_t channel, uint32_t duty) { uint32_t resolution = 0; ledc_ll_get_duty_resolution(LEDC_LL_GET_HW(), group, timer, &resolution); - uint32_t max_duty = (1 << resolution) - 1; - - if ((duty == max_duty) && (max_duty != 1)) { - duty = max_duty + 1; - } - ledc_set_duty(group, channel, duty); ledc_update_duty(group, channel); @@ -265,15 +252,16 @@ uint32_t ledcWriteTone(uint8_t pin, uint32_t freq) { bus->channel_resolution = 10; uint32_t res_freq = ledc_get_freq(group, timer); - ledcWrite(pin, 0x1FF); + ledcWrite(pin, 0x200); // LEDC 50% duty is 2^10 / 2 = 0x200 return res_freq; } return 0; } uint32_t ledcWriteNote(uint8_t pin, note_t note, uint8_t octave) { - const uint16_t noteFrequencyBase[12] = {// C C# D Eb E F F# G G# A Bb B - 4186, 4435, 4699, 4978, 5274, 5588, 5920, 6272, 6645, 7040, 7459, 7902 + const uint16_t noteFrequencyBase[12] = { + // C C# D Eb E F F# G G# A Bb B + 4186, 4435, 4699, 4978, 5274, 5588, 5920, 6272, 6645, 7040, 7459, 7902 }; if (octave > 8 || note >= NOTE_MAX) { @@ -391,13 +379,15 @@ static bool ledcFadeConfig(uint8_t pin, uint32_t start_duty, uint32_t target_dut ledc_cbs_t callbacks = {.fade_cb = ledcFnWrapper}; ledc_cb_register(group, channel, &callbacks, (void *)bus); - //Fixing if all bits in resolution is set = LEDC FULL ON - uint32_t max_duty = (1 << bus->channel_resolution) - 1; + uint32_t max_duty = (1 << bus->channel_resolution); // Max LEDC duty - if ((target_duty == max_duty) && (max_duty != 1)) { - target_duty = max_duty + 1; - } else if ((start_duty == max_duty) && (max_duty != 1)) { - start_duty = max_duty + 1; + if (target_duty > max_duty) { + log_w("Target duty %d was adjusted to the maximum duty %d", target_duty, max_duty); + target_duty = max_duty; + } + if (start_duty > max_duty) { + log_w("Starting duty %d was adjusted to the maximum duty %d", start_duty, max_duty); + start_duty = max_duty; } #if SOC_LEDC_SUPPORT_FADE_STOP @@ -411,7 +401,7 @@ static bool ledcFadeConfig(uint8_t pin, uint32_t start_duty, uint32_t target_dut // Wait for LEDCs next PWM cycle to update duty (~ 1-2 ms) while (ledc_get_duty(group, channel) != start_duty); - if (ledc_set_fade_time_and_start(group, channel, target_duty, max_fade_time_ms, LEDC_FADE_NO_WAIT) != ESP_OK) { + if (ledc_set_fade_time_and_start(group, channel, target_duty, _fade_time_ms, LEDC_FADE_NO_WAIT) != ESP_OK) { log_e("ledc_set_fade_time_and_start failed"); return false; } @@ -446,6 +436,17 @@ void analogWrite(uint8_t pin, int value) { return; } } + // Arduino API says that duty goes from 0 to (2^resolution) - 1 + // But LEDC works with duty from 0 to (2^resolution) + // Therefore, it will adjust Arduino MAX Duty to be the LEDC MAx Duty + uint32_t max_duty = (1 << bus->channel_resolution) - 1; + if (value < 0 || value > max_duty) { + log_w("Duty is out of range. Valid duty range for pin d is 0 to %d", pin, max_duty); + return; + } + if ((value == max_duty) && (max_duty != 1)) { + value = max_duty + 1; + } ledcWrite(pin, value); } } From 940cf59b119249adce98df2a649979a3ca92c6bb Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Mon, 7 Apr 2025 09:57:47 -0300 Subject: [PATCH 2/5] feat(ledc): check duty limit based on the resolution --- cores/esp32/esp32-hal-ledc.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index 9e134902080..72b787bab2c 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -183,7 +183,13 @@ bool ledcWrite(uint8_t pin, uint32_t duty) { if (bus != NULL) { uint8_t group = (bus->channel / 8), channel = (bus->channel % 8); + uint32_t max_duty = (1 << bus->channel_resolution); // Max LEDC duty + if (duty > max_duty) { + log_w("Target duty %d was adjusted to the maximum duty %d", duty, max_duty); + duty = max_duty; + } + ledc_set_duty(group, channel, duty); ledc_update_duty(group, channel); @@ -203,7 +209,12 @@ bool ledcWriteChannel(uint8_t channel, uint32_t duty) { //Fixing if all bits in resolution is set = LEDC FULL ON uint32_t resolution = 0; ledc_ll_get_duty_resolution(LEDC_LL_GET_HW(), group, timer, &resolution); + uint32_t max_duty = (1 << resolution); // Max LEDC duty + if (duty > max_duty) { + log_w("Target duty %d was adjusted to the maximum duty %d", duty, max_duty); + duty = max_duty; + } ledc_set_duty(group, channel, duty); ledc_update_duty(group, channel); From 443f7e10f1d5bc88af426314d9f805bceaff8992 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Mon, 7 Apr 2025 09:58:52 -0300 Subject: [PATCH 3/5] fix(ledc): removing white space in code line --- cores/esp32/esp32-hal-ledc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index 72b787bab2c..fdbc4939f1f 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -189,7 +189,6 @@ bool ledcWrite(uint8_t pin, uint32_t duty) { log_w("Target duty %d was adjusted to the maximum duty %d", duty, max_duty); duty = max_duty; } - ledc_set_duty(group, channel, duty); ledc_update_duty(group, channel); From 591a1610ff42581ca0b307bd753b33b933766a61 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Mon, 7 Apr 2025 10:14:02 -0300 Subject: [PATCH 4/5] fix(ledc): reverting not necessary change --- cores/esp32/esp32-hal-ledc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index fdbc4939f1f..9b3ec036144 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -411,7 +411,7 @@ static bool ledcFadeConfig(uint8_t pin, uint32_t start_duty, uint32_t target_dut // Wait for LEDCs next PWM cycle to update duty (~ 1-2 ms) while (ledc_get_duty(group, channel) != start_duty); - if (ledc_set_fade_time_and_start(group, channel, target_duty, _fade_time_ms, LEDC_FADE_NO_WAIT) != ESP_OK) { + if (ledc_set_fade_time_and_start(group, channel, target_duty, max_fade_time_ms, LEDC_FADE_NO_WAIT) != ESP_OK) { log_e("ledc_set_fade_time_and_start failed"); return false; } From cdd04b1d406802d27638af7e250f4c6b5d4176c4 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Mon, 7 Apr 2025 15:14:43 -0300 Subject: [PATCH 5/5] feat(ledc): testing values that shall be consistent --- cores/esp32/esp32-hal-ledc.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/cores/esp32/esp32-hal-ledc.c b/cores/esp32/esp32-hal-ledc.c index 9b3ec036144..0b044d7e97d 100644 --- a/cores/esp32/esp32-hal-ledc.c +++ b/cores/esp32/esp32-hal-ledc.c @@ -377,6 +377,21 @@ static bool ledcFadeConfig(uint8_t pin, uint32_t start_duty, uint32_t target_dut #endif uint8_t group = (bus->channel / 8), channel = (bus->channel % 8); + uint32_t max_duty = (1 << bus->channel_resolution); // Max LEDC duty + + if (target_duty > max_duty) { + log_w("Final duty %d was adjusted to the maximum duty %d", target_duty, max_duty); + target_duty = max_duty; + } + if (start_duty > max_duty) { + log_w("Starting duty %d was adjusted to the maximum duty %d", start_duty, max_duty); + start_duty = max_duty; + } + if (start_duty >= target_duty) { + log_e("Starting duty must be lower than the final duty"); + return false; + } + // Initialize fade service. if (!fade_initialized) { ledc_fade_func_install(0); @@ -389,17 +404,6 @@ static bool ledcFadeConfig(uint8_t pin, uint32_t start_duty, uint32_t target_dut ledc_cbs_t callbacks = {.fade_cb = ledcFnWrapper}; ledc_cb_register(group, channel, &callbacks, (void *)bus); - uint32_t max_duty = (1 << bus->channel_resolution); // Max LEDC duty - - if (target_duty > max_duty) { - log_w("Target duty %d was adjusted to the maximum duty %d", target_duty, max_duty); - target_duty = max_duty; - } - if (start_duty > max_duty) { - log_w("Starting duty %d was adjusted to the maximum duty %d", start_duty, max_duty); - start_duty = max_duty; - } - #if SOC_LEDC_SUPPORT_FADE_STOP ledc_fade_stop(group, channel); #endif @@ -454,7 +458,7 @@ void analogWrite(uint8_t pin, int value) { log_w("Duty is out of range. Valid duty range for pin d is 0 to %d", pin, max_duty); return; } - if ((value == max_duty) && (max_duty != 1)) { + if (value == max_duty) { value = max_duty + 1; } ledcWrite(pin, value);