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

wiring analogWrite() function does not respect previously set pinMode() #7836

Closed
afdeprado opened this issue Jan 23, 2021 · 3 comments · Fixed by #7841
Closed

wiring analogWrite() function does not respect previously set pinMode() #7836

afdeprado opened this issue Jan 23, 2021 · 3 comments · Fixed by #7841
Labels
waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.

Comments

@afdeprado
Copy link

Basic Infos

  • [X ] This issue complies with the issue POLICY doc.
  • [X ] I have read the documentation at readthedocs and the issue is not addressed there.
  • [X ] I have tested that the issue is present in current master branch (aka latest git).
  • [X ] I have searched the issue tracker for a similar issue.
  • [X ] If there is a stack dump, I have decoded it.
  • [X ] I have filled out all fields below.

Platform

  • Hardware: [ESP-12E]
  • Core Version: [master branch and 2.7.4 (3.20704.0)]
  • Development Env: [Platformio]
  • Operating System: [Windows]

Settings in IDE

  • Module: [Nodemcu]
  • Flash Mode: [do not know - not relevant]
  • Flash Size: [4MB]
  • lwip Variant: [do not know - not relevant]
  • Reset Method: [nodemcu]
  • Flash Frequency: [do not know - not relevant]
  • CPU Frequency: [80Mhz]
  • Upload Using: [SERIAL]
  • Upload Speed: [115200] (serial upload only)

Problem Description

The circuit I'm using the NodeMCU in requires PWM open-drain outputs. Even when setting the outputs to open-drain via pinMode(), passing OUTPUT_OPEN_DRAIN, the analogWrite() function resets the output to push-pull mode (OUTPUT).

The problem can be found in line 65 of the source file Arduino/cores/esp8266/core_esp8266_wiring_pwm.cpp (in the current master branch), where the pinMode() is reset to OUTPUT, regardless of what was previously set via this method.

To reproduce the problem, the sketch below can be run, with the output pin connected via a pull-down resistor (10k) to ground, and an oscilloscope connected to the output. The output should stay at 0V all the time (as pwm should either connect the output to GND, or leave it open, where the pull-down resistor would keep it at GND potential, if open-drain was respected). The observed output transitions between 0V and 3V3, as corresponds to a push-pull output configuration.

MCVE Sketch

#include <Arduino.h>

void setup() {
   pinMode(14, OUTPUT_OPEN_DRAIN);
   analogWrite(14, 128);
}

void loop() {
   // Nothing needed here
}

Debug Messages

No debug messages
@earlephilhower
Copy link
Collaborator

Interesting use case!

This is actually what the Arduino upstream versions do, so I suggest this is expected behavior:
https://github.com/arduino/ArduinoCore-avr/blob/master/cores/arduino/wiring_analog.c#L96-L104

You should be able to change the pinMode() immediately after your analogWrite(), or adjust your own fork of the core to remove the pinMode() command.

@earlephilhower earlephilhower added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jan 24, 2021
@klugem
Copy link
Contributor

klugem commented Jan 25, 2021

Funnily enough, I had the exact same problem yesterday. A constant current source I want to control via PWM expects an open-drain PWM signal.

I would suggest to add an optional parameter to analogWrite to allow setting the pin to OUTPUT_OPEN_DRAIN mode. I opened a pull request with that change #7841.

@afdeprado
Copy link
Author

Michael, I think your solution is a very good alternative, as it doesn't disrupt the current working, nor compatibility with upstream Arduino versions, but solves a problem (and probably, an unintended bug in the original Arduino wiring libraries). I like the final proposed solution with a new analogWriteEx method. For my current project, I just duplicated the method and edited it to suit my purposes, as I was in a hurry.

Thanks everyone involved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants