search

electro-smith / libDaisy

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

UART DMA Rx rework #557

Closed stephenhensley closed 1 year ago

stephenhensley commented 1 year ago

This is a "breaking" change to the API, but this should be justified on two accounts:

  1. The existing internally managed "FIFO" DMA Rx mode of the UART peripheral isn't very stable -- the "normal DMA" mode stuff introduced in versions after v5.0.0 seemed to have made this unstable.
  2. The MIDI implementation used this mode, and can be migrated without affecting others' code. Otherwise, migration is not too crazy.

That said, I understand an argument for leaving the "FifoMode" functions in place, and managing the fifo internally, it just doesn't provide the flexibility of being able to resize, or manage the callback more yourself for particular applications. One other solution would be to leave the old functions in place with a "default" callback/FIFO, and have that be replaceable.

So this adds a new "DmaListen" mode that starts up a circular receive that will also trigger on the IDLE interrupt (one full frame of high-bytes). This then updates the byte-buffer passed in (w/ cache-maintenance), and calls the user callback to do whatever with that amount of data. This is very similar to the fifo mode that was there (and could likely fill that same slot), but I pulled it out of the "new standard DMA" stuff, and simplified this particular use-case so that any introduced error could be easier to find.

The MidiUartTransport was updated to use this new mode with a callback that feeds a FIFO for the MIDI Parser to consume. (No changes to the user facing API of the MIDI stuff).

This feels 99% more stable than the latest few libDaisy versions, but still occasionally glitch (possibly overrun or something) when a lot of data is coming through (>150BPM 8x 16th notes w/ CC, and clock messages). v5.0.0 still feels a bit more stable in comparison, and I'd like to poke around a bit more to see what the final issues might be.

Otherwise, I'm feeling pretty good about this.

Lastly, this PR is based off of #555 since that may be getting merged in first, I'll rebase if necessary depending on when that goes to master.

github-actions[bot] commented 1 year ago

Unit Test Results

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

Results for commit 0e42d355. ± Comparison against base commit 07a813a0.

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

stephenhensley commented 1 year ago

this is now duplicated by #583 so I'm going to close it.