Fix for Issue #4266 - second PWMAudioOut interferes with the first one

[DavePutz]

For the RP2040 chips; in common_hal_audiopwmio_pwmaudioout_play() we were calling common_hal_audiopwmio_pwmaudioout_get_playing() and common_hal_audiopwmio_pwmaudioout_stop() early on; which means we were accessing the DMA channels before we had set them up by calling audio_dma_setup_playback(). The impact was to stop the DMA for the first PWMAudioOut when setting up the second. In addition, a break statement while checking for a pacing timer was outside the proper braces. Also (perhaps unrelated), a call to audio_dma_stop() in audio_dma_get_playing() was being made even when the channels were not busy.

Closes: #4266

[jepler]

[I added "closes:" line to the initial comment, so that accepting this PR will automatically close the related issue]

[jepler]

What I think I see is that by removing the "stop", dma_hw->timer[self->pacing_timer] is no longer set to 0, causing each play-while-already-playing to allocate a new pacing timer.

If you revise your test program to repeatedly start a new looping sample without closing it first, does it soon fail due to "No DMA pacing timer found"?

I did not actually test anything, I just looked at the code.

[DavePutz]

You are correct; we need to stop if the dma channel is currently playing. So, I've made modifications in common_hal_audiopwmio_pwmaudioout_play() to detect if that is the case and only do the 'stop' if the dma channel is already in use.

[jepler]

there are also pre-commit changes that need attention. thanks for your continued work here!

[jepler]

Note that ...get_playing checks for the dma channel number:

bool audio_dma_get_playing(audio_dma_t *dma) {
    if (dma->channel[0] == NUM_DMA_CHANNELS) {
        return false;
    }

so this added check may be unnecessary. If it is necessary, please add a source comment why and check if PWMAudioOut needs the added check too.

[DavePutz]

The issue is that in common_hal_audiopwmio_pwmaudioout_get_playing() we are accessing self->pacing_timer before it has been set. So, the second PWMAudioOut will reset the timer (and so stop the first one). I could either initialize self->pacing timer in audio_dma_init() or leave the current code as is. Which do you think would be cleaner?

[DavePutz]

Decided that initializing the pacing_timer was a better way to go, so made the appropriate changes.

[jepler]

I'm still fuzzy on how this works, but if it passes your testing and the testing I suggested, then let's give it a try. I didn't perform any testing myself.