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

pinMode disables interrupts as a side effect #6669

Closed
1 task done
quentinmit opened this issue Apr 30, 2022 · 8 comments · Fixed by #6695
Closed
1 task done

pinMode disables interrupts as a side effect #6669

quentinmit opened this issue Apr 30, 2022 · 8 comments · Fixed by #6695
Assignees
Labels
Area: Peripherals API Relates to peripheral's APIs. Status: Solved
Milestone

Comments

@quentinmit
Copy link

Board

ESP32-DevKitC

Device Description

DevKitC

Hardware Configuration

For the demo code below, connect a switch or square wave to pin 19.

Version

v2.0.2

IDE Name

PlatformIO

Operating System

macOS

Flash frequency

40MHz

PSRAM enabled

no

Upload speed

115200

Description

The pinMode call, in addition to changing pullups/whether a pin is driving the output or not/etc, appears to disable pin interrupts as a side effect. In the ESP-IDF version of the code, intr_type is explicitly set to GPIO_INTR_DISABLE. In the older register code, the field is accidentally overwritten by the write to GPIO.pin[x].val when it's really just trying to set GPIO.pin[x].pad_driver.

This is a problem because while pinMode is IRAM_ATTR and safe to call from interrupt handlers, attachInterrupt is not (and it's also rather heavyweight), so it's not possible to re-enable the interrupt if pinMode is being used in an ISR.

pinMode should just preserve whatever previous state of interrupts existed; if the user has an interrupt configured on a pin and wants to change the pin mode, they probably know what they are doing. If they want to disable the interrupt they can call detachInterrupt directly.

Sketch

#include "Arduino.h"

// This sketch should print the number of times pin 19 changes.

uint32_t interrupt_count = 0;

void IRAM_ATTR pin_interrupt(void *arg) {
  interrupt_count++;
  pinMode(19, INPUT);
}

void setup() {
  pinMode(19, INPUT);
  attachInterruptArg(19, pin_interrupt, 0, CHANGE);
}

void loop() {
  ESP_LOGD("loop", "interrupt count %d", interrupt_count);
  delay(500);
}

Debug Message

n/a

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.
@quentinmit quentinmit added the Status: Awaiting triage Issue is waiting for triage label Apr 30, 2022
@SuGlider
Copy link
Collaborator

Issue confirmed with the latest Arduino Core Version 2.0.3-RC1.

@P-R-O-C-H-Y, PTAL

@VojtechBartoska VojtechBartoska 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 May 2, 2022
@TD-er
Copy link
Contributor

TD-er commented May 2, 2022

Hmm I think this also affects PWM.
Set PWM on a pin and call pinMode => No more active PWM on that pin.

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

Hi @quentinmit
When calling pinMode(pin,mode); it configures the pin for default config, which is interrupt disabled.
So you need to call attachInterruptArg(19, pin_interrupt, 0, CHANGE); again after pinMode to make it work :)

#include "Arduino.h"

// This sketch should print the number of times pin 19 changes.

uint32_t interrupt_count = 0;

void IRAM_ATTR pin_interrupt(void *arg) {
  interrupt_count++;
  pinMode(19, INPUT);
  attachInterruptArg(19, pin_interrupt, 0, CHANGE);
}

void setup() {
  pinMode(19, INPUT);
  attachInterruptArg(19, pin_interrupt, 0, CHANGE);
}

void loop() {
  ESP_LOGD("loop", "interrupt count %d", interrupt_count);
  delay(500);
}

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

Hi @TD-er,

If you setup some PWM peripheral like LEDC on a pin, when you call pinMode on same pin, the configuration will set the pin function to be GPIO and the signal from peripheral will be no longer connected to the pin.

@TD-er
Copy link
Contributor

TD-er commented May 4, 2022

Hi @TD-er,

If you setup some PWM peripheral like LEDC on a pin, when you call pinMode on same pin, the configuration will set the pin function to be GPIO and the signal from peripheral will be no longer connected to the pin.

Yep, found that too, after updating to IDF4.4
With the older SDK it behaved differently.

@P-R-O-C-H-Y P-R-O-C-H-Y added Resolution: Wontfix Arduino ESP32 team will not fix the issue Area: Peripherals API Relates to peripheral's APIs. and removed Status: Needs investigation We need to do some research before taking next steps on this issue labels May 4, 2022
@P-R-O-C-H-Y P-R-O-C-H-Y moved this from Done to Under investigation in Arduino ESP32 Core Project Roadmap May 4, 2022
@P-R-O-C-H-Y P-R-O-C-H-Y removed the Resolution: Wontfix Arduino ESP32 team will not fix the issue label May 4, 2022
@P-R-O-C-H-Y
Copy link
Member

P-R-O-C-H-Y commented May 4, 2022

@quentinmit

I have read you description again and it make sense, to keep the state interrupt state so when changing pinMode, it won't be disabled. I will add this to roadmap :)

@P-R-O-C-H-Y P-R-O-C-H-Y added the Status: Needs investigation We need to do some research before taking next steps on this issue label May 4, 2022
@VojtechBartoska VojtechBartoska added this to the 2.0.4 milestone May 4, 2022
@quentinmit
Copy link
Author

Hi @quentinmit When calling pinMode(pin,mode); it configures the pin for default config, which is interrupt disabled. So you need to call attachInterruptArg(19, pin_interrupt, 0, CHANGE); again after pinMode to make it work :)

#include "Arduino.h"

// This sketch should print the number of times pin 19 changes.

uint32_t interrupt_count = 0;

void IRAM_ATTR pin_interrupt(void *arg) {
  interrupt_count++;
  pinMode(19, INPUT);
  attachInterruptArg(19, pin_interrupt, 0, CHANGE);
}

void setup() {
  pinMode(19, INPUT);
  attachInterruptArg(19, pin_interrupt, 0, CHANGE);
}

void loop() {
  ESP_LOGD("loop", "interrupt count %d", interrupt_count);
  delay(500);
}

Hi @P-R-O-C-H-Y. Your sample code is invalid because attachInterruptArg may not be called from an interrupt handler.

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

Hi @quentinmit
If you have a time, can you test changes in linked PR? That should fix your issue.
Let me know, if there is still a problem or fixed :)

@VojtechBartoska VojtechBartoska moved this from Under investigation to In Review in Arduino ESP32 Core Project Roadmap May 5, 2022
Repository owner moved this from In Review to Done in Arduino ESP32 Core Project Roadmap May 9, 2022
@VojtechBartoska VojtechBartoska added Status: Solved and removed Status: Needs investigation We need to do some research before taking next steps on this issue labels May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Peripherals API Relates to peripheral's APIs. Status: Solved
Projects
Development

Successfully merging a pull request may close this issue.

5 participants