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

Demo of PWM 100% problem on ESP32 (for discussion) #11786

Merged
merged 3 commits into from
Apr 20, 2021

Conversation

barbudor
Copy link
Contributor

Description:

This is to demo and track a problem with ESP32's PWM not able to reach 100% duty cycle
The most annoying problem is not able to reach 0% on PWM_i inverted outputs which prevent powering off the output.

How to see the problem :

  • connect a LED to a GPIO with a 150..220 ohms resistor toward 3V3
  • set the GPIO as PWM_i
  • Try to power off the led
  • Without this patch, the led will remain slighly on

Looking with a scope the GPIO signal shows that while we would expect the signal to be a continuous HI to reach 0% duty cycle for a PWM_i, a 1us pulse to LOW remains every cycle (1/1023th) but that is enough to slighly lite the LED.

The sample patch I provide force the pwm value to 1024 if the calculated value is 1023 which is enough to do the trick.

While this seems to fix the problem for ligthts (using color or channel), the same problem can be seen everywhere analogWrite() is used : PWM command, LedPWM, ....

I'm not sure to understand all the intrinsic around PWMrange and what impact modifying the whole PWMrange logic would have on Tuya dimmers for example

I can't tell this is a bug of the ESP32 HAL has I haven't found a definite description of what the ESP32 LedControl module should do. But this is clearly for now a difference with the ESP8266 that do not show this behavior.

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works on Tasmota core ESP8266 V.2.7.4.9
  • The code change is tested and works with core ESP32 V.1.0.6
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

@s-hadinger
Copy link
Collaborator

@arendst please put on hold. We are discussing this issue on Discord and I would favor a skipping 1 at mid range (512) rather than at end of range

@barbudor barbudor changed the title PWM 100% problem on ESP32 Demo of PWM 100% problem on ESP32 (for discussion) Apr 18, 2021
@Dlloydev
Copy link

I've recently added an AnalogWrite library that will resolve the missing full-on PWM issue. For any bit width, the missing step is included. For example, 8-bit resolution has 257 steps (zero-based) 0 is fully off, 1-255 is the PWM duty cycle range and 256 is fully on (high).

arduino-library-badge
GitHub link ... https://github.com/Dlloydev/ESP32-ESP32S2-AnalogWrite

@barbudor
Copy link
Contributor Author

@Dlloydev This is already how the ESP32 analogWrite(), based on lecdWrite(), is behaving whereas the ESP8266 has 100% duty cycle at 255.
I feel our position is that 0..100% should be matched with 0..255 (or 0..1023) (like the ESP8266) and not 0..256.

@Dlloydev
Copy link

Actually, I agree that 255 should be 100% full on. I'm new with the ESP32, but it seems that the hardware will support this as I briefly looked at their technical reference manual.

@arendst arendst added the on hold by dev team Result - Feature request put on hold by member of development team label Apr 19, 2021
@hallard
Copy link
Contributor

hallard commented Apr 19, 2021

@barbudor discovered this issue on new Denky D4 board which as a Common Cathode RGB Led (so that is why it's PWM inverted). The fact is with common Anode RGB led, setting PWM to 255 (or 1023) even if it's not really full illuminated it could be 99.6% or 99.9% so I can imagine no human eye can see the difference (so the issue)
I already had this bug on ESP8266 long time ago where I needed an analogWrite(256) to light off the led and looks like it has been fixed since but also explained on #5957

The deal is that analogWrite is not native to ESP32 and has been added in compatibility module in tasmota so opening this issue to ESP32 team won't be supported. The new api is ledcWrite ( atfer setup of ledcAttachPin() and ledcSetup()) and even the official demo PWM sample of ESP32 reference this issue, see comments on sample code so my guess we need to live with that one

As @barbudor state the easier fix is that if we are in PWM inverted mode and PWM value is the latest of the range, add one or set it to 1024 on ESP32. But I'll try to open a issue on ESP32 core team, worth trying to fix at the source.

@barbudor
Copy link
Contributor Author

As you pointed, tasmota is not using Arduino for analogWrite on ESP32 but a compatibility layer.
I've already adapted the code but was unable to publish yesterday due to ISP problems.
I'll push it today after a last check with the scope.

@barbudor
Copy link
Contributor Author

TL;DR;

  • Arduino doc say between 0 (always off) and 255 (always on)
  • ESP8266 : 0..100% is 0..2^N-1 (compliant)
  • ESP32 : 0..100% is 0..2^N (not compliant)

Best place to adapt is probably in libesp32/ESP32-to-ESP8266-compat/src/esp8266toEsp32.h
Draft PR updated following @s-hadinger for performing the fix at mid-range
Will do a final check tonight with a scope to be 100% sure

Details

ESP8266

The esp8266/Arduino code is referring to the Arduino official doc for analogWrite():

value: the duty cycle: between 0 (always off) and 255 (always on)

The code defines the high and low duration of the PWM signal:

  uint32_t high = (analogPeriod * val) / analogScale;
  uint32_t low = analogPeriod - high;

Where analogScale is the value provided with analogWriteRange() which makes:

  • val = 0 => high = 0 and low = analogPeriod => signal is low 100%
  • val = analogScale => high = analogPeriod and low = 0 => signal is high 100%

ESP32

On the ESP32 the hardware PWM is based on a

counter which will count up to the value specified in LEDC_HSTIMERx_DUTY_RES. An overflow interrupt will be generated once the counting value reaches 2^LEDC_HSTIMERx_DUTY_RES − 1, at which point the counter restarts counting from 0
A channel takes the 20-bit value from the counter of the selected high-speed timer and compares it to a set of two values in order to set the channel output. The first value it is compared to is the content of LEDC_HPOINT_HSCHn;
if these two match, the output will be latched high. The second value is the sum of LEDC_HPOINT_HSCHn and LEDC_DUTY_HSCHn[24..4]. When this value is reached, the output is latched low

(ESP32 Technical Reference Manual section 14.2.2 and 14.2.3)

The ESP32 immplementation is in https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-ledc.c

Initialisation

  • LEDC_HSTIMERx_DUTY_RES is initialized with the analogWiteRange()
  • LEDC_HPOINT_HSCHn is initialized at 0
  • define the idle_level to LOW (pin value when PWM is stopped)

analogWrite() => ledcWrite()

  • if duty == 0 : the PWM is stopped and pin is set LOW (idle_level)
  • if duty != 0 : duty is written to LEDC_DUTY_HSCHn
    So with duty != 0, the signal is HIGH from 0 to duty-1 and low from duty to DUTY_RES (range, typically 1023)
    When DUTY_RES=1023 and duty=1023, the signal is HIGH from 0 to 1022 and LOW for 1023. Thus the ratio is 1023/1024.
    When DUTY_RES=1023 and duty=1024, the signal is always HIGH because the counter never reach 1024.

@s-hadinger
Copy link
Collaborator

Thanks @barbudor
Your last fix looks good to me, simple and slick. All that I love.

@arendst I suggest you merge.

@arendst arendst merged commit 33dcbb4 into arendst:development Apr 20, 2021
@barbudor barbudor deleted the esp32_pwm_full branch May 26, 2021 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold by dev team Result - Feature request put on hold by member of development team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants