Open jbflow opened 5 months ago
File Rename and Updates
The existing file AbletonJS.py
was renamed to AbletonJSBase.py
and within this file, several updates were done including the removal of unused imports or variables and a change in the class name from AbletonJS
to AbletonJSBase
.
Introduction of New Property
A new property socket
was added to the class, along with accompanying getter and setter methods.
Updates to Existing Methods
Two previously existing methods, tick
and command_handler
, were updated to better serve their function.
Removal of A Method
The method disconnect
was removed entirely from the class.
Addition of New Files and Classes
Three new files, AbletonJSTCP.py
, AbletonJSUDP.py
, and Socket/Interfaces.py
, each containing a new class, were added.
Socket Handlers
Two files, Socket/TCPSocket.py
and Socket/UDPSocket.py
, were added/updated as new socket handlers for TCP and UDP respectively.
Shutdown Method
A new method shutdown
with enhanced capabilities was added to the socket class.
Socket Type Flag
The __init__.py
file was updated to use either AbletonJSTCP
or AbletonJSUDP
depending on the socket_type
flag.
Added Dependencies
Finally, new dependencies were added to the package.json
file to provide the resources required by the new functionality.
The overall changes aim to improve the organization of code, increase efficiency, and make way for new, improved classes to handle TCP and UDP sockets.
That's awesome, thank you so much for this PR and the work you put into making this work with WebSockets! Let's check if it would be possible to make this into a package that can be used on NodeJS, Deno, and in the browser.
There's a build tool called DNT that allows us to write TypeScript focused on the browser and that takes care of creating versions that can be used on NodeJS by, for example, shimming the WebSocket implementation automatically.
For the client side, something like PartySocket might be interesting to look into as it nicely automatically handles reconnects when the connection drops for some reason.
If you're happy with the Python script's side of the PR, we could merge it into a separate branch and I can take care of the TypeScript side. What do you think?
I'm also curious about the latency of WebSockets running in a separate thread compared to the current UDP approach of polling for data every few milliseconds. Have you done any tests to compare the two in that regard?
No problem, it's great to find other devs working with the Live API. Thanks for sharing theses resources. 🎆
Not heard of Deno or DNT before so will have a read.
👀 PartySocket - as I am currently handling that functionality manually.
I have a few tweaks to make to the Python script that I have made in my own project so I will reflect those here, so yeah merging to a new branch sounds like a good plan, will push my changes over the next 24 hours for you to review.
Haven't looked at testing yet but it's something that needs doing. when I update this PR I will roll back some of the breaking changes so we can look at running both implementations side by side to compare.
@leolabs PR ready for review, breaking changes and clutter removed and working from node as before or you can switch it to TCP to connect from a websocket. :)
Thank you, I'll have a look at this now! I think once we get the WebSockets implementation working properly on the JS side, we can safely remove the UDP implementation and just release this as a major version with potential breaking changes to keep the code simple.
Regarding the port situation, I think you asked about it in one of the other threads, we could use an approach where the Python plugin chooses a free port out of a list of, say, 8 predefined ones (e.g. 39031, 39032, 39033, 39034, 39035, 39036, 39037, 39038). This should avoid conflicts where the OS is already using a port, and the client library can just try to connect to each port and if that isn't available, use the next one.
Usually, the first port in the list is free and can be used so this approach shouldn't have a huge performance impact for establishing a connection. What do you think?
Awesome, the approach I've taken should allow for both to run side by side until we're confident in this approach, the limited testing I did last night it doesn't seem to have broken anything for running in Node.
With regards to ports, that was the approach I was thinking of taking, id agree that it's unlikely we're going to encounter another bound port, the most likely scenario is if live crashes or our own socket fails to close and unbind properly and then it fails to reconnect after starting because it's still open and bound to the previous instance, in which case we can just move to one of the next predefined ports.
Another note on this PR which I forgot leave in my comments, I started looking at chunking for large message sizes but had to remove it because there was an issue with framing, I'm handling this in my code by batching the messages, combined with the queue it seems to work ok, might also be possible to limit the queue size and have it send once it hits a certain size to keep the package size below the limit. What are your thoughts on this?
Hey @jbflow, please excuse my super late reply on this! I haven't found the time to thoroughly test this PR yet, but will hopefully get to it next week.
I think I have removed all breaking changes so this should work as before. I have set it back to using the UDP socket with an optional variable you can change in the init file to have it run the TCP socket, and also implemented a socket interface and a base class for the main script so we can test and compare the two sockets and this should help to optimise the code without breaking anything else.
Confirmation that this method does work from postman below.
What:
To test TCP socket
To do: