PWMBuzzer and Speaker refactor

[martinohanlon]

As an experiment I have refactored PWMBuzzer and Speaker.

My aims were:

• to simplify PWMBuzzer
• have PWMBuzzer still act like a simple Buzzer (e.g. you can turn it on and off using a button, but its frequency and volume can be set)
• refactor Speaker so that rather than being a factory, it is a more complex OutputDevice similar to RBGLED.

This code is in a seperate branch - https://github.com/RaspberryPiFoundation/picozero/tree/speaker

You can see the PR here - #28

This looks/feels like a simpler implementation, doesnt confuse PWMBuzzer and appears to support the same function set.

@tracygardner

What do you think? If you are ok with it, I will pull it into dev.

There is some missing functionality from the original here - https://github.com/RaspberryPiFoundation/picozero/pull/28/files#diff-7d562e79b5dec9a5ca24810cfd4d70ff492c0dad33a569e1652693ee7ab79aabR651 - I wasnt sure what it was meant to do.

[tracygardner]

I don't understand what you've done with the functionality.

Each note needs a duration (or default) and you need a small gap between notes to distinguish them. See: https://github.com/RaspberryPiFoundation/picozero/blob/dev/docs/examples/speaker_tune.py

Plus Marc's request: #21

[martinohanlon]

I don't understand what you've done with the functionality.

re this code - https://github.com/RaspberryPiFoundation/picozero/pull/28/files#diff-7d562e79b5dec9a5ca24810cfd4d70ff492c0dad33a569e1652693ee7ab79aabR651

I didnt understand what it was doing so made a note to come back to it later - I could do with some help understanding it / implementing it.

I think the parameter tune can be:

1. a single "note", represented as:

• a frequency - 440
• a midi note - 60
• a string note -"e4"

2. a list of single notes - [440, 60, "e4"]
3. a list of 2 value tuples of (note, duration) - [ (440,1), (60, 2), ("e4", 3) ]

When passed a list of notes the speaker should be turned off for 10% of the duration between each note (a delay).

Is this right?

A few questions/thoughts running through my brain:

• what should duration be when passed a list - the duration of the whole sequence or just 1 note?
• how is duration used when the duration of each note has been passed? Does it become the default duration if one has been passed?
• is there any occurrence where you would want to use a frequency that is in the midi note range?
• there isnt a way a changing the delay between each note when playing a list. Should there be?
• would it be easy to understand / implement if there were 2 methods play (for single notes) and play_tune (for lists)?

[tracygardner]

Ah cool, it sounded like you thought the functionality was ready, but it would break our current examples and projects.

I added support for 2) at Marc's request but I'm not sure how important it is to musical people who will likely want to specific timing. Kids messing around might use it though.

• Duration for a list is currently the default duration if no duration is specified.
• Durations that are omitted after a duration is specified will use that duration (as supported by other music APIs)
• The approach taken matches other implementations that use Midi and Frequency. We could have an override parameter to force one or the other but currently we suggest using frequencies down to 150 (midi is up to 128).
• There is a way of changing the delay in the list in a previous version. You can use this to create a staccato effect or other articulation. I removed it because I thought you'd say the API was getting too complicated! I implemented it as a multiplier parameter so that notes always last the same length (but actually play for different proportions of that time) - based on input from real musical people.
• I did have two separate methods, but users seemed to want one so a made the single note version internal. I could go either way on that. I'd actually consider different methods for the wait True/False as in Scratch with start/play so that you have a different mental model for how they work. (My team seemed to prefer the parameter though with only one method name to remember.)

Something to be aware of is that a common pattern in to have notes with a very short duration when creating sound effects.
See: https://github.com/RaspberryPiFoundation/picozero/blob/dev/docs/examples/speaker.py

I didn't use a generator for single notes as I wasn't sure whether this would be an issue.

[martinohanlon]

@tracygardner I have update the tune_generator to cover these scenarios. https://github.com/RaspberryPiFoundation/picozero/pull/28/files#diff-7d562e79b5dec9a5ca24810cfd4d70ff492c0dad33a569e1652693ee7ab79aabR646

It seems to work i.e. I get values coming out!

Could you have a look through. If your happy ill merge into dev

[tracygardner]

@martinohanlon

I tried the speaker example: https://github.com/RaspberryPiFoundation/picozero/blob/dev/docs/examples/speaker.py

And it doesn't work. The tada function sort of works but something weird is going on compared with the dev branch. Maybe the volume is low for the first note?

The chirp function doesn't work at all. Due to the timing?

beep also doesn't work but you may not have pulled that from the dev branch before you started as that was a recent addition.

I will investigate.

[tracygardner]

Just had a look and the issue is the parameter order:

def play(self, tune=440, volume=1, duration=1, n=1, wait=True):

Do you have any problem with flipping volume and duration? (Duration is much more used, especially with tone buzzers that are often used at max volume most of the time.)

There's also an issue with the volume instead of the duration being used for a single note.

tune = [(tune, volume)]

[martinohanlon]

Thanks for testing and fixing the issues.

Do you have any problem with flipping volume and duration? (Duration is much more used, especially with tone buzzers that are often used at max volume most of the time.)

No, not at all. I thought i copied it off the old play, obvs not.

[martinohanlon]

Pushed to master