aolsenjazz / super-controller

Give your MIDI devices super powers: take control of the lights, messages, and communication between controllers.
MIT License
19 stars 2 forks source link

Linux support #5

Open assaron opened 1 year ago

assaron commented 1 year ago

Hi, nice piece of software!

However, I had troubles running it on my Linux machine (Debian 12). What are you thoughts on Linux support?

More precisely, here are my issues:

1) On Linux I have different names for the ports, for example my Novation Launchkey Mini MK3 has the following two ports: Launchkey Mini MK3:Launchkey Mini MK3 Launchkey Mi 20:0 and Launchkey Mini MK3:Launchkey Mini MK3 Launchkey Mi 20:1, as opposed to Launchkey Mini MK3 MIDI Port and Launchkey Mini MK3 DAW Port in the driver names. I had to adjust the names in the sources of the drivers to make it work. I guess some way to select the driver manually for the port should be useful.

2) There is a resource leakage, as Node rtMidi can't properly close unused connections: https://github.com/justinlatimer/node-midi/issues/118 As a woraround I had to change polling interval to 1000 seconds. I guess the proper fix would be to create new midi.Input()/midi.Output() only when needed, not on every updatePorts().

3) I couldn't figure out how to save/restore configuration properly. Should be there some menu? I have none. I only have a suggestion to save session when I close the application, and don't see any way to restore it.

aolsenjazz commented 1 year ago

Happy to provide it!

All for Linux support, and the only reason it's not currently supported is because I haven't gotten to it yet.

  1. Interesting re. port naming. Until now, I had assumed that system midi drivers would expose the same name for a given controller. Short term, I think the solution you proposed makes sense. Long-term, would be nice from a UX perspective to provide OS-specific bindings. A similar functionality to the one you proposed is already implemented on OSX w.r.t. adapter-style devices. If you've an apple computer, it might be worth running mimic, creating an Adapter Device, and seeing how SC responds to get an idea of the workflow.

  2. I don't know much about Linux MIDI drivers. Sounds like a reasonable fix.

  3. On OSX there's both a system menu and appropriate keybinds. As Linux isn't supported yet, these don't exist for it. Would be very easy to implement. Can provide more info on the steps if you've the time to write some components.

Changing label to enhancement as Linux isn't official supported

assaron commented 1 year ago

1) I'm not sure how exactly the port name is generated on Linux, in particular how stable the number 20 is. I understand that the names is combined from the ALSA port info, as its available from the output of aconnect -l command:

client 20: 'Launchkey Mini MK3' [type=kernel,card=1]
    0 'Launchkey Mini MK3 Launchkey Mi'
    1 'Launchkey Mini MK3 Launchkey Mi'

I have the same client number on two different machines (other is Ubuntu), so it's relatively stable, but I think option to select the driver should be more comprehensive. For improved UX, some default suggestions could be done by pattern matching.

3) Yeah, I think I've figured the menu thing, probably I can make it work.

assaron commented 1 year ago

And I think I have a fix for 2.

However, one thing that doesn't seem to fully work is loading configurations. I'm missing at which point the device initialization is called? Or how it's supposed to work?

My expectation is:

  1. Open SuperController.
  2. Open configuration
  3. Devices from that configuration are initialized and working.

What I have to do is:

  1. Open SuperController
  2. Add the device
  3. Load the configuration.
  4. Now the device works (although pad colors aren't reset to the off state, I have to press the pad first).

Is it expected behavior?

aolsenjazz commented 1 year ago

Your expectation is how it works on my end (on OSX). If by device initialization, you mean running the control sequence and updating light states, that happens PortService.updatePorts.

Each time updatePorts() is called, it reconciles devices which are configured in the current project (either by adding a device or loading a .controller file) against what ports are available. If a port is available and not yet connected, it will open the connection and run the control sequence and load initial light config.

Update ports is called when SC detects a change in the available midi ports (disconnect or new connection), and also twice in background.ts. Keeping track of available MIDI devices is... a fun challenge. Especially with > 1 devices connect of the same model.

My best bet as to why you need to add the device first is because loading the configuration file isn't opening the connection to the hardware ports, but by first adding to project, you're able to open them prior to loading the config. Lots of entropy here though, so this is a shot in the dark.

aolsenjazz commented 1 year ago

Some googling seems to confirm that client 20 is stable, at least for one device (1-2 ports) in multiple Linux flavors. Multiple forums with users showing similar client listing.

My main question then would be - how does multiple midi devices affect this? Multiple devices of same model?

assaron commented 1 year ago

My main question then would be - how does multiple midi devices affect this? Multiple devices of same model?

I believe the client numbers are just incremented for the consequent devices. I have some vague memory, that I was disconnecting midi-keyboard when it was still used, so that the port wasn't properly closed, and on reattaching there were port numbers like 21 and 22.

But I don't think it matters much, the part with the client number can be just stripped away, keeping the name and the subport number.

assaron commented 1 year ago

I found the problem why it wasn't initialized -- there was a null pointer exception in my code which wasn't reported anywhere for some reason.

I'll clean the changes and make a pull request a bit later.

assaron commented 1 year ago

There is an interesting thread on midi port names in Linux, in particular this message is informative: https://linuxmusicians.com/viewtopic.php?p=147380#p147380 In short, I wouldn't rely on any particular naming scheme, as apparently it can be changed with kernel updates.

assaron commented 1 year ago

I created a pull request with working version, but for a full support there is still need and option to select driver manually. Could you implement it? I hope it shouldn't be hard but it would require a bit more global changes: configuration would need to include the driver name, not just a device name.

assaron commented 1 year ago

And another thing: does "Save As" dialog on macOS adds file extension automatically? For me I have to manually append ".controller".

aolsenjazz commented 1 year ago

Thanks for your work! Looking forward to checking it out.

should be easy to implement, will try to hit it this week

on osx, .controller is prepopulated in the dialog

aolsenjazz commented 1 year ago

Added the bits you'll need in 8eb2585a8ad732b927b7b34bfbdeb8a054f007a1

Can you confirm that everything looks right on your os? I had everything inverted so I could test UI elements on osx. Should have reinverted everything correctly though.

Also, would you confirm that message propagation work correctly for both named drivers and anonymous drivers (using the translator function)? Thanks

assaron commented 1 year ago

Thanks for the quick work!

I had a major problem though: my mk3 is not initialized for some reason. The control sequence seem to be sent, but there is no response from the device. It should both change light stats physically, as well as to respond with some messages, neither of which happens. Do you have any ideas what could happen?

Also one relatively minor thing appeared: the port names don't fit into the left panel, so I can't see which of the ports is :0 (aka MIDI) and which is :1 (aka DAW). Text wrapping or some other way to see the full port name would help there.

2023-09-05-214953_1920x1043_scrot

assaron commented 1 year ago

@aolsenjazz I've figured out what was the problem. The DrivenPortPair was initialized with an incorrect driver.

https://github.com/aolsenjazz/super-controller/blob/dev/src/main/port-service/port-manager.ts#L44 -- here the driver is looked up by the port name, which does not work on linux.

I was able to make it work by setting the proper driver just before pp.runControlSequence() https://github.com/aolsenjazz/super-controller/blob/dev/src/main/port-service/index.ts#L67 with these lines:

const driver = getDriver(config.driverName);
pp.driver = driver;

I guess this can be the way to go, and actually the driver can be set to null first in DrivenPortPair constructor and only initialized properly in initDevice, where we do know which driver to use.

aolsenjazz commented 1 year ago

Thanks for looking into this!

Given the decoupling of drivers and config, it doesn't seem like good design to have ports and drivers coupled anymore imo; it makes more sense to me to look up drivers via the DeviceConfig object. Better/more linear relationships. I'll look into removing DrivenPortPairs and replacing with lookup via configs.

Also, opened #8 to make sure that doesn't slip thru cracks