justinlatimer / node-midi

A node.js wrapper for RtMidi providing MIDI I/O
MIT License
745 stars 119 forks source link

getPortName returns incorrect value #199

Closed aolsenjazz closed 3 years ago

aolsenjazz commented 3 years ago

getPortName returns the incorrect name for the Impulse25 MIDI Controller. Other clients (Ableton, MIDI Monitor) see the device's name as 'Impulse' whereas getPortName return 'Impulse Impulse '.

Code to reproduce:

const midi = require("midi");

const input = new midi.Input();
const portName = input.getPortName(0).split(" ").join("_"); // replace spaces with underscores for visibility

console.log(portName);
// logs Impulse__Impulse_

Environment deets: OSX 11.2 Core MIDI Node 14.4.0 Please let me know what more info I can provide.

Will investigate this further but I've little-to-no experience with native node libraries :). Thanks for writing and making available such a great library.

justinlatimer commented 3 years ago

Looks like the name of the port is determined in RtMidi here. Not sure why it concatenates the name like that?

aolsenjazz commented 3 years ago

Thanks for the quick reply. It looks like rtmidi is giving a weird name because the Impulse25 includes the Device name in its Endpoint names (which I suppose isn't? conventional). If you'd like, I can open a PR in rtmidi to check for existence of Device name in Endpoint names and return a more appropriate name.

aolsenjazz commented 3 years ago

The device I'm using has a leading whitespace in its endpoint name, messing up that concatenation logic 🙃🙃.

Fixed with a CFStringTrim.

Opened PR#248 in rtmidi. Will update if merged.

justinlatimer commented 3 years ago

Nice one! If you don't get traction with upstreaming to RtMidi in a few weeks, we might be able to just put in the fork node-midi uses directly.

aolsenjazz commented 3 years ago

Excellent. Will open a PR in due time if that's the case. Thanks again!

aolsenjazz commented 3 years ago

Merged in rtmidi

justinlatimer commented 3 years ago

Updated RtMidi in 7c659437963c311bc62d94898ad8867e220a8899, published to npm in v2.0.0