Skip to content

AttachInterrupt does not work on output of PWM (2.0.2) #6140

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

Closed
1 task done
mstegen opened this issue Jan 16, 2022 · 8 comments · Fixed by #6160
Closed
1 task done

AttachInterrupt does not work on output of PWM (2.0.2) #6140

mstegen opened this issue Jan 16, 2022 · 8 comments · Fixed by #6160
Assignees

Comments

@mstegen
Copy link
Contributor

mstegen commented Jan 16, 2022

Board

ESP32Dev module

Device Description

I'm using a PWM output, and setup a pin interrupt so i can detect the start of the PWM signal.
This works fine in 2.0.1, but not on 2.0.2

Hardware Configuration

no

Version

latest master

IDE Name

PlatfromIO

Operating System

Win10/Linux

Flash frequency

80Mhz

PSRAM enabled

no

Upload speed

115200

Description

attachInterrupt should be able to attach to the output signal of the PWM signal generated by ledC.
this does work in 2.0.1, but not in 2.0.2.

I noticed that there were changes made to the LEDC code (#6045)
when using the old (2.0.1) code for ledcAttachPin in my code, it works again.
So something in the new code, does not allow a PIN interrupt to be attached to the PWM output.

Below is test code to reproduce the problem.

Sketch

#include <Arduino.h>

#define PIN_PWM_OUT 19
#define PWM_CHANNEL 0

volatile uint32_t isrCounter = 0;

// PWM pin low to high transition ISR
void IRAM_ATTR onPulse() {
    // increment counter
    isrCounter++;
}

void setup() {

    pinMode(PIN_PWM_OUT, OUTPUT);            

    // Setup PWM on channel 0, 1000Hz, 10 bits resolution
    ledcSetup(PWM_CHANNEL, 1000, 10);             // channel 0  => Group: 0, Channel: 0, Timer: 0
    ledcAttachPin(PIN_PWM_OUT, PWM_CHANNEL);      // attach the channels to the GPIO to be controlled
    ledcWrite(PWM_CHANNEL, 512);                  // channel 0, duty cycle 50%

    // Setup PIN interrupt on rising edge
    attachInterrupt(PIN_PWM_OUT, onPulse, RISING);
    
    Serial.begin(115200);
    while (!Serial);
}

void loop() {
  
    Serial.printf("ISR Counter: %u\n",isrCounter);
    delay(1000);
}

Debug Message

The Counter in above test code will not increase.

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.
@mstegen mstegen added the Status: Awaiting triage Issue is waiting for triage label Jan 16, 2022
@me-no-dev
Copy link
Member

@P-R-O-C-H-Y will you please give this a shot?

@TD-er
Copy link
Contributor

TD-er commented Jan 20, 2022

I'm looking into the ledcWrite and related functions too on 2.0.2
Are you sure it is a counting issue and not just that it doesn't output a PWM signal on the pin?

I can't get the ESP32 to output PWM signal on GPIO pins since 2.0.2 (at least not using the same code as before)

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

P-R-O-C-H-Y commented Jan 20, 2022

Hi @mstegen, move function pinMode(PIN_PWM_OUT, OUTPUT); after ledcAttachPin(PIN_PWM_OUT, PWM_CHANNEL); and it will work again :)
Making a PR, that will add the pinMode(pin, OUTPUT); in ledcAttachPin so there will be no need to call that separately in your code 👍

void setup() {

    // Setup PWM on channel 0, 1000Hz, 10 bits resolution
    ledcSetup(PWM_CHANNEL, 1000, 10);             // channel 0  => Group: 0, Channel: 0, Timer: 0
    ledcAttachPin(PIN_PWM_OUT, PWM_CHANNEL);      // attach the channels to the GPIO to be controlled
   
    pinMode(PIN_PWM_OUT, OUTPUT); //moved here

    ledcWrite(PWM_CHANNEL, 512);                  // channel 0, duty cycle 50%

    // Setup PIN interrupt on rising edge
    attachInterrupt(PIN_PWM_OUT, onPulse, RISING);
    
    Serial.begin(115200);
    while (!Serial);
}

@TD-er The LEDC PWM outputs are working as normal :) No problem in 2.0.2
If you use the sketch above, it has to work :)

@TD-er
Copy link
Contributor

TD-er commented Jan 20, 2022

OK will test.
Does the code (with the proposed changes) still work on older SDK versions?

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

P-R-O-C-H-Y commented Jan 20, 2022

@TD-er Yes, tried with 2.0.2 and 2.0.0 and both works as expected :)

me-no-dev pushed a commit that referenced this issue Jan 20, 2022
@TD-er
Copy link
Contributor

TD-er commented Jan 20, 2022

I probably found why it isn't working on my setup.
I have a function to try an find a free channel so I'm not interfering with other libraries.
On pre-2.0.2 it worked just fine, but now apparently ledcRead() is no longer returning 0 if it is free to use.
Will have to look into it a bit more and if I found something indicating it is a bug, I will open a new issue for it.

Ah apparently that code of mine is no longer needed as it is now implemented here:
96c184d

@mstegen
Copy link
Contributor Author

mstegen commented Jan 20, 2022

Hi @mstegen, move function pinMode(PIN_PWM_OUT, OUTPUT); after ledcAttachPin(PIN_PWM_OUT, PWM_CHANNEL); and it will work again :) Making a PR, that will add the pinMode(pin, OUTPUT); in ledcAttachPin so there will be no need to call that separately in your code 👍

Great @P-R-O-C-H-Y ! thanks a lot for fixing this.
If i only knew that it this could be fixed this easily...

@VojtechBartoska VojtechBartoska removed the Status: Awaiting triage Issue is waiting for triage label Jan 24, 2022
@G-EDM
Copy link

G-EDM commented Feb 3, 2024

delete "pinMode(PIN_PWM_OUT, OUTPUT);", PWM is a separate block without CPU. And you cannot connect an interrupt to this output.

Are you sure? It worked with the old version. I'm using MCPWM to generate PWM and also attached an interrupt to the Pin. It worked until today after upgrading from 3.x to the new 6.5. It was working pretty nice so it is possible.

After setting up MCPWM I used pinMode(pwm_pin, GPIO_MODE_INPUT_OUTPUT); and then attachInterrupt(). Seems to be broken after the upgrade.

Edit: Removing the pinMode() call seems to make it work again. But also have big issues with I2S now producing totally messed up readings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants