PWM: Further analogWrite broken after ledcDetachPin

[habazut]

Board

ESP32 Dev Module

Device Description

Random Devkit

Hardware Configuration

Any GPIO than can do PWM

Version

v2.0.4

IDE Name

Arduino IDE

Operating System

Whatever

Flash frequency

Whatever

PSRAM enabled

no

Upload speed

Whatever

Description

The following sequence of function calls

analogWrite(pin, 128);
ledcDetachPin(pin);
analogWrite(pin,128);

does only give PWM once and not after the second call to analogWrite.

Problem: Internal conditional on pin_to_channel array prevents second ledcAttachPin.

Solution:

$ diff -u .arduino15/packages/esp32/hardware/esp32/2.0.4/cores/esp32/esp32-hal-ledc.c{.orig,}
--- .arduino15/packages/esp32/hardware/esp32/2.0.4/cores/esp32/esp32-hal-ledc.c.orig	2022-07-06 11:47:49.000000000 +0200
+++ .arduino15/packages/esp32/hardware/esp32/2.0.4/cores/esp32/esp32-hal-ledc.c	2022-08-10 00:18:47.778755484 +0200
@@ -222,6 +222,8 @@
       pin_to_channel[pin] = cnt_channel--;
       ledcAttachPin(pin, cnt_channel);
       ledcSetup(cnt_channel, 1000, 8);
+    } else {
+      ledcAttachPin(pin, cnt_channel);
     }
     ledcWrite(pin_to_channel[pin] - 1, value);
   }

Apply inline patch and it works.

Regards,
Harald.

Sketch

See above, you can reproduce

Debug Message

See above, you can reproduce

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

• I confirm I have checked existing issues, online documentation and Troubleshooting guide.

[habazut]

Ok, I see no reaction (1 week).

Harald.

[P-R-O-C-H-Y]

Ok, I see no reaction (1 week).

Harald.

@habazut Its on my list.

[habazut]

I found one more bug in that corner. The first time you do analogWrite() to the pin it does
ledcAttachPin(pin,cnt_channel);
but cnt_channel is 16 and channels are numbered 0 to 15.

--- esp32-hal-ledc.c.orig	2022-07-06 11:47:49.000000000 +0200
+++ esp32-hal-ledc.c	2022-08-30 08:24:09.113581755 +0200
@@ -219,10 +219,12 @@
           log_e("No more analogWrite channels available! You can have maximum %u", LEDC_CHANNELS);
           return;
       }
-      pin_to_channel[pin] = cnt_channel--;
+      pin_to_channel[pin] = --cnt_channel;
       ledcAttachPin(pin, cnt_channel);
       ledcSetup(cnt_channel, 1000, 8);
+    } else {
+      ledcAttachPin(pin, pin_to_channel[pin]);
     }
-    ledcWrite(pin_to_channel[pin] - 1, value);
+    ledcWrite(pin_to_channel[pin], value);
   }
 }

I recommend this patch instead which does put the values 0 to 15 into the pin array.

Then it would be nice to have an API to get the channel/pin relationship so that it's possible to change the frequency on a pin. Or do a analogWriteFrequency(pin, frequency) function.

Greetings,
Harald.

[P-R-O-C-H-Y]

Hi @habazut, I will take a look on the bugs and about the new API you mentioned, official Arduino don't have any of this, so why don't you just use our LEDC API without analogWrite?

[habazut]

I use analogWrite() because this is supposed to work on AVR, ESP32, SAMD, STM32 without #ifdef-hell.
I think he analogWriteFrequency was available in one of the other non-AVR versions of the Arduino API.

Regards,
Harald.

[P-R-O-C-H-Y]

Hi @habazut, there is no bug in the analogWrite API.
I have added quick debug information to analogWrite() and call it 16 times and all channels from 15 to 0 are set.
Check output message:

[    21][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH15 - GPIO0!
[    21][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH14 - GPIO2!
[    24][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH13 - GPIO4!
[    32][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH12 - GPIO16!
[    39][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH11 - GPIO17!
[    47][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH10 - GPIO5!
[    54][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH9 - GPIO18!
[    62][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH8 - GPIO19!
[    69][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH7 - GPIO12!
[    76][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH6 - GPIO13!
[    84][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH5 - GPIO14!
[    91][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH4 - GPIO27!
[    98][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH3 - GPIO26!
[   106][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH2 - GPIO25!
[   113][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH1 - GPIO33!
[   121][D][esp32-hal-ledc.c:226] analogWrite(): ledc channel setuped CH0 - GPIO32!

About the analogWriteFrequency I will create this function, that will change frequency for all analogWrite channels used.

[P-R-O-C-H-Y]

@habazut PR created, please give it a try.
Added analogWriteFrequency + analogWriteResolution.
Thanks

[P-R-O-C-H-Y]

@hazabut, The fix is in the ledAttachPin() where the duty was always set to 0. I won't edit the analogWrite() with your suggested changes, but instead you can now add ledcAttachPin(pin,analogGetChannel(pin)) to make it work for you.

  analogWrite(pin, 128);
  delay(50);
  ledcDetachPin(pin);
  delay(50);
  ledcAttachPin(pin,analogGetChannel(pin));
  analogWrite(pin,128);