search

emlyn / sonic-pi-tool

🎻 Controlling Sonic Pi from the command line, in Python.
Mozilla Public License 2.0
62 stars 8 forks source link

Add support for enabling and configuring cue server #20

Closed lilyinstarlight closed 3 years ago

lilyinstarlight commented 3 years ago

This PR adds support for enabling the cue server from start-server and adds a --cue-server argument for setting the listener to be internal, external, or off. I've tested it successfully in all three configurations by checking the erlang.log output and with test code similar to the example from #19.

Notably this change requires the Server instance to be passed to the Installation.run method for sending OSC commands where before the two classes had no interaction. Let me know if passing the object as a parameter to Installation.run is not the preferred method for being able to send OSC commands after server initialization.

Also let me know if something like an enum or two bools is preferred when calling Installation.run instead of the plain string from the command line (with click ensuring the string is one of three choices).

Fixes #19

lilyinstarlight commented 3 years ago

I could only get this to happen one time, but it does appear that the Sonic Pi server may say Sonic Pi Server successfully booted before the Erlang server is ready leading to a race between the Erlang server starting and the Ruby server forwarding the cue stuff to it.

See race.txt for logs from the one invocation I could get it to happen for.

emlyn commented 3 years ago

This is great, thanks!

I am a little unsure about passing the Server instance into Installation.run though. I wonder if it would be cleaner to have a method on Server to configure the cue server, and have Installation.run take a callback that it calls once the server is started. Then start_server could pass in a nested function, or a lambda, as the callback. If you want to give that a try that would be fantastic, otherwise I'm happy to merge it as is for now, then I can look into modifying it myself later when I've got a minute.

That race condition is annoying. Maybe there's something else that could be looked for in a log file to determine when everything is definitively started. Otherwise maybe a small sleep (although ugly, and not guaranteed to work) might reduce the chance of it happening.

lilyinstarlight commented 3 years ago

Yeah I thought a callback would be a better idea but I felt it would have made the diff a bit more intrusive. I'll see if I can move the logic to a callback. You mention a nested function, are you just meaning a nested function definition in start_server?

As for the race condition, I could only get it to happen once (by accident) during many runs and I went to see how Sonic Pi itself handles it. Well it doesn't seem to handle it as far as I could tell but it does a bunch of GUI stuff between getting "server successfully booted" and setting up the cue server so maybe the delay between those is enough to prevent it from occurring in Sonic Pi. I also noticed that one of the Erlang startup lines is after "server successfully booted" as of the 3.3.0 beta (this did not happen in 3.2.2) so it is possible this is a recent bug in Sonic Pi (and I think the Erlang stuff was recently reorganized). Notably this Erlang startup line happened after the race error messages in the example that it occurred.

lilyinstarlight commented 3 years ago

I've pushed a version (rebased to the latest commit now) using a nested function definition in start_server as a callback and tested it successfully. Let me know if this looks good or if you have changes you want in naming of variables or anything else related to the PR.

I might play around from the Sonic Pi end with the race condition as I'm not certain a fix for that should be in this project if it is a regression from 3.2.2.

emlyn commented 3 years ago

This looks good, thanks so much!