Skip to content

Make USB MIDI interface names customizable - #9146

Merged
dhalbert merged 3 commits into
adafruit:mainfrom
brushmate:customizable_midi_interface_names
Apr 6, 2024
Merged

dhalbert merged 3 commits into
adafruit:mainfrom
brushmate:customizable_midi_interface_names

Conversation

@brushmate

@brushmate brushmate commented Apr 5, 2024 •

Copy link
Copy Markdown

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

@dhalbert dhalbert left a comment •

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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

dhalbert commented Apr 6, 2024

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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

dhalbert commented Apr 6, 2024

Copy link
Copy Markdown
Collaborator

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

@dhalbert

dhalbert commented Apr 6, 2024

Copy link
Copy Markdown
Collaborator

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 

@dhalbert dhalbert left a comment

Copy link
Copy Markdown
Collaborator

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
@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
Copy Markdown
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