electro-smith / libDaisy

Hardware Library for the Daisy Audio Platform
https://www.electro-smith.com/daisy
MIT License
331 stars 143 forks source link

Uart wrapup #496

Closed beserge closed 2 years ago

beserge commented 2 years ago

multi-pin, multi-peripheral support via the config structs and shared DMA

github-actions[bot] commented 2 years ago

Unit Test Results

    1 files  ±0    16 suites  ±0   0s :stopwatch: ±0s 150 tests ±0  150 :heavy_check_mark: ±0  0 :zzz: ±0  0 :x: ±0 

Results for commit 9dcd89f0. ± Comparison against base commit 3b0174e3.

:recycle: This comment has been updated with latest results.

beserge commented 2 years ago

Test Procedure

I ran each of the examples/uart tests on the Daisy Patch. All RX examples were fed by a Field running the Blocking TX example. All TX was verified with my logic analyzer. If it's not mentioned here, it worked fine.

Blocking TX DMA RX / DMA TX Blocking RX

These examples are written with a simple dma flag to avoid the blocking call being interrupted by the dma call.

If you try to write it in a more normal manner

I tried using the ScopedIrqBlocker in the BlockingTx/Rx to avoid being interrupted by the DMA. However, this causes the blocking call to get stuck forever. I think there's an internal cb which readies gets the blocking call ready which we're blocking in addition to the dma. Unless we have a good way to block the DMA cb, but not whatever HAL cb we need here, I think the flag based method I used in the tests is OK for now. This is sort of an edge case anyways.

stephenhensley commented 2 years ago

I tried using the ScopedIrqBlocker in the BlockingTx/Rx to avoid being interrupted by the DMA. However, this causes the blocking call to get stuck forever.

This is because the blocking functions use HAL_Delay() which waits for the SysTick variable to increment, and if IRQs are blocked, that will never increment.

I don't think it would be the worst to require that a peripheral use only one method-type (i.e. DMA or blocking) for both rx/tx since the peripheral is linked, but I am open to other solutions. That said, it seems like a limitation of how the HAL's state machine within the UART peripheral is configured.

CorvusPrudens commented 2 years ago

Each of these was tested on USART_1 and USART_3, along with concurrent audio passthrough.

Blocking Receive

Blocking Transmit

DMA Receive

DMA FIFO Receive

DMA Transmit

Blocking Transmit + DMA Receive

Dma Transmit + Blocking Receive

Blocking Transmit + FIFO Receive

DMA^2

MIDI

One notable issue that I don't think is mentioned anywhere is the USART_1 - USB conflict. It seems like any USB usage will prevent USART_1 from working as expected in rather unexpected ways. We should probably make a note of that somewhere in the documentation.

Overall, from my testing, it seems like the functionality is solid.

stephenhensley commented 2 years ago

Just about ready to merge this

just two small things: