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

PWM flickering with small analogWrite values [$15] #836

Closed
ge0rg opened this issue Sep 29, 2015 · 71 comments
Closed

PWM flickering with small analogWrite values [$15] #836

ge0rg opened this issue Sep 29, 2015 · 71 comments

Comments

@ge0rg
Copy link

ge0rg commented Sep 29, 2015

Hi, running the ESP8266 to control an RGB LED set via PWM on pins 12, 13, 15 using analogWrite().

When setting very low PWM values (<5), there is occasional flickering of the LEDs (the light turns to full brightness for a noticeable moment). Sometimes this is reinforced by network traffic, sometimes it happens without any apparent activity. The flickering is unrelated to the time when analogWrite() is called.

It helps a little to reduce the PWM frequency from 1KHz to 200Hz, but the flickering still happens from time to time.

I am aware that the PWM is based on a timer IRQ, not a hardware implementation, and I presume this issue is triggered by some other part of the code (wifi? tcp?) disabling interrupts for a short moment, leading to the timer interrupt being missed.

Unfortunately, the code in core_esp8266_wiring_pwm.c is not well documented and it's hard to understand for an outsider what exactly is happening there.

Would it be possible to mitigate the issue in the timer code? Maybe change it from edge- to level triggered or have the ISR called more often?

There is a $15 open bounty on this issue. Add to the bounty at Bountysource.

@igrr
Copy link
Member

igrr commented Sep 29, 2015

Timer interrupt is edge triggered — this is part of the chip design and it is not possible to change it in software. I suppose using NMI for PWM would fix this problem because NMI is higher priority than WiFi interrupts.

@ge0rg
Copy link
Author

ge0rg commented Sep 29, 2015

Would it be possible to try that out with moderate overhead? s/timer1/???/ in the pwm source?

@igrr
Copy link
Member

igrr commented Sep 29, 2015

You may try replacing ETS_FRC_TIMER1_INTR_ATTACH(timer1_isr_handler, NULL)
with ETS_FRC_TIMER1_NMI_INTR_ATTACH(timer1_isr_handler)
in core_esp8266_timer.c, line 45. Not sure if it will work though.

edit: Most likely there will be issues because ETS_FRC1_INTR_DISABLE will no longer work.

@ge0rg
Copy link
Author

ge0rg commented Sep 30, 2015

Replacing the regular interrupt with NMI leads to a watchdog reset ~10 seconds after startup:

ets Jan  8 2013,rst cause:4, boot mode:(1,6)
wdt reset

I even tried feeding the (soft) dog with system_soft_wdt_feed from the ISR, to no avail. It seems there is no function to feed the hardware watchdog, though.

Setting the timer to TIM_LOOP doesn't seem to improve the situation either - I hoped that a repeat of the ISR if it was missed will actually lead to some useful results.

I think I'll try the pwm_* functions from the official SDK next; let's hope they have some magic sauce to provide better stability.

@igrr
Copy link
Member

igrr commented Sep 30, 2015

As I mentioned, current implementation will have issues with NMI because it relies on a "critical section" implemented using ETS_FRC1_INTR_DISABLE. So the right thing to do would be fix this by disabling the timer or detaching NMI interrupt for the duration of the critical section.

@ge0rg
Copy link
Author

ge0rg commented Oct 5, 2015

I'm not sure this has to do with the PWM critical section at all. Merely adding a ETS_FRC_TIMER1_NMI_INTR_ATTACH(noop_function); to the code leads to a wdt reset. It rather seems to me that the espressif code is using an Nmi handler internally for watchdog purposes, and as long as we don't know which function was added as the Nmi handler, we can't just override it (without calling into it from our own handler).

I'd like to help out with that, but my knowledge of the platform and the different SDKs is rather limited. I'd be glad to get one or two pointers in the right direction.

P.S: When using the IoT_Demo from 1.3, the PWM is much more smooth and reliable, and it's got a usable value range of ~ 0..22222.

@mangelajo
Copy link
Contributor

Hmm, I tried this early this weekend, and I found the flickering.

Analyzing the code I saw that we block the interrupt of the timer while we're copying the old values to the new ones (so the timer won't get half-copied info).

Also, I found that flickering is more likely to happen while you're doing heavy operations over the flash.

I think that a good strategy, and probably no NMI needed:

  1. Move all the ISR/PWM the code to ICACHE RAM, so there's no issues with caching, and blocked flash.

  2. Instead of disabling the interrupt for mutex, I'd do a mutex via a flag.
    a) The setter code, waits for the flag to be clear, before trying to set any new values,
    b) Set's the new value in a set of structures (mailbox-like),
    c) Set's the flag
    d) ISR updates PWM with any value it already had (to avoid any jitter)
    e) ISR finds the flag, and copies the new values to a final structure it will use.
    f) ISR clears the flag.

Does it sound reasonable?

@igrr
Copy link
Member

igrr commented Oct 6, 2015

I'm fixing the first part (moving handlers to RAM), commit coming in a few minutes.
Your approach with a flag does sound reasonable, actually it's the right way to go even if we are going to use NMI. I think NMI is necessary because otherwise we have flickering when there is heavy WiFi traffic.

@igrr
Copy link
Member

igrr commented Oct 6, 2015

First part pushed, fixing #819.

@ge0rg
Copy link
Author

ge0rg commented Oct 6, 2015

Wow, this really is a significant improvement! I'm test running the new code now and it looks much more stable.

I'm still experiencing a slight flickering when the PWM values get changed (i.e. when running a color fader), and a more noticeable flickering when one of the PWM values is switching between 0 and non-0, which seems to trigger a longer code path.

I wonder if it would be acceptable to mark all of the routines behind analogWrite as ICACHE_RAM_ATTR to solve the first problem.

I hope the second problem will be addressed with the flag-based ISR rewrite.

@Diaoul
Copy link
Contributor

Diaoul commented Oct 11, 2015

I'm experiencing the same issue with GPIO0 and GPIO2. ESP8266 is listening for POST requests and changes the value accordingly so that means that when the change happens, there is some network load. My knowledge is very limited on the ESP8266 so I don't know if I can be of any help troubleshooting this, however I can test the patches.

@Diaoul
Copy link
Contributor

Diaoul commented Oct 30, 2015

I'd be glad to help though my knowledge is limited, can you point me where to start and what should be done?

@demlak
Copy link

demlak commented Nov 1, 2015

same here.. sketch to reproduce:

const int pin = 13;
void setup() {
  pinMode(pin,OUTPUT);
  Serial.begin(115200);
}
void loop() {
  analogWrite(pin, 1);
  delay(10); //to not overload esp8266
}

btw.. same flickering on analog Values >= 1019 as on <=4

edit:
value 5 and 1018 also has flickering.. but very rarely, compared with the other values..

edit2:
value 6 and 1017 also flickering.. but very very rarely... like 2-3 times an hour or so..

@Diaoul
Copy link
Contributor

Diaoul commented Nov 1, 2015

Using master branch is a lot better than the releases however some flickering still occur on network trafic, it's blinking once then back to the correct PWM setting.

@igrr igrr modified the milestone: 2.0.0 Nov 5, 2015
@ArnieO
Copy link

ArnieO commented Nov 26, 2015

I am experiencing the same flickering on ESP-03s.
@igrr : I have several ESP-8266 LED faders that have been running for several months with LUA code. I have never seen any glitching there, so I am not so certain if this is really only a HW issue?
My workaround until further will be to make sure the code avoids the sensitive value intervals.

@igrr
Copy link
Member

igrr commented Nov 26, 2015

This isn't a hardware issue. @mangelajo described software changes that need to be done pretty accurately.

@Diaoul
Copy link
Contributor

Diaoul commented Nov 26, 2015 via email

@ArnieO
Copy link

ArnieO commented Nov 26, 2015

I found a workaround that seems to work well for me:
The default PWM frequency is 1000 Hz. By reducing to 200 Hz I got rid of the flickering on my module. (I tested step by step until the flickering disappeared).
PWM frequency is set to 200 Hz by this code:
analogWriteFreq(200);

@igrr igrr modified the milestones: 2.0.0, 2.1.0 Nov 30, 2015
@demlak
Copy link

demlak commented Dec 6, 2015

thx a lot for sharing this workaround...
seems to fix the given problem of random flickering..

BUT opens another problem: 200hz seems to be too low to give a smooth output.. no random flickering of different brightnes anymore.. but continue flickering because of too low frequency..

@ArnieO
Copy link

ArnieO commented Dec 6, 2015

I know 200 Hz is a bit low but on the ESP-03 I use I find the output to be smooth except for the very lowest setting, when the light is anyway almost invisible. So for me, the workaround is fully acceptable and without any noticeable drawback.

@demlak
Copy link

demlak commented Dec 6, 2015

i´ll take a look at a low-pass filter for me.. but this is not ideal..

EDIT:
nevermind.. i just tested.. analogwrite value of 1.. with 200hz..
there are still brightnes flicker sometimes...

so.. problem still exists.. also at 200hz

@9H1LO
Copy link

9H1LO commented Feb 2, 2016

issue still exists, flicker to full o/p when there is network traffic, i notice it when using more than 3 pwm channels, seems ok at 200hz up to 3ch i need 4 channels (rgbw with mqtt) and pca9685 will be over kill although i found it to work great

@krissfr
Copy link

krissfr commented Feb 4, 2016

i have the problem too, i need to control 5 pwm output (3 leds, 2 fans) from mqtt messages, but each wifi transmission flicker all leds (and make the fan noisy during the transmission). i need to use a high pwm frequency for the fan, so workaround are not good for my usage.

I hope somone with a high esp8266/C++ developpement skill will find a solution

@micw
Copy link

micw commented Nov 20, 2016

Hello,

I had serious problems with flickering when I fade my LEDs. PWM seems not to flicker when the LEDs are dimmes to a certain value, only when analogWrite is called with changing values.

I completely solved it by disabling interrupts before calling analogWrite and re-enable afterwards. There's my minimal sketch:

void setup() {
  analogWriteFreq(1000);
  pinMode(D1, OUTPUT);
  pinMode(D2, OUTPUT);
  pinMode(D3, OUTPUT);
  Serial.begin(115200);
  Serial.println(PWMRANGE);
}

int value=0;
int delta=1;

void loop() {
  value+=delta;
  if (value>1023) {
    value=1023;
    delta=-delta;
  } else if (value<0) {
    value=0;
    delta=-delta;
  }
  noInterrupts();
  analogWrite(D1,value);
  analogWrite(D2,value);
  analogWrite(D3,value);
  interrupts();
  delay(1);
}

Without noInterrupts/interrupts it flickers like hell. My guess is that it is a race condition when writing new pwm value while PWM interrupt is executed.

Regards,
Michael.

@micw
Copy link

micw commented Dec 7, 2016

Update: Flicker occurs again for some reason on my sketch when there's network traffic ;-(

@micw
Copy link

micw commented Dec 7, 2016

I have switched my sketch to use https://github.com/StefanBruens/ESP8266_new_pwm. I use a period of 5000 which gives me perfect 1kHz pwm. No flicker at all, works like charm.
Only drawback is that the API is not as clean as analogWrite().

IMO you really should replace the current implementation by this one.

@simonliu009
Copy link

simonliu009 commented Dec 28, 2016

@micw Does https://github.com/StefanBruens/ESP8266_new_pwm mean you switch to the ESP8266 SDK?

@micw
Copy link

micw commented Jan 1, 2017

@simonliu009 no, you can still use arduino ide with esp8266 extensions.

@JoaoLopesF
Copy link

Hi,

I made one sample to use the https://github.com/StefanBruens/ESP8266_new_pwm in Arduino.
See it in: https://github.com/JoaoLopesF/ESP8266-Arduino-PWM-SDK-Sample
Only this works fine to me.
Regards,
Joao

JK-de added a commit to JK-de/ESPEasyPluginPlayground that referenced this issue Apr 19, 2017
with fading and gamma correction.
stable release.

Note: ESP-software-PWM has flickering problems with values <6 and >1017. See esp8266/Arduino#836
psy0rz pushed a commit to letscontrolit/ESPEasyPluginPlayground that referenced this issue Apr 19, 2017
with fading and gamma correction.
stable release.

Note: ESP-software-PWM has flickering problems with values <6 and >1017. See esp8266/Arduino#836
@devyte
Copy link
Collaborator

devyte commented Oct 20, 2017

@igrr I see two possibilities here: migrate to the assembly pwm, or finish the original proposal with the flag.
What are your thoughts here?

@igrr
Copy link
Member

igrr commented Oct 20, 2017

ESP8266_new_pwn is GPLv2, so we can't use it here, unfortunately.
Fixing the race condition in the existing code should be a feasible option, i think.

@devyte
Copy link
Collaborator

devyte commented Oct 20, 2017

@igrr I opened an issue there asking if the license can be changed to the same one in this repo. If so, we can continue that part of the discussion, and maybe contrast the two approaches. If not, so be it, we stick to the original plan.
Does that sound good?

@igrr
Copy link
Member

igrr commented Oct 20, 2017 via email

@d-a-v
Copy link
Collaborator

d-a-v commented Dec 13, 2017

+1s are now and currently needed at @devyte's issue in ESP8266_new_pwn

@devyte
Copy link
Collaborator

devyte commented Jun 20, 2018

@earlephilhower does #4640 address this?

@earlephilhower earlephilhower self-assigned this Jun 25, 2018
@earlephilhower
Copy link
Collaborator

@devyte A quick test shows no flickering for a few minutes at 1023/1024 value, but this is something I'd hope someone with a real DSO or logic analyzer could test (set a trigger on a pulse > than 20us and let it run overnight at 123/1024).

const int pin = LED_BUILTIN;
void setup() {
  pinMode(pin,OUTPUT);
  analogWriteFreq(1000);
  analogWriteRange(1024);
}
void loop() {
  analogWrite(pin, 1023);
  delay(10); //to not overload esp8266
}

Also see PWM was NMI (whereas Tone() and Servo() were not) in the original code. The current waveform generator is not NMI, so can have that ~10us or so jitter under heavy load. Doesn't cause visible changes, but this may be more than the old code was producing since it was guaranteed CPU priority/ It's a 2-line change to go to NMI, but unless we hear of some specific issue I'd rather not use NMI as I don't have enough info on how it would affect the WiFi and other parts of the chip.

@devyte
Copy link
Collaborator

devyte commented Jul 12, 2018

@earlephilhower I don't think a DSO is needed to close this issue. The original description talks about visible flickering of an RGB led. I suggest just testing for visible flickering under heavy WiFi load, such as @d-a-v 's stress test, on a board that has an RGB led like the WittyCloud or similar. If no flickering, we're good.

@devyte
Copy link
Collaborator

devyte commented Jul 12, 2018

Per discussion with @earlephilhower and his test results, this is not reproducible. Closing.

@devyte devyte closed this as completed Jul 12, 2018
@devyte devyte modified the milestones: 2.5.0, 2.4.2 Jul 12, 2018
@crywolf87
Copy link

sorry, It's possible create a pwm with 5 hz frequency to esp8266?

@nos317
Copy link

nos317 commented Dec 14, 2019

I have the same question of @crywolf87
Anyone have a solution?

@JAndrassy
Copy link
Contributor

sorry, It's possible create a pwm with 5 hz frequency to esp8266?

see the Arduino standard BlinkWithoutDelay example. it is 1 Hz. change the interval to 200 and you get 5 Hz

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

No branches or pull requests