robbielyman / seamstress

seamstress is an art engine
GNU General Public License v3.0
129 stars 12 forks source link

MIDI ports aren't populated until after script init #57

Closed dndrks closed 1 year ago

dndrks commented 1 year ago

hihi! hope all's well :) i ran into something unexpected when working on extending the PMAP system to include MIDI echo (for devices like the MIDI Fighter Twister), which seems useful to raise!

test case

i'd like to capture the names of all the connected MIDI output devices in an option-style parameter, so i connect a few MIDI devices to my computer and run this script:

function init()
  device_names = {}
  for i = 1,16 do
    device_names[i] = i..": "..midi.voutports[i].name
  end
  params:add_option('midi_target','midi target',device_names)
end

expected results

at script load, my midi_target parameter will initialize with a list of all the currently-connected MIDI output devices. this is also the result from running the script on norns.

actual results

the script initializes fine, but my connected MIDI output devices don't populate in the list -- everything past 1: seamstress_out shows up as x: none.

to work around this, i can force a refresh after a 1 second delay:

function init()
  device_names = {}
  for i = 1, 16 do
    device_names[i] = i .. ": " .. midi.voutports[i].name
  end
  params:add_option("midi_target", "midi target", device_names)

  clock.run(function()
    clock.sleep(1)
    force_refresh()
  end)
end

function force_refresh()
  for i = 1, 16 do
    device_names[i] = i .. ": " .. midi.voutports[i].name
  end
  params:lookup_param("midi_target").formatter = function(param)
    return device_names[(type(param) == "table" and param:get() or param)]
  end
  paramsMenu.redraw()
end

^ this does collect all connected devices into the parameter options as expected. also, in case it helps to have confirmed, i get the same results with midi.vinports.

extra diggin'

idk if this is helpful, but along the debug trail i popped a few prints to try and understand the timing of everything.

i added this to core/midi.lua (when the connected state is updated):

function Midi.update_connected_state()
  for i = 1, 16 do
    if Midi.vinports[i].device ~= nil then
      Midi.vinports[i].connected = true
      print("input device " .. i .. " connected", os.clock()) -- DEBUG PRINT
    else
      Midi.vinports[i].connected = false
    end
    if Midi.voutports[i].device ~= nil then
      Midi.voutports[i].connected = true
      print("output device "..i.." connected", os.clock()) -- DEBUG PRINT
    else
      Midi.voutports[i].connected = false
    end
  end
end

and added these lines to bookend the init in my test script:

print("script init started: " .. os.clock())
[other stuff]
print("script init done: " .. os.clock())

which returns the following on script load:

SEAMSTRESS
seamstress version: 0.20.4
input device 1 connected    4165.252375
input device 1 connected    4165.252409
output device 1 connected   4165.252414
script init started: 4165.2619
script init done: 4165.26194
>> searching for MIDI mapping: <path_to_pmap_file>
>> MIDI mapping file not present, using defaults
input device 1 connected    4165.527454
output device 1 connected   4165.527589
input device 2 connected    4165.527601
input device 1 connected    4165.527627
output device 1 connected   4165.527635
input device 2 connected    4165.527646
input device 3 connected    4165.527653
input device 1 connected    4165.527679
output device 1 connected   4165.527688
input device 2 connected    4165.527695
output device 2 connected   4165.527701
input device 3 connected    4165.527705
input device 1 connected    4165.52773
output device 1 connected   4165.527736
input device 2 connected    4165.527743
output device 2 connected   4165.52775
input device 3 connected    4165.527756
output device 3 connected   4165.527765

if that first instance of output device 1 connected were to snag all the connected output devices, the timing would work out great for the example script, but it looks like there are a few additional calls to midi.update_connected_state() before the additional devices are discovered and logged, which comes after the script has already initialized?

hope this helps, please lmk if there's anything i can do to assist!!

p3r7 commented 1 year ago

oh, yeah, i stumbled upon that. i thought it might have been a conscious decision (non-blocking midi device registration?).

i just bind midi devices post-init w/ the midi.add callback and do something very unholy for params containing a list of midi devices: resetting the param definition after each new device is found/removed.

it is kinda cool as devices can be hot-pluggued & unpluggued without having to register them anywhere beforehand. but it's true that for when having a param to hold the list of devices it's trickier to handle API-wise.

robbielyman commented 1 year ago

Thanks for the very clear issue report!

I think the issue is this line of midi.zig which tells the MIDI discovery thread to wait one second before starting up. https://github.com/ryleelyman/seamstress/blob/b974b80a5ab68b220510573d92f78531133c505a/src/midi.zig#L171

Should be an easy fix; gives me a chance to clean up this file a little too. Thanks!