Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MIDI output endpoint to use the out direction #18654

Merged
merged 1 commit into from
Oct 9, 2022

Conversation

nezumee
Copy link
Contributor

@nezumee nezumee commented Oct 9, 2022

Description

Bug: cannot send MIDI messages to keyboard. The callback functions registered via midi_register_* are never invoked. This blocks implementing things like MIDI controller feedback where DAW software sends messages to the controller to turn indicator lights on/off etc.

Fix: update the endpoint direction to match the stream direction for MIDI output in lufa.c. The IN/OUT mismatch was introduced in f209f91, likely a copypaste error.

Note that I don't actually understand how this works :), the fix came from reviewing code and applying common sense. I did validate in a custom keyboard that this change unblocks sending MIDI notes to the keyboard and receiving them via midi_register_noteon_callback.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added the core label Oct 9, 2022
@nezumee
Copy link
Contributor Author

nezumee commented Oct 9, 2022

First time making a PR here, please say if I'm doing anything wrong.

This is a small fix for what's likely a rarely used feature (based on the state of the docs). I think it can be useful though, in case people want to make MIDI controllers based on QMK.

Copy link
Contributor

@keyboard-magpie keyboard-magpie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A very tidy first PR, well found.

@zvecr zvecr added the bug label Oct 9, 2022
@zvecr zvecr merged commit d6d6cdc into qmk:develop Oct 9, 2022
ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants