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

[ESP8266 - core 2.5.0]: BUGS in analogWrite functions #5957

Closed
2 tasks
hasenradball opened this issue Apr 7, 2019 · 18 comments
Closed
2 tasks

[ESP8266 - core 2.5.0]: BUGS in analogWrite functions #5957

hasenradball opened this issue Apr 7, 2019 · 18 comments
Assignees
Labels
waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.

Comments

@hasenradball
Copy link
Contributor

----------------------------- Delete above -----------------------------

Basic Infos

  • 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.
  • If there is a stack dump, I have decoded it.
  • [x ] I have filled out all fields below.

Platform

  • Hardware: ESP12E
  • Core Version: 2.5.0
  • Development Env: Arduino IDE 1.88
  • Operating System: Windows 10

Settings in IDE

  • Module: NodeMCU1.0 (ESP12E)
  • Flash Mode:
  • Flash Size: 4MB
  • lwip Variant:
  • Reset Method:
  • Flash Frequency:
  • CPU Frequency:
  • Upload Using: Serial
  • Upload Speed:

Problem Description

If you use the function analogWrite(PIN, PWM) the you will find the following issues:

1.) the change of PWMRANGE via analogWriteRange(new_range) is not possible!
-> if you set 1000 or 255 for example there ist no change -> only 1023.

2.) if you write analogWrite(PIN, 0) the voltage is not zero -> an LED will not go off
-> if you write digitalWrite(PIN, LOW) the LED goes off.
-> for my understanding digitalWrite(PIN, LOW) and analogWrite(PIN, 0) should have the same effect on the Pin!
-> checked via Oszi

MCVE Sketch

#include <Arduino.h>
// #include <ESP8266WiFi.h>
#define LED 2
#define LED2 4

int brightness = 0; // how bright the LED is
int fadeAmount = 5; // how many points to fade the LED by

//=======================================================================
// Power on setup
//=======================================================================
void setup() {
Serial.begin(115200);
pinMode(LED, OUTPUT);
pinMode(LED2, OUTPUT);
analogWriteRange(1023);
delay(100);
Serial.println();
Serial.println(PWMRANGE);
}

//=======================================================================
// Main Program Loop
//=======================================================================
void loop() {
delay(5000);

for(int i = 0; i < PWMRANGE; i = i + PWMRANGE/10) {
analogWrite(LED, constrain(i, 0, PWMRANGE));
analogWrite(LED2, constrain(PWMRANGE - i, 0, PWMRANGE));
Serial.print("PWM: ");
Serial.print(i);
Serial.print(" \n\n");
delay(2000);
}

digitalWrite(LED2, HIGH);
delay(10000);

digitalWrite(LED2, LOW);
delay(10000);

//Continuous Fading
Serial.println("Fadding Started");
while(1)
{
// set the brightness of pin 9:
analogWrite(LED, brightness);
analogWrite(LED2, PWMRANGE - brightness);

// change the brightness for next time through the loop:
brightness = constrain(brightness + fadeAmount, 0, PWMRANGE);
//Serial.println(brightness);

// reverse the direction of the fading at the ends of the fade:
if (brightness <= 0 || brightness >= PWMRANGE) {
  fadeAmount = -fadeAmount;
  delay(4000);
}
// wait for 30 milliseconds to see the dimming effect
delay(20);

}
}
//=======================================================================

Debug Messages

Debug messages go here
@devyte
Copy link
Collaborator

devyte commented Apr 9, 2019

the change of PWMRANGE via analogWriteRange(new_range) is not possible

Correct. PWMRANGE is not some variable that can be changed, it's a macro defined to the capability of the underlying hardware. Therefore, analogWriteRange(PWMRANGE) sets the range to the max that the hw allows, while e.g.: analogWriteRange(400) sets the range to a lower value.
There is no getter function to get the currently set range.

A Note: I thought Arduino had the same interface, but I just checked, and it doesn't. Instead, it has analogWriteResolution(bits). I guess this could be implemented in terms of our current analogWriteRange(), i.e. along the lines of:
void analogWriteResolution(uint32_t bits) {analogWriteRange(<insert correct calculation here>);}

digitalWrite(PIN, LOW) and analogWrite(PIN, 0) should have the same effect

Our pwm is software-based, there is no pwm hw on the ESP.
@earlephilhower what is the effect of setting a 0-value in the waveform generator? Is the pin not guaranteed to be off?

@devyte devyte added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Apr 9, 2019
@earlephilhower
Copy link
Collaborator

PWMRANGE is different on the 8266 vs the AVR and had been since the beginning. If you're expecting it to equal 256, that would be the first problem.

analogWrite(PIN, 0) in core_esp8266_wiring_pwm.cpp simplifies to

pinMode(pin, OUTPUT);
stopWaveform(pin);
digitalWrite(pin, LOW);

So from my own experience and the code examination it looks like it is fine, too.

@hasenradball
Copy link
Contributor Author

Hi, thanks for your answer.
So if I change the PWmrange via analogWriteRange() I can not check it via PWMRANGE, right?
But is it possible toe set it to Values e.g. 255, 1000, ect...?

So alnalogWrite(PIN, 0) should be the same as digitalWrite(LOW), right?

@devyte
Copy link
Collaborator

devyte commented Apr 10, 2019

can not check it via PWMRANGE

Correct.

But is it possible toe set it to Values e.g. 255, 1000

Yes.

So alnalogWrite(PIN, 0) should be the same as digitalWrite(LOW)

Per @earlephilhower, it is the same as the three lines of code he said.

Closing as user error for the first part and can't reproduce for the second. Also, your code is incomplete, and I see at least one bug in your i calculation, so also user error for the second part.

@devyte devyte closed this as completed Apr 10, 2019
@hasenradball
Copy link
Contributor Author

I will check again with the new infomation in mind.
I also checked the code in waveform the
Pwm =0 should be the same as pin LOW.

@hasenradball
Copy link
Contributor Author

hasenradball commented Apr 10, 2019

Ok I checked the behaviour again.
The following result I can tell.
Look to this part of code:

 analogMap &= ~(1 << pin);
  uint32_t high = (analogPeriod * val) / analogScale;
  uint32_t low = analogPeriod - high;
  pinMode(pin, OUTPUT);
  if (low == 0) {
    stopWaveform(pin);
    digitalWrite(pin, HIGH);
  } else if (high == 0) {
    stopWaveform(pin);
    digitalWrite(pin, LOW);
  } else {
    if (startWaveform(pin, high, low, 0)) {
      analogMap |= (1 << pin);
    }
}

The part1 if (low == 0) ... works the PIN is then LOW completly, BUT

The part2 if(high == 0)... does not work!
if PWMRANGE is 1023 and I set the PIN to 1023 the PIN is not HIGH!
More than 1 µs are missing in the PWM to reach 100% at 1 kHz.

@hasenradball
Copy link
Contributor Author

And if I set analogWriteRange(100) for example both sides will not be reached not high and not low.

@hasenradball
Copy link
Contributor Author

Here you see a screenshot at analogWrite(PIN, 1023).
If #define PWM_RANGE 1023.

DS1Z_QuickPrint2

@hasenradball
Copy link
Contributor Author

I would say we should have a look for a better implementation. Because the reaching of the Endpoint HIGH and LOW depends on the PWMRANGE and the frequency.

@devyte
Copy link
Collaborator

devyte commented Apr 10, 2019

@hasenradball In the code referenced above for analogWrite(), I understand that high and low are the times the pin should be high and low within a pwm period.
Then, for analogFreq=1000, analogScale=1023 (default) I see the following:

For val=1023:
analogPeriod = 1000000 / analogFreq = 1000
high = analogPeriod * val /analogScale = 1000 * 1023 / 1023 = 1000
low = analogPeriod - high = 1000 - 1000 = 0
=> correct.

for val=0:
analogPeriod = 1000000 / analogFreq = 1000
high = analogPeriod * val /analogScale = 1000 * 0 / 1023 = 0
low = analogPeriod - high = 1000 - 0 = 1000
=> correct.

For analogFreq=1000, analogScale=100, I see the following:

For val=100:
analogPeriod = 1000000 / analogFreq = 1000
high = analogPeriod * val /analogScale = 1000 * 100 / 100 = 1000
low = analogPeriod - high = 1000 - 1000 = 0
=> correct.

for val=0:
analogPeriod = 1000000 / analogFreq = 1000
high = analogPeriod * val /analogScale = 1000 * 0 / 100 = 0
low = analogPeriod - high = 1000 - 0 = 1000
=> correct.

In all cases above I think low==0 and high==0 is in fact being hit.

Beware your logic:. In the code in the first post:
for(int i = 0; i < PWMRANGE; i = i + PWMRANGE/10)
PMWRANGE is not divisible by 10, so the above will miss the last iteration: the max value for i will be 1020.
Then:
analogWrite(LED2, constrain(PWMRANGE - i, 0, PWMRANGE));
the subtraction is 1023 - 1020 which yields 3, and not 0.

I suggest thinking your code through carefully.

@hasenradball
Copy link
Contributor Author

Hi, I adapted the code today so that I reached in a loop 1023 and 0.
The code looks like:

#include <Arduino.h>
// #include <ESP8266WiFi.h>
#define LED 2
#define LED2 4
#define PWM_RANGE 1023

 
int brightness = 0;    // how bright the LED is
int fadeAmount = 1;    // how many points to fade the LED by
 
//=================
//   Power on setup
//==================
void setup() {
  Serial.begin(115200);
  pinMode(LED, OUTPUT);
  pinMode(LED2, OUTPUT);
  analogWriteRange(PWM_RANGE);
  delay(100);
  Serial.println();
  Serial.println(PWM_RANGE);
}

//==============
//  Main Program Loop
//==============

void loop() {
  delay(5000);
  
  analogWrite(LED, 0);
  analogWrite(LED2, PWM_RANGE);
  delay(5000);

  analogWrite(LED, PWM_RANGE);
  analogWrite(LED2, 0);
  delay(5000);
 
  //Continuous Fading
  Serial.println("Fadding Started");
  while(true)
  {
    // set the brightness of pin 9:
    analogWrite(LED, brightness);
    analogWrite(LED2, PWM_RANGE - brightness);
  
    // change the brightness for next time through the loop:
    brightness = constrain(brightness + fadeAmount, 0, PWM_RANGE);
    //Serial.println(brightness);
    
    // reverse the direction of the fading at the ends of the fade:
    if (brightness <= 0 || brightness >= PWM_RANGE) {
      fadeAmount = -fadeAmount;
      delay(4000);
    }
    // wait for 30 milliseconds to see the dimming effect
    delay(20);
  }

Your calculations looks right, but the Oszi show the difference.
Have you an idea whats wrong?

@hasenradball
Copy link
Contributor Author

Ok the code of calculating high and low looks fine.
is it possible to make a debugging in the pwm class?
Like a serial print for instance.

Hypothesis:
A) code does not behave than calculated above
B) code is correct an we had a hw issue in pwm.

@devyte
Copy link
Collaborator

devyte commented Apr 11, 2019

Our sw PWM is implemented as a specific case of the waveform generator. It is this which allows it to work at the same time as e.g.: the Servo class, or the buzzer.
The waveform generator was implemented by @earlephilhower , and per his comment he doesn't see what you see.
That means it's up to you to prove there is an issue.
You would have to dig into the waveform generator behavior for the 0 and 1023 cases to check what is going on in your case.

@hasenradball
Copy link
Contributor Author

Thanks for your reply,
Thats right what you say, but I see it on the Scope.
Is there a debugging method during run available or via Serial?
Or is it only possible to review the code?

@earlephilhower
Copy link
Collaborator

@hasenradball Can you give a 2-3 line MCVE snippet with your specific analog frequency and values being written that fails? Some simple straight-line code without loops or calculations that doesn't do what you think it should?

Something like:

  analogWriteRange(1023);
  analogWrite(D1, 1000); // See duty cycle of 98% on scope
  delay(1000);
  analogWrite(D1, 0);
  delay(1000);  // Expect to see 0 on scope, see duty cyclke of XXX% instead

That way we isolate any core issue from the calling app, and it's trivial to reproduce by a 3rd party.

@hasenradball
Copy link
Contributor Author

@earlephilhower Hi thanks for your reply.
As I see for the moment the issue what I saw in the scope could be a triggering issue.
I will check it again but I think it was a Measurement issue caused by Trigger definition.
I am sorry for the effort :-)

@hasenradball
Copy link
Contributor Author

hasenradball commented Apr 12, 2019

@earlephilhower Hi, ist it possible to send you a small video from the iphone to show you the issue I mean?
And then to discuss to check how to verify it.

The following issues I see on the scope:

  • with PWMRANGE = 1023 fading from 0...1023 and vice versa -> at 100% it is not switched to HIGH!
  • with PWMRANGE < 1023 fading from 0...1023 and vice versa -> at 100% not HIGH and at 0% not LOW!

BUT: setting without fading it seems to be fine.

@hasenradball
Copy link
Contributor Author

@earlephilhower Hi I think we can close the topic. The issue was a triggering issue on the scope.
With the following MCVE I can not reproduce the issue.

ESP8266_LED_PWM.zip

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

No branches or pull requests

3 participants