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

analogWrite is reset to the minimum or maximum value #4944

Closed
X-Stas-EEE opened this issue Jul 20, 2018 · 10 comments
Closed

analogWrite is reset to the minimum or maximum value #4944

X-Stas-EEE opened this issue Jul 20, 2018 · 10 comments
Assignees
Labels
type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Milestone

Comments

@X-Stas-EEE
Copy link

Platform

  • Hardware: ESP-12E
  • Core Version: win-2.5.0-dev
  • Development Env: Arduino IDE
  • Operating System: Windows

Settings in IDE

  • Module: Nodemcu 1.0
  • Flash Mode: ?
  • Flash Size: 4MB
  • lwip Variant: v2 Lower Memory
  • Reset Method: nodemcu?
  • Flash Frequency: 40Mhz?
  • CPU Frequency: 80Mhz
  • Upload Using: SERIAL
  • Upload Speed: 256000

Problem Description

AnalogWrite is reset to zero or maximum value. The plugged led gets stuck on the maximum PWM level or turns off. At the same time the buzzer continues to play sounds. Thus, it becomes clear that the module does not hang. To replicate the problem run the attached code and wait for a while. It may take a few hours. The problem is mentioned in #4640

Sketch

#define LED_PIN D1 // GPIO5
#define BUZ_PIN D2 // GPIO4
int bright;
bool increase;
int note;
unsigned long cur_millis = millis();
unsigned long prev_millis = cur_millis;

void setup() {
  Serial.begin(115200);
  Serial.print("\n\nSDK Version=");
  Serial.println(ESP.getCoreVersion());
  pinMode(LED_PIN, OUTPUT);
  pinMode(BUZ_PIN, OUTPUT);
  bright = 0;
  increase = true;
}

void loop() {
  cur_millis = millis();
  if (abs(cur_millis - prev_millis) > 100) {
    analogWrite(LED_PIN, bright);
    if (increase) {
      bright++;
    } else {
      bright--;
    }
    if (bright >= 105) {
      increase = false;
    } else if (bright <= 10) {
      increase = true;
    }
    prev_millis = cur_millis;
    tone(BUZ_PIN, (double)bright+200, 85);
  } 
}

Debug Messages

SDK:2.2.1(cfd48f3)/Core:win-2.5.0-dev/lwIP:2.0.3(STABLE-2_0_3_RELEASE/glue:arduino-2.4.1-10-g0c0d8c2)/BearSSL:94e9704


SDK Version=00000000
scandone
state: 0 -> 2 (b0)
state: 2 -> 3 (0)
state: 3 -> 5 (10)
add 0
aid 4
cnt 

connected with HUT, channel 1
dhcp client start...
ip:192.168.1.7,mask:255.255.255.0,gw:192.168.1.1
pm open,type:2 0

@earlephilhower
Copy link
Collaborator

@X-Stas-EEE I'll try and repro and look at this tonight. If you have the chance, could you add a
WiFi.mode(WIFI_OFF);
to the beginning of the setup() loop to verify that it's not related in any way to the WiFi stack (which does use interrupts and other long-running processes in the background)? If it fails with WiFi on (like your run) but passes with it off, that's a good pointer.

@X-Stas-EEE
Copy link
Author

@earlephilhower OK, I'll run the test again with this line. Hope the bug will be caught this night. Otherwise, I'll leave the ESP running for a couple of days. I will not be able to observe the execution of the test this weekend.

@X-Stas-EEE
Copy link
Author

X-Stas-EEE commented Jul 21, 2018

@earlephilhower The bug is still there, even with the Wi-Fi switched off. The modified sketch:

#include <ESP8266WiFi.h>

#define LED_PIN D1 // GPIO5
#define BUZ_PIN D2 // GPIO4
int bright;
bool increase;
int note;
unsigned long cur_millis = millis();
unsigned long prev_millis = cur_millis;

void setup() {
  WiFi.mode(WIFI_OFF);
  Serial.begin(115200);
  Serial.print("\n\nSDK Version=");
  Serial.println(ESP.getCoreVersion());
  pinMode(LED_PIN, OUTPUT);
  pinMode(BUZ_PIN, OUTPUT);
  bright = 0;
  increase = true;
}

void loop() {
  cur_millis = millis();
  if (abs(cur_millis - prev_millis) > 100) {
    analogWrite(LED_PIN, bright);
    if (increase) {
      bright++;
    } else {
      bright--;
    }
    if (bright >= 105) {
      increase = false;
    } else if (bright <= 10) {
      increase = true;
    }
    prev_millis = cur_millis;
    tone(BUZ_PIN, (double)bright+200, 85);
  } 
}

@earlephilhower
Copy link
Collaborator

earlephilhower commented Jul 21, 2018

Thanks! I think this is related to

if (now >= wave->nextServiceCycle) {

I should be subtracting the two values and checking for <0. As-is, I think I can make it fail by making nextservicecycle = 2^32-1 cycles and the ESP cycle counter overflowing before I get to handfle it. In that case, this edge will not be caught because now=1, which is past the service time but < nextservicecycle.

I will try and repro the failure on my system tonight, then do the fix and see if it lasts a full day.

@earlephilhower earlephilhower added this to the 2.4.2 milestone Jul 21, 2018
@earlephilhower
Copy link
Collaborator

Was able to catch it after only ~25 minutes of runtime. It has a chance of self-correcting every 2^32/80M = ~53 seconds due to counter wraparound. Just did the fix (1-liner) and restarted and started my logic analyzer. With any luck the USB analyzer will run overnight and not crash (crappy analyzer, unrelated to the code or ESP8266!) and I can dump and post-process to verify no PWM freezes.

earlephilhower added a commit to earlephilhower/Arduino that referenced this issue Jul 21, 2018
When the ESP cycle counter rolls over, the "now" can be smaller than the
next-edge time of a waveform generator.  This would cause the edge to be
missed on that specific pin, and make it look like PWM was hung.

Use proper comparison between current time and edge time.

Fixes esp8266#4944

Also remove the "sigma-delta.c.unused" file which was replaced by a
working one some time ago.
@earlephilhower
Copy link
Collaborator

earlephilhower commented Jul 21, 2018

The logic analyzer only captured ~2.5hrs before stopping, but analysis of that time shows it was clean, and the LED was still fading in and out this morning, so I think we've got a fix!

Please give PR #4945 a try, @X-Stas-EEE.

@earlephilhower earlephilhower added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jul 21, 2018
earlephilhower added a commit that referenced this issue Jul 22, 2018
When the ESP cycle counter rolls over, the "now" can be smaller than the
next-edge time of a waveform generator.  This would cause the edge to be
missed on that specific pin, and make it look like PWM was hung.

Use proper comparison between current time and edge time.

Fixes #4944

Also remove the "sigma-delta.c.unused" file which was replaced by a
working one some time ago.
@X-Stas-EEE
Copy link
Author

@earlephilhower
I've just run the test. Will wait for several hours.
The first observation: it seems that the maximum brightness level became lower.

@earlephilhower
Copy link
Collaborator

Cool, looking fwd. to the update!

FWIW the brightness levels should be absolutely unaffected, the change was related to it missing an edge when the edge happened near a timer wraparound (~every minute IIRC) which would end up causing a full-on or full-off until (at least) the next time the wraparound occurred. Pretty obvious problem in the code, but only hit in cases when the interrupt was a little late (due to other things, like add'l waveforms).

@X-Stas-EEE
Copy link
Author

X-Stas-EEE commented Jul 23, 2018

Everithing looks good so far. I've deleted my previous comment because it contained erroneous information.

@X-Stas-EEE
Copy link
Author

@earlephilhower I confirm that the bug has been fixed! Thanks a lot for your work!

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

No branches or pull requests

2 participants