search

micropython / micropython-esp32

Old port of MicroPython to the ESP32 -- new port is at https://github.com/micropython/micropython
MIT License
678 stars 218 forks source link

Add ESP32 PWM support, mirroring the API of the ESP8266. #79

Closed vandys closed 7 years ago

vandys commented 7 years ago

Hi, first time contribution to micropython, so just let me know if anything should be changed and bounce this request. Thanks!

nickzoic commented 7 years ago

That's brilliant, thanks for your contribution.

I skimmed through it and it looks pretty good, I'll have a closer look most likely tomorrow. -----Nick

dpgeorge commented 7 years ago

@vandys thanks! For licensing/copyright purposes, can you please outline which parts of the code you wrote, which parts are copied (and from where), what information you used to work out the ESP32 API and what other code you looked at during the writing. For such a large piece of new code we need to be sure it can retain the MIT license.

Just a note on code style: maximum line length in this project is roughly 90-100 characters, so no need to break lines under that.

vandys commented 7 years ago

I started by copying the esp8266 machine_pwm.c. I used information from the ESP32 Technical Reference Manual, the esp-idf documentation, and the SDK's sample ledc example code (but I did not copy that code, just studied it to understand the SDK's API for PWM). So aside from the code copied from the esp8266 PWM support, everything else you see there is just new code I wrote.

I wasn't an employee of anybody when I wrote it, and I wrote it with the understanding and intention that it's simply a derivative work of the existing micropython code. I freely and willingly contribute it to the project and intend that it not change the legal status of the micropython code base in any way, even if it is included in that base in whole or part.

laurivosandi commented 7 years ago

Hi, I tried to enable PWM for pin 13 - everything works great :)

Then for pin 34, which according to pinout should support PWM, it says:

E (169529) ledc: ledc_channel_config(258): LEDC GPIO output number error

As I understandand you used LEDC PWM backend for your code and for other PWM pins there is different implementation, which pins actually use LEDC?

Also it seems to think that enabling PWM for pin 15 is perfectly okay, pin 15 is connected to ground :D

vandys commented 7 years ago

The PWM code here is just using the SDK's API, and if there's some other PWM implementation then I haven't caught a whiff of it. In driver/gpio.h they list GPIO pins 0 through 33 as input and output. Pin 15 on my own board does not seem to be connected to ground, FWIW.

laurivosandi commented 7 years ago

Hi, where have here something similar to this https://raw.githubusercontent.com/gojimmypi/ESP32/master/images/myESP32%20DevKitC%20pinout.png

According to this PWM should work on 34

vandys commented 7 years ago

Well, you need to talk with the SDK folks at Espressif. For instance, from their headers:

What in the image you sent makes you believe it'll do output on that pin? You will want to get the ESP32 Technical Reference Manual. Section 4.3 applies here.

MrSurly commented 7 years ago

I was thinking of adding a wrapper around the MCPWM; PWM would be a sub-object of this module.

Mostly, I want to do accurate pulse timing. Yes, I'm aware of machine.time_pulse_us().

dpgeorge commented 7 years ago

@laurivosandi that pin out looks wrong.

dpgeorge commented 7 years ago

I was thinking of adding a wrapper around the MCPWM; PWM would be a sub-object of this module.

It looks like MCPWM and LEDC are 2 different ways to do PWM on a pin. It doesn't really matter what the underlying implementation is, as long as the machine.PWM class works as expected (it's not yet properly specified in the docs so it's subject to change).

If MCPWM provides lots of extra functionality that doesn't fit a standard peripheral (is it similar enough to a Timer?) then it could go in a new "esp32" module as MCPWM class.

vandys commented 7 years ago

that pin out looks wrong.

I'm sorry, which pin out? The one from the earlier discussion? Pin 34?

dpgeorge commented 7 years ago

I'm sorry, which pin out? The one from the earlier discussion? Pin 34?

Sorry, yes, the link that @laurivosandi posted above to "myESP32 DevKitC pinout.png" looks wrong in that it says all IO pins should be able to do PWM when (as you pointed out) higher-numbered pins are input only.

vandys commented 7 years ago

Ok, thanks. I'm closing this pull request too, will change code and re-test against new SDK.

MrSurly commented 7 years ago

If MCPWM provides lots of extra functionality that doesn't fit a standard peripheral (is it similar enough to a Timer?) then it could go in a new "esp32" module as MCPWM class.

It's more focused around motor control -- you could drive BLDC motors, for example. Or generate servo signals.

But, it has a separate "capture" module that's mostly for edge capture / pulse timing. Not purely PWM.

It looks like LEDC is more about using timers, and animating led brightness?

dpgeorge commented 7 years ago

The follow-up to this was merged in #96