electro-smith / libDaisy

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

Fix issue with new uart class and midi #539

Closed beserge closed 1 year ago

beserge commented 1 year ago

The FIFO based mode had the double buffering happening in two different places. Now, it still happens in both places, but it depends on one function, and it wont actually do the operation twice.

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 bb08e632. ± Comparison against base commit 5b7d7f33.

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

stephenhensley commented 1 year ago

works like a charm!

jwjmayo commented 1 year ago

updated libDaisy and looks like midi trs in has become very unstable, after exactly 42 notes (84 events) the midi handler behaves very strangely. Could it be related to this update? Should I be updating the HandleMidiMessage code?

TheSlowGrowth commented 1 year ago

Can you provide a reproducer for us to replicate the issue?

jwjmayo commented 1 year ago

Thanks. I’ll see if I can come up with something next week.

On Sat, 3 Sep 2022 at 10:54, TheSlowGrowth @.***> wrote:

Can you provide a reproducer for us to replicate the issue?

— Reply to this email directly, view it on GitHub https://github.com/electro-smith/libDaisy/pull/539#issuecomment-1236086798, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVK6QRAGMNLWMKHZDMLS55TV4MN55ANCNFSM55PIYSUA . You are receiving this because you commented.Message ID: @.***>

-- 208 Grand Union Studios 332 Ladbroke Grove London W105AD

jwjmayo commented 1 year ago

OK so I compared my midi handler code with the patch/Midi and mine was missing the if(m.data[1] != 0) condition inside the noteOn noteOff cases. Adding that code into my midi handler seems to have solved the problem.

stephenhensley commented 1 year ago

@jwjmayo prior to this fix, there was an issue with the internal callback for UART handling being called multiple times per UART transaction.

This would be seem to function properly for the first several MIDI messages, and then quickly degrade after that.

The issue existed briefly after the last release (where UART was updated quite heavily), and the previous fix for the last fix for MIDI message handling.

It's curious that your parsing of 0-velocity note messages needed to change between those updates, though I believe handling of those was touched during the fix for the previous status-byte issues found in mid-May.

Feel free to open a new issue if it's behaving outside of the expected behavior, or if you're still having issues.

jwjmayo commented 1 year ago

I’m testing the trs midi at the moment and it is still occasionally a little unstable so I’ll try to ascertain how to reproduce it and raise an issue.