analogWrite changed values which are not max or min aren't updated to the output

[EricMc1289]

@earlephilhower

Hi, maybe there is a bug in the pwm function of analogWrite with the latest changes. I have testet 2.4.2 and latest git. If you try to set 3 pwm channels at once like in the function below to set the rgb color of a LED Stripe, you wont get every color updated correctly.

void setColorToOutput(uint8_t r, uint8_t g, uint8_t b) {
      analogWrite(14, r);
      analogWrite(12, g);
      analogWrite(13, b);
}

The configuration of the pwm is as follows for both functions:

      analogWriteFreq(1000);
      analogWriteRange(255);

There was a very strange behaviour: F.E. if i got to the color yellow direktly (r=255, g=150, b=0) there was only red LEDs on but when i switched from green (r=0, g=255,b=0) to yellow it seems to work but with (g=255). From my research of this behavior i think there is a problem with updating the values, when they are not min or max. If you update to min or max it works but if you update within the range of the pwm, so if the old value is not 0 or 255, in my case , it will not update the output.

While testing and trying to fix that i found the following solution by changing my function a littlebit:

static uint8_t rOld = 0;
static uint8_t gOld = 0;
static uint8_t bOld = 0;

void setColorToOutput(uint8_t r, uint8_t g, uint8_t b) {
      if (r != rOld) {
            analogWrite(14, 0);
            analogWrite(14, r);
            rOld = r;
      }
      if (g != gOld) {
      analogWrite(12, 0);
      analogWrite(12, g);
      gOld = g;
      }
      if (b != bOld) {
            analogWrite(13, 0);
            analogWrite(13, b);
            bOld = b;
      }
}

These changes are a very quick and dirty patch and now it works like a charm. So I assume there is somewhere a bug in updating the pwm values for an existing waveform object.

[earlephilhower]

@EricMc1289 please supply a MCVE, I cannot reproduce any problem with my own testing.

void setup() {
  // put your setup code here, to run once:
      analogWriteFreq(1000);
      analogWriteRange(255);
}

void loop() {
  // put your main code here, to run repeatedly:
  analogWrite(D3, 10);
  delay(50);
  analogWrite(D3, 250);
  delay(50);
  analogWrite(D3, 128);
  delay(50);
  
}

You can see the 3 different periods in the captured logic analyzer output:

Code inspection doesn't show anything, either. setWaveform just sets the value that will be applied on the next cycle, and that's straight-line code.

[gislf]

I just upgrade to 2.4.2 from 2.3.0 and and meet with similar issue.
The hardware is ESP8285 01M module.
The program do analog write to all avaliable IO pins in a loop with various PWM values.
The problem only occur on GPIO9 and GPIO10, only 0 or 100% duty cycle(in my case 0 or 8191) can be write. The rest value in between does not work.
Switch back to 2.3.0 fixed the issue for now.

[earlephilhower]

@gislf That is actually a different issue than what @EricMc1289 was saying.

GPIO6-11 are used for the flash interface normally. Looks like you have some board that only uses a 2x wide irom interface, not a 4x. I didn't think any existed in the wild, and therefore GPIO9 and 10 are not allowed to be PWM'd (to save memory).

Could you open a new issue with this so we can track it? The fix is trivial, but I'd like to track it...

[earlephilhower]

@gislf Please try #5055 which adds back GPIO9/10 to the allowed waveform generators.

[gislf]

@earlephilhower Thank you for the quick response! Yes ESP8285 has build in flash, so additional GPIO pins are available. May be it’s a good idea to enable these pin for PWM on ESP8285. The board I use is ESP -01M which has 11 GPIO. https://www.elecrow.com/download/esp_01m_datasheet_en.pdf

[EricMc1289]

To put this in an MCVE is quite hard, because it is a bigger framework and normaly the mode changes by http request. But hopefully this will show the issue.

So this is the sample file:

#include "Colors.h"

RGB HSVToRGB(HSV hsv);
void setColorToOutput(uint8_t r, uint8_t g, uint8_t b);


void setup() {
      analogWriteFreq(1000);
      analogWriteRange(255);
}

void loop() {
  runMode(4); //LED's are red
  delay(2500);
   runMode(5); //LED's stays red instead of yellow
  delay(2500);
  runMode(6); //LED's turn green
  delay(2500);
   runMode(5); //LED's turn yellow ??
  delay(2500);
}


void setColorToOutput(uint8_t r, uint8_t g, uint8_t b) {
      analogWrite(14, r);
      analogWrite(12, g);
      analogWrite(13, b);
}

void runMode(uint8_t mode) {
	
	switch (mode) {

		//Staedy modes-------------------------------------------
	case 0:
		setColorToOutput(0, 0, 0);
		break;

	case 1:
		setColorToOutput(0xff, 0x90, 0x25);
		break;

	case 2:
		setColorToOutput(0xFF, 0xc5, 0x60);
		break;

	case 3:
		setColorToOutput(0xFF, 0xFF, 0xFF);
		break;

	case 4: //red
		setColorToOutput(HSVToRGB(HSV(0, 1., 1.)).get());
		break;

	case 5://orange
		setColorToOutput(HSVToRGB(HSV(20, 1., 1.)).get());
		break;

	case 6://yellow
		setColorToOutput(HSVToRGB(HSV(50, 1., 1.)).get());
		break;
	case 7://green-yellow
		setColorToOutput(HSVToRGB(HSV(80, 1., 1.)).get());
		break;
	case 8://green
		setColorToOutput(HSVToRGB(HSV(120, 1., 1.)).get());
		break;
	case 9:
		setColorToOutput(HSVToRGB(HSV(135, 1., 1.)).get());
		break;
	case 10:
		setColorToOutput(HSVToRGB(HSV(160, 1., 1.)).get());
		break;
	case 11:
		setColorToOutput(HSVToRGB(HSV(195, 1., 1.)).get());
		break;
	case 12:
		setColorToOutput(HSVToRGB(HSV(240, 1., 1.)).get());
		break;
	case 13:
		setColorToOutput(HSVToRGB(HSV(255, 1., 1.)).get());
		break;
	case 14:
		setColorToOutput(HSVToRGB(HSV(290, 1., 1.)).get());
		break;
	case 15:
		setColorToOutput(HSVToRGB(HSV(340, 1., 1.)).get());
	
	default:
		mode++;
		break;

	}
}

RGB HSVToRGB(HSV hsv) {
	double r = 0, g = 0, b = 0;

	if (hsv.S == 0)
	{
		r = hsv.V;
		g = hsv.V;
		b = hsv.V;
	}
	else
	{
		int i;
		double f, p, q, t;

		if (hsv.H == 360)
			hsv.H = 0;
		else
			hsv.H = hsv.H / 60;

		i = (int)trunc(hsv.H);
		f = hsv.H - i;

		p = hsv.V * (1.0 - hsv.S);
		q = hsv.V * (1.0 - (hsv.S * f));
		t = hsv.V * (1.0 - (hsv.S * (1.0 - f)));

		switch (i)
		{
		case 0:
			r = hsv.V;
			g = t;
			b = p;
			break;

		case 1:
			r = q;
			g = hsv.V;
			b = p;
			break;

		case 2:
			r = p;
			g = hsv.V;
			b = t;
			break;

		case 3:
			r = p;
			g = q;
			b = hsv.V;
			break;

		case 4:
			r = t;
			g = p;
			b = hsv.V;
			break;

		default:
			r = hsv.V;
			g = p;
			b = q;
			break;
		}

	}

	return RGB((unsigned char)(r * 255), (unsigned char)(g * 255), (unsigned char)(b * 255));
}

and the colors.h:

#pragma once
#include <cmath>
#include <Arduino.h>

using namespace std;

class RGB
{
public:
	unsigned char R;
	unsigned char G;
	unsigned char B;

	RGB(unsigned char r, unsigned char g, unsigned char b);
	uint32_t get();
	boolean Equals(RGB rgb);
};

class HSV
{
public:
	double H;
	double S;
	double V;

	HSV(double h, double s, double v);
	boolean Equals(HSV hsv);
};

Normally i would trigger the mode change with an http request by the httpserver. In 2.4.1 it works as seen above. With Version 2.4.2. it only works if I change the setColorToOutput function as described at the first post.

[earlephilhower]

@EricMc1289 The code snippet is still missing bits and can't compile. Just make a new sketch that compiles and runs for you, and shows the error, and then I can look at it.

Right now it looks like it ends up looking exactly like my simple test case which passes, with the exception that you set 3 pins, and not 1. I've rerun the code w/3 pins and it still runs fine, so w/o a working MCVE there's nothing I can do:

void setup() {
  // put your setup code here, to run once:
      analogWriteFreq(1000);
      analogWriteRange(255);
}

void loop() {
  // put your main code here, to run repeatedly:
  analogWrite(D3, 10);
  analogWrite(D2, 100);
  analogWrite(D1, 30);
  delay(50);
  analogWrite(D3, 250);
  analogWrite(D2, 200);
  analogWrite(D1, 100);
  delay(50);
  analogWrite(D3, 128);
  analogWrite(D2, 10);
  analogWrite(D1, 240);
  delay(50);
  
}

[earlephilhower]

Another idea: Add Serial.printf()s to your full app and capture the values you're writing to analogWrite. That way your full app will be running and you can try those specific sequence of values in the short sketch I posted above and see if it fails (changing the pins, of course!). If so, publish those #s and we can take a look at it.

[earlephilhower]

Also tried the edge cases, still looks fine on my tests:

void setup() {
  Serial.begin(115200);
  // put your setup code here, to run once:
  analogWriteFreq(1000);
  analogWriteRange(255);
  analogWrite(D3, 255);
  delay(1000);
  analogWrite(D3, 254);
  delay(1000);
  analogWrite(D3, 128);
  delay(1000);
  analogWrite(D3, 1);
  delay(1000);
  analogWrite(D3, 200);
  delay(1000);
  analogWrite(D3, 0);
  delay(1000);
}

void loop() {
  // put your main code here, to run repeatedly:
}

[earlephilhower]

It's been 3 weeks without any response from the original author, and nobody else seems to have had this problem (or at least they haven't reported it here), so I'm going to close this as a can't reproduce. The other one, the 8285 extra pins, is being tracked in a separate bug.