esp-rs / esp32-hal

A hardware abstraction layer for the esp32 written in Rust.
Apache License 2.0
192 stars 28 forks source link

Ledc support #68

Closed sepotvin closed 2 years ago

sepotvin commented 2 years ago

This pull request add the support for the LEDC (LED PWM Controller). Only fixed frequency is supported at this time. No support for hardware fade or interrupts yet. High Speed and Low Speed channels are available.

MabezDev commented 2 years ago

Hi @sepotvin, thanks for the contribution! This is looking good, just a couple of things.

How to update the duty cycle once the channel has been configured? My example use case:

   let mut ledc = LEDC::new(clock_control_config);
    ledc.set_global_slow_clock(LSGlobalClkSource::ABPClk);
    let mut lstimer0 = ledc.get_timer::<LowSpeed>(timer::Number::Timer0);
    lstimer0
        .configure(timer::config::Config {
            duty: timer::config::Duty::Duty8Bit,
            clock_source: timer::LSClockSource::SlowClk,
            frequency: 24_000_000.Hz(),
        })
        .unwrap();

    let mut channel0 = ledc.get_channel(channel::Number::Channel0);

    let mut duty = 0.1;
    let mut delta = 0.05;
    loop {
        channel0
            .configure(channel::config::Config {
                timer: &lstimer0,
                duty,
                // output_pin: pins.gpio4,
                output_pin: unsafe {
                    Peripherals::steal().GPIO.split().gpio4
                },
            })
            .ok();
        duty += delta;
        if duty >= 1.0 {
            delta = -0.05;
        } else if duty <= 0.1 {
            delta = 0.05;
        }
        sleep(100.ms());
    }
}

Which creates a nice breathing fade, however as you can see I had to resort to unsafe to keep passing in the pin. It should be possible to update the duty without passing in the pin again.

I understand if you don't want to implement this right now, but it would be helpful to see how it would be possible in the future :)

sepotvin commented 2 years ago

I implemented a set_duty() member function on the Channel struct which should enable you to change the duty on the fly. I had to refactor things a little in the Channel interface (pin is provided in new() now) due to the compiler not being able to infer the pin type of the ChannelIFace Trait when trying to call set_duty(). I'm not sure I'm completely happy with the new interface but at least it should work.

I got tired of manually updating the 16 copies of the hw channel programming so I added a dependency on the paste crate to be able to create macros to do it instead. Let me know if it is not ok and I will revert to manually doing cut-n-paste for each channel.

I also removed a few unwrap that should not fail in the normal case but could fail if the hw interface is called directly instead of going through the official iface. It is not the way it should be used but I cannot make the HW Interface non public as it leaks via the ChannelFace.

Let me know if you have any more comments.

MabezDev commented 2 years ago

Awesome! I'm sure there are things to improve upon, but this a great addition as is!

Thanks for the contribution @sepotvin, much appreciated!

Bonus gif: https://gfycat.com/rightblushingiberianlynx