esphome / issues

Issue Tracker for ESPHome
https://esphome.io/
290 stars 36 forks source link

LEDC automatic channel selection bug. Output is then at wrong frequency and does not map correctly to output (ESP32) #3114

Open kolins-cz opened 2 years ago

kolins-cz commented 2 years ago

The problem

First channel (Light) works at 25kHz instead of 2441Hz set in config. Mapping is also very wrong: 0-37% in HA - duty cycle nicely increases from 0% to 99% 38-47% in HA - duty cycle begins over 0%-99% 48-100% in HA - full ON (100% PWM)

Second channel (Fan) works as expected. 25Khz as set and nice linear mapping 0-100% Third channel (Servo) seems to be working at 50Hz and 1-2ms pulse lenght

Which version of ESPHome has the issue?

2022.2.6

What type of installation are you using?

Home Assistant Add-on

Which version of Home Assistant has the issue?

2022.3.1

What platform are you using?

ESP32

Board

ESP32 devkit original

Component causing the issue

ledc

Example YAML snippet

output:
  - platform: ledc
    pin: GPIO19
    frequency: 2441Hz
    id: pwmlight1

  - platform: ledc
    frequency: 25kHz
    pin: GPIO16
    id: pwmfan1

  - platform: ledc
    frequency: 50Hz
    pin: GPIO0
    id: pwmservo1

### probably of no concern, just for clarity
light:
  - platform: monochromatic
    output: pwmlight1
    name: "Mini Growbox Light"

fan:
  - platform: speed
    output: pwmfan1
    name: "Mini Growbox Fan"

servo:
  - id: my_servo
    output: pwmservo1
    auto_detach_time: 1s
    transition_length: 1s

Anything in the logs that might be useful for us?

No response

Additional information

Video of oscilloscope and HA demonstrating the problem of incorrect mapping. Note the frequency is 25kHz:

https://www.youtube.com/watch?v=DYrGPcBBq4o

Solution to problem is to specify ledc channels manually. It solves the issues. I was told by ssieb on discord that this should work automatically.

Code that solves this problem:

output:
  - platform: ledc
    pin: GPIO19
    frequency: 2441Hz
    id: pwmlight1
    channel: 0

  - platform: ledc
    frequency: 25kHz
    pin: GPIO16
    id: pwmfan1
    channel: 2

  - platform: ledc
    frequency: 50Hz
    pin: GPIO0
    id: pwmservo1
    channel: 4
kolins-cz commented 2 years ago

Found some dismissed issues that explains similar behaviour. Users before me did not have oscilloscope or knowledge to confirm this and were dismissed as their fault: https://github.com/esphome/issues/issues/2369 https://github.com/esphome/issues/issues/48

kolins-cz commented 2 years ago

Issue was heavily re-written due to misleading nature of this bug. Without realizing that Light was at wrong frequency and Fan component worked correctly, wrong mapping led me to Monochromatic light component which was verified to work correctly. Problem was in ledc for the whole time.

probot-esphome[bot] commented 2 years ago

Hey there @ottowinter, mind taking a look at this issue as it has been labeled with an integration (ledc) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

OttoWinter commented 2 years ago

Correct the issue is with the ledc channels. Two adjacent channels share the same ledc timer and so share the same frequency. It would probably be good to change how the ledc channels are auto-assigned to go like [0, 2, 4, ..., 14, 1, 3, ..., 15] so it only is an issue if you use more than 8 channels and highlight more in the docs that this is the case

ssieb commented 2 years ago

I haven't checked the esphome code yet, but the ESP itself has no requirement for adjacent channels to share a timer. Any channel can use any timer, but there are only 4 timers available. There are 8 fast channels and 8 slow channels, each with a set of 4 timers. Given the way the esphome config system works, it could be a bit tricky, but a smarter allocation system would be possible. e.g. at the simplest, any channels with the same frequency should use the same timer. But that breaks down if the frequency is changed at runtime for something like rtttl.

Joshfindit commented 2 years ago

Came here to file a feature request as an end user, and found this issue which it turns out is exactly what I came to shed light on.

First a couple notes:

Logic analysis

What I have to contribute is logic analysis graphs of the two different setups. These are with the same setup which is as follows:

ESPHome code

output:
  - platform: ledc
    pin: 16
    frequency: 19531Hz
    # channel: 0
    id: gpio_16
  - platform: ledc
    pin: 17
    frequency: 50000Hz
    # channel: 2
    id: gpio_17
  - platform: ledc
    pin: 21
    # channel: 4
    frequency: 100000Hz
    id: gpio_21
  - platform: ledc
    pin: 22
    # channel: 6
    frequency: 300000Hz
    id: gpio_22

light:
  - platform: monochromatic
    output: gpio_16
    name: "$devicename gpio_16"
  - platform: monochromatic
    output: gpio_17
    name: "$devicename gpio_17"
  - platform: monochromatic
    output: gpio_21
    name: "$devicename gpio_21"
  - platform: monochromatic
    output: gpio_22
    name: "$devicename gpio_22"

HomeAssistant

Here are the logic outputs for the two configurations

With the channels commented out

image

Logging shows that ESPHome chooses sequential channels:

[19:36:09][C][ledc.output:118]: LEDC Output:
[19:36:09][C][ledc.output:119]:   Pin GPIO16
[19:36:09][C][ledc.output:120]:   LEDC Channel: 0
[19:36:09][C][ledc.output:121]:   Frequency: 19531.0 Hz
[19:36:09][C][ledc.output:118]: LEDC Output:
[19:36:09][C][ledc.output:119]:   Pin GPIO17
[19:36:09][C][ledc.output:120]:   LEDC Channel: 1
[19:36:09][C][ledc.output:121]:   Frequency: 50000.0 Hz
[19:36:09][C][ledc.output:118]: LEDC Output:
[19:36:09][C][ledc.output:119]:   Pin GPIO21
[19:36:09][C][ledc.output:120]:   LEDC Channel: 2
[19:36:09][C][ledc.output:121]:   Frequency: 100000.0 Hz
[19:36:09][C][ledc.output:118]: LEDC Output:
[19:36:09][C][ledc.output:119]:   Pin GPIO22
[19:36:09][C][ledc.output:120]:   LEDC Channel: 3
[19:36:09][C][ledc.output:121]:   Frequency: 300000.0 Hz

With the channels uncommented

image (Note the exact same timescale of the graphs)

Logging confirms the channels:

[19:31:14][C][ledc.output:118]: LEDC Output:
[19:31:14][C][ledc.output:119]:   Pin GPIO16
[19:31:14][C][ledc.output:120]:   LEDC Channel: 0
[19:31:14][C][ledc.output:121]:   Frequency: 19531.0 Hz
[19:31:14][C][ledc.output:118]: LEDC Output:
[19:31:14][C][ledc.output:119]:   Pin GPIO17
[19:31:14][C][ledc.output:120]:   LEDC Channel: 2
[19:31:14][C][ledc.output:121]:   Frequency: 50000.0 Hz
[19:31:14][C][ledc.output:118]: LEDC Output:
[19:31:14][C][ledc.output:119]:   Pin GPIO21
[19:31:14][C][ledc.output:120]:   LEDC Channel: 4
[19:31:14][C][ledc.output:121]:   Frequency: 100000.0 Hz
[19:31:14][C][ledc.output:118]: LEDC Output:
[19:31:14][C][ledc.output:119]:   Pin GPIO22
[19:31:14][C][ledc.output:120]:   LEDC Channel: 6
[19:31:14][C][ledc.output:121]:   Frequency: 300000.0 Hz
[19:31:14][C][ledc.output:118]: LEDC Output:

Now that I have a logic analyzer, the inconsistencies in PWM LED brightness make a LOT more sense. Not only do the channels use the same start times for pulses, but the lengths are actually different leading to LEDs that are a different brightness than what ESPHome tells them to be.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Joshfindit commented 1 year ago

unstale: I still feel like this is a worthwhile improvement to esphome and don’t have the skills to submit a PR that accomplishes it

adarazs commented 1 year ago

This also gave me a lot of headache when I tried to control an LED with 100 Hz PWM and a computer fan with 25kHz. If I did not have an oscilloscope handy I would have also been very confused where the problem lies. I created a pull request for the documentation to at least spare some of the time for others: https://github.com/esphome/esphome-docs/pull/3126

thargy commented 1 year ago

This has just wasted 2 weeks of head-scratching and meltdowns. My fan controller and current limiter signals (both PWM) worked fine in prototyping, but when I combined them the fans stopped working correctly and I tore down and rebuilt the boards multiple times until I saw the one small comment about channels referencing this issue.

Please can we get an update that auto-separates channels when the frequencies are mismatched and the channel is unspecified? Or at least, can we get a warning?

Joshfindit commented 1 year ago

To simplify the right-now solution, I propose two changes:

  1. Auto-assign alternating timers as @Ottowinter suggested. This will completely solve the issue for configs with 4 or less ledc devices which I suspect will be over 90% of users
  2. Add a warning when there are more than 4 ledc devices

Stretch goals:

kiler129 commented 8 months ago

I think the documentation needs some improvement here. While the workaround of using every-other channel makes sense, is timer sharing a problem if frequency is the same between channels? While I see here what is the problem and why is this a problem, I'm struggling to grasp how is this causing an issue?

Joshfindit commented 8 months ago

... is timer sharing a problem if frequency is the same between channels?

I don’t have hard data right now, but I suspect that the answer is no (or mostly no with some problematic edge cases) It’s an interesting question though, I wish I had set two to the same frequency.

..., I'm struggling to grasp how is this causing an issue?

If your question is “what negative effect does this have on typical users?” some answers are above:

but when I combined them the fans stopped working correctly a lot of headache when I tried to control an LED with 100 Hz PWM and a computer fan with 25kHz the lengths are actually different leading to LEDs that are a different brightness than what ESPHome tells them to be. Output is then at wrong frequency and does not map correctly to [the output setting]

If your question is “what underlying mechanism makes this happen?” the TL;DR is that the ESPs happily take LEDC channel assignments from ESPHome, and then assign them to sequential channels regardless of whether 2 channels can share timers while still getting the frequency they asked for.

j9brown commented 6 months ago

Another solution would be to represent the ledc timers as separate objects similar to how it is done for I2C busses. That way the sharing of timers becomes explicit in the configuration and is more representative of the hardware capabilities.

If I understand the ESP32 technical reference correctly, any number of LEDC channels can share the same timer.

# New top-level object for describing parameters to ledc timers
ledc_timer:
 - id: slow
   frequency: 100 Hz
 - id: fast
   frequency: 25 kHz

# Outputs reference the timers
output:
  # Binds an ledc channel to the slow timer
  - platform: ledc
     ledc_timer_id: slow
     id: slow_1
     pin: 10
  # Binds another ledc channel to the slow timer
  - platform: ledc
     ledc_timer_id: slow
     id: slow_2
     pin: 11
  # Binds a third ledc channel to the slow timer
  - ledc_timer_id: ledc
     id: slow_3
     pin: 12
  # A channel bound to the fast timer
 - platform: ledc
     ledc_timer_id: fast
     id: fast_1
     pin: 13

The frequency setting action can then be defined on the ledc timer object.

For backwards-compatibility, we could implicitly declare an ledc timer object whenever the configuration explicitly specifies a frequency for an ledc channel and emulate per-channel frequency setting action by updating the timer. (Or we could deprecate the old behavior.)

I-hate-2FA commented 4 months ago

i have same issue, when setting ledc frequency to control a pfm motor and using rtttl buzzer at same time it wont work correctly