Properly handle Program Change and Channel Aftertouch messages #1162
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hathach
reviewed
Oct 25, 2021
| @@ -279,6 +279,12 @@ uint32_t tud_midi_n_stream_write(uint8_t itf, uint8_t cable_num, uint8_t const* | |||
| stream->buffer[0] = (cable_num << 4) | msg; | |||
| stream->total = 4; | |||
| } | |||
| else if ( msg == 0xC || msg == 0xD) | |||
There was a problem hiding this comment.
may be using the enum MIDI_CIN_PROGRAM_CHANGE and MIDI_CIN_CHANNEL_PRESSURE would be better. I know others are using magic number. We will change it later on to a switch to prevent missing case like this.
There was a problem hiding this comment.
Yes, the if statements above and below should also be enums (0x8, 0xE, 0xF) but I was trying to match style.
|
Thanks! I can do a separate PR that changes all the magic numbers to enums. |
hathach
approved these changes
Oct 25, 2021
Don't worry about it if you don't have time. I will do that refactor work later on next time I need to do something with midi. The PR is good to merge now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the PR
MIDI Program Change and Channel Aftertouch messages are not currently handled. They are sent correctly by accident, depending on leniency of host OS version and if
tud_midi_n_stream_write()is called with a buffer containing a whole message buffer and not byte-by-byte. These types of MIDI messages are 2-bytes long (instead of the common 3-bytes) andtud_midi_n_stream_write()doesn't have case to handle them. This PR adds that case.Additional context
See issues adafruit/Adafruit_TinyUSB_Arduino#130 and adafruit/Adafruit_CircuitPython_MIDI#37 for backstory.
Also see this TinyUSB test program based off of
midi_test.