[stm] implementation of audiopwmio #4399
Conversation
9c55311 to
fc9b42a
Compare
There was a problem hiding this comment.
Thanks for this PR! I'm unable to get this working on my Meowbit with the WAV sample code in the Circuitpython documentation. The speaker makes a little strangled sound, and then the program hangs irrecoverably, requiring a manual erase of the entire flash. If you have a test sketch of your own, I'm happy to try that.
The style and timer allocation stuff looks generally ok, I can't point out where the hang might be at first glance. There are also some minor things to fix unrelated to performance.
| @@ -194,3 +194,12 @@ void common_hal_mcu_pin_claim(const mcu_pin_obj_t* pin) { | |||
| void common_hal_mcu_pin_reset_number(uint8_t pin_no) { | |||
| reset_pin_number(pin_no / 16, pin_no % 16); | |||
| } | |||
|
|
|||
| void set_drive_mode(const mcu_pin_obj_t *pin, uint32_t mode) { | |||
There was a problem hiding this comment.
This should be a static function within PWMAudioOut.c, it doesn't need to be in Pin.c. The function in DigitalIO is very old and shouldn't be adjusting the pull and speed as it does, so it needs to be tweaked anyway.
| GPIO_InitTypeDef GPIO_InitStruct = {0}; | ||
| GPIO_InitStruct.Pin = pin_mask(self->pin->number); | ||
| GPIO_InitStruct.Mode = (drive_mode == DRIVE_MODE_OPEN_DRAIN ? | ||
| set_drive_mode(self->pin, drive_mode == DRIVE_MODE_OPEN_DRAIN ? |
There was a problem hiding this comment.
Please revert this as per the previous comment. This module needs some reworks, and Pin.c isn't a good place to abstract this kind of thing in the meantime.
| * | ||
| * The MIT License (MIT) | ||
| * | ||
| * Copyright (c) 2021 Artyom Skrobov for Adafruit Industries |
There was a problem hiding this comment.
We use "for Adafruit Industries" to designate work that was sponsored by Adafruit, so you don't have to include it in your byline.
|
@hierophect thank you for the review!
My guess is that it's #4397 :-)
|
|
@tyomitch ah, thanks for the full context, that probably is it. I'll make a note of my problem for Scott in the issue. |
Based on nrf PWMAudioOut by @jepler and stm PulseOut by @hierophect Tested on a Meowbit
|
@tyomitch thanks for the updates. Just a gentle note that here on the Circuitpython github, our general policy is to update PRs without force-pushing, so we can see what you've edited and the comments are not lost. It makes it easier to tell when re-testing with hardware is necessary. Is this ready for another review? |
Yes please :) I will try to avoid force-pushing PRs, but for the record, overwritten commits are not lost, and you can see the changes since the first review at https://github.com/adafruit/circuitpython/compare/fc9b42a..b40d072 -- inevitably very noisy because of an intervening global reformatting (a52eb88) |
Based on nrf PWMAudioOut by @jepler and stm PulseOut by @hierophect
Tested on a Meowbit