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

Make USB MIDI interface names customizable #9146

Merged
merged 3 commits into from Apr 6, 2024

Conversation

brushmate
Copy link

@brushmate brushmate commented Apr 5, 2024

This pull request should solve the issues described in #4191 by introducing 4 new methods to the usb_midi module that allow for customizing the MIDI streaming interface, audio control interface, MIDI IN jack, and MIDI OUT jack names.

As suggested by @dhalbert I took a look at #8989 and basically just copied the code and adapted it to usb_midi. Since I have like zero experience in C programming I would be glad to get some feedback on the code. I feel like there are some unnecessary repetitions.

I tested this on a Raspberry Pi Pico.

@brushmate brushmate force-pushed the customizable_midi_interface_names branch from 03a2e72 to ca881e1 Compare April 5, 2024 12:04
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

Instead of four different routines, how about a single routine with optional arguments? Here is the signature:

usb_midi.set_names(self, *, streaming_interface_name = None, audio_control_interface_name = None, in_jack_name = None, out_jack_name = None)

They will be MP_ARG_KEY_ONLY and won't change if the arg is the default None. There are a lot of examples of this kind of thing in constructors, and also in .configure in busio.SPI.configure().

@brushmate brushmate force-pushed the customizable_midi_interface_names branch from ca881e1 to ae9197b Compare April 5, 2024 20:42
@brushmate
Copy link
Author

I guess that makes sense. I was able to implement this method, but there is a lot of repeated code for each argument. What would be an idiomatic approach for getting rid of the repetitions?

@dhalbert
Copy link
Collaborator

dhalbert commented Apr 6, 2024

I guess that makes sense. I was able to implement this method, but there is a lot of repeated code for each argument. What would be an idiomatic approach for getting rid of the repetitions?

You could just factor the repeated code into a static helper routine that takes two arguments, the input name and where to put it.

@brushmate brushmate force-pushed the customizable_midi_interface_names branch from ae9197b to 814ebc6 Compare April 6, 2024 10:09
@brushmate
Copy link
Author

I tried to do that, but failed. CircuitPython will always fall back to the default values. I guess something is wrong with all this pointer magic, but I have no clue how to properly debug the code.

@dhalbert
Copy link
Collaborator

dhalbert commented Apr 6, 2024

I am recovering the ae9197b change before the refactoring I requested, and will try to fix this up on your fork branch.

@dhalbert
Copy link
Collaborator

dhalbert commented Apr 6, 2024

I touched up the last commit so it works, touch up the documentation, and also fixed a build overflow problem.

test boot.py:

import usb_midi
usb_midi.set_names(streaming_interface_name="new_streaming_interface_name", audio_control_interface_name="new_audio_control_interface_name", in_jack_name="new_in_jack_name", out_jack_name="new_out_jack_name")

Testing the name setting:

$ sudo lsusb -vd 239a:80f2 | egrep 'iJack|iInterface'
      iInterface              4 CircuitPython CDC control
      iInterface              5 CircuitPython CDC data
      iInterface              6 CircuitPython Mass Storage
      iInterface              7 CircuitPython HID
      iInterface              9 new_audio_control_interface_name
      iInterface              8 new_streaming_interface_name
        iJack                  10 new_in_jack_name
        iJack                   0 
        iJack                  11 new_out_jack_name
        iJack                   0 

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thank you for this!

@dhalbert dhalbert merged commit 0f66ecb into adafruit:main Apr 6, 2024
415 checks passed
@dhalbert dhalbert mentioned this pull request Apr 6, 2024
@brushmate brushmate deleted the customizable_midi_interface_names branch April 7, 2024 10:16
@brushmate
Copy link
Author

Thank you for the assistance and the quick feedback on this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

usb_midi Device Name
2 participants