Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ledcFade can lead to dead-lock on ESP32-classic #9212

Closed
1 task done
TD-er opened this issue Feb 5, 2024 · 5 comments · Fixed by #9257
Closed
1 task done

ledcFade can lead to dead-lock on ESP32-classic #9212

TD-er opened this issue Feb 5, 2024 · 5 comments · Fixed by #9257
Assignees
Labels
Status: Needs investigation We need to do some research before taking next steps on this issue

Comments

@TD-er
Copy link
Contributor

TD-er commented Feb 5, 2024

Board

ESP32 (classic)

Device Description

ESP32-classic

Hardware Configuration

Nothing attached to the board

Version

latest master (checkout manually)

IDE Name

PlatformIO

Operating System

Windows 11

Flash frequency

40MHz

PSRAM enabled

yes

Upload speed

115200

Description

Several calls to ledcAttach followed by ledcFade on the same pin without calling ledcDetach for that pin will lead to a completely frozen system on ESP32.

N.B. the same code on ESP32-C2 does not show this behavior. Not tested on other ESP32-variants.

Since there doesn't seem to be a simple check to see if a pin is already attached, I added a simple ledcWrite check first to set the start duty cycle for the upcoming fade.
If this returns false, I run ledcAttach on that pin.

So either of these 2 variants do work:

    if (!ledcWrite(gpio, start_duty)) {
      // Pin not yet attached
      if (!ledcAttach(gpio, frequency, resolution)) {
        return false;
      }
    }

    if (!ledcFade(gpio, start_duty, target_duty, fadeDuration_ms)) {
      return false;
    }
    ledcDetach(gpio);
    if (!ledcAttach(gpio, frequency, resolution)) {
      return false;
    }

    if (!ledcFade(gpio, start_duty, target_duty, fadeDuration_ms)) {
      return false;
    }

Sketch

// Not working code
while (true) {
    if (ledcAttach(gpio, frequency, resolution)) {
      ledcFade(gpio, start_duty, target_duty, fadeDuration_ms);
    }
    delay(2*fadeDuration_ms);
}

Debug Message

Node just freezes after a few calls
Probably due to the semaphore code in ledcFadeConfig

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.
@SuGlider
Copy link
Collaborator

SuGlider commented Feb 5, 2024

@TD-er - based on the fact that you are using the C2 to test it, I guess that this is about Arduino Core 3.0.0-Alpha2.
Is it correct?

@P-R-O-C-H-Y - Please take a look. It may be related to this change: https://github.com/espressif/arduino-esp32/pull/9031/files

@SuGlider SuGlider added Status: Needs investigation We need to do some research before taking next steps on this issue and removed Status: Awaiting triage Issue is waiting for triage labels Feb 5, 2024
@TD-er
Copy link
Contributor Author

TD-er commented Feb 5, 2024

Yep, the latest code from Arduino and ESP-IDF.
So it might be that this is even a soon-to-be-bug in Arduino if it doesn't fail yet on your tests :)

@P-R-O-C-H-Y
Copy link
Member

Hi @TD-er, I am now taking a look at this. Just quick question, why would you call every time the ledcAttach(), is that because of the pin is using for other purposes also?

@P-R-O-C-H-Y
Copy link
Member

P-R-O-C-H-Y commented Feb 15, 2024

I have used you non-working code in testing sketch, but no issues on my side.

// use 12 bit precission for LEDC timer
#define LEDC_TIMER_12_BIT  12

// use 5000 Hz as a LEDC base frequency
#define LEDC_BASE_FREQ     5000

// fade LED PIN (replace with LED_BUILTIN constant for built-in LED)
#define LED_PIN            4

// define starting duty, target duty and maximum fade time
#define LEDC_START_DUTY   (0)
#define LEDC_TARGET_DUTY  (4095)
#define LEDC_FADE_TIME    (3000)

void setup() {
}

void loop() {
    if (ledcAttach(LED_PIN, LEDC_BASE_FREQ, LEDC_TIMER_12_BIT)) {
      ledcFade(LED_PIN, LEDC_START_DUTY, LEDC_TARGET_DUTY, LEDC_FADE_TIME);
    }
    delay(2*LEDC_FADE_TIME);
}

Serial output:

[139902][V][esp32-hal-periman.c:229] perimanSetBusDeinit(): Deinit function for type LEDC (12) successfully set to 0x400d15ec
[139913][V][esp32-hal-periman.c:154] perimanSetPinBus(): Pin 4 successfully set to type INIT (0) with bus 0x0
[139925][V][esp32-hal-periman.c:154] perimanSetPinBus(): Pin 4 successfully set to type LEDC (12) with bus 0x3ffb8464
[139938][I][esp32-hal-ledc.c:116] ledcAttachChannel(): LEDC attached to pin 4 (channel 1, resolution 12)
[145949][V][esp32-hal-periman.c:229] perimanSetBusDeinit(): Deinit function for type LEDC (12) successfully set to 0x400d15ec
[145960][V][esp32-hal-periman.c:154] perimanSetPinBus(): Pin 4 successfully set to type INIT (0) with bus 0x0
[145972][V][esp32-hal-periman.c:154] perimanSetPinBus(): Pin 4 successfully set to type LEDC (12) with bus 0x3ffb8464
[145985][I][esp32-hal-ledc.c:116] ledcAttachChannel(): LEDC attached to pin 4 (channel 0, resolution 12)

Edit: as the pin is not used for anything else, the ledcAttach() should actually fail, as the pin is already attached to LEDC.
This unintended behaviour appears with latest LEDC manual channel selection feature. You can see in output that, the LEDC channel was switching from 0 to 1 and back.

@TD-er
Copy link
Contributor Author

TD-er commented Feb 15, 2024

Hi @TD-er, I am now taking a look at this. Just quick question, why would you call every time the ledcAttach(), is that because of the pin is using for other purposes also?

The actual use case is about letting a LED fade from one state to another state and then back again.
And since it is done via commands from rules in my application, the command just doesn't make any assumptions of the pin state and starts the fade process. (not sure if that's the best approach, but that was how it was done)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs investigation We need to do some research before taking next steps on this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants