Add PWMOutputDevice.pulse method

[lurch]

...which simply calls blink with different default arguments

[lurch]

This may seem a bit trivial, but it means you can just do:

from gpiozero import PWMLED
from signal import pause

led = PWMLED(21)
led.pulse()
pause()

Argument-order is exactly the same as the existing PWMOutputDevice.blink method.

[bennuttall]

Looks good! Could it cope without on_time and off_time though? Mind you, I can imagine cases you might want it to stay on or stay off between pulses. Yeah, that makes sense.

@waveform80 anything you'd change about this?

[waveform80]

My only concern about this is API bloat (it adds a method to a base class, and there's already a method which accomplishes its goals, albeit without default parameters). I'll defer to @bennuttall's judgement on where to draw the line on such things though.

It's also worth noting we already limit which methods appear in the concrete API docs (see the method lists in https://raw.githubusercontent.com/RPi-Distro/python-gpiozero/master/docs/api_output.rst) and hence this method wouldn't automatically appear unless added to the PWMLED list (it will automatically appear in the generic classes docs though as they don't limit which methods appear on the assumption that they're "advanced").

[bennuttall]

Ok I'll think about it.

[lurch]

Hmmm, maybe it would be worth adding pulse() just to the PWMLED class then, rather than the base PWMOutputDevice?
My initial thinking was that pulse() was kind-of-like an analogue-equivalent of the digital blink() method.

EDIT: And I guess if pulse was added, then https://gist.github.com/bennuttall/aae0ec488216033774ac#file-onboard_leds_pwm-py could be re-written as:

from gpiozero import PWMLED
from signal import pause

red = PWMLED(35)
green = PWMLED(47)

def opp():
    while True:
        yield 1 - red.value

green.source = opp()
red.pulse()
pause()

[waveform80]

To be honest, that could already be written with PWMLED's blink using red.blink(0, 0, 1, 1) out something similar - no need for that while loop :-)

[lurch]

Hmmm, red.pulse() or red.blink(0, 0, 1, 1)... I certainly know which I'd find easier! ;-)

[bennuttall]

I think if we were to add a pulse method, it should set on_time and off_time to 0 and only allow configuration of fade_in_time and fade_out_time, otherwise it's exactly the same as blink, just with default params, the first two of which you'd have to provide anyway, e.g:

led.pulse(0, 0, 2, 2)

or you'd have to use named parameters:

led.pulse(fade_in_time=2, fade_out_time=2)

Whereas with in_time and out_time not configurable, it would just be:

led.pulse(2, 2)

If you need to pulse but leave on or off for any amount of time, just use blink.

I was unsure, but I'm warming to the idea now.

[lurch]

Fair enough. I guess the other option would be to re-order the arguments of pulse to pulse(fade_in_time, fade_out_time, on_time, off_time, n, background) but that's probably just too confusing, so I'm happy to remove the on_time and off_time parameters, and delegate any more advanced usage to the blink method.

What's the consensus about leaving the method in the base PWMOutputDevice class, or moving it to just the PWMLED class?

[bennuttall]

Yeah definitely confusing.

I'm up for merging with my suggestion thrown in. @waveform80, ok?

May as well leave on PWMOutputDevice.

[waveform80]

All sounds good to me (i.e. leave it on PWMOutputDevice, remove on_time, off_time)

[bennuttall]

Looks like TrafficLights doesn't have pulse. I'll look into it.

[bennuttall]

Just for fun*, try this with a buzzer:

>>> buzzer = PWMOutputDevice(pin)
>>> buzzer.pulse()

* for values of fun = my ears hurt

[lurch]

Looks like TrafficLights doesn't have pulse. I'll look into it.

Yeah, you'll need to add pulse to LEDBoard too. I was going to add that to the PR at the same time as removing on_time and off_time, but you beat me to it ;-)

[bennuttall]

I like pulse. Makes blink look boring.