TCP support

[daveyarwood]

Hi, thanks for go-osc, it's great!

I'm working on adding TCP support to go-osc, both on the client and server side. In fact, I've just finished implementing it on my fork. It will be a while before I can make a PR, as I'm still testing the changes I've made. But I figured I would file an issue to give you a heads up and get some feedback about the changes, which I can summarize here.

API level

• Clients and servers have a SetNetworkProtocol(NetworkProtocol) method.

  • NetworkProtocol is a new enum with values UDP and TCP.
  • UDP is the default. You can opt in to TCP: client.SetNetworkProtocol(TCP)
  • This is the only change you need to make in order to send/receive TCP packets instead of UDP.

• Added a NewServer constructor function that mirrors NewClient

  • UDP is the default network protocol, can be overridden with server.SetNetworkProtocol(TCP)

• Incidental: added a server.CloseConnection function to facilitate testing and give users the ability to forcibly stop a running server.

  • server.serve sets server.udpConnection or server.tcpListener to the connection after creating it.
  • server.CloseConnection closes the appropriate connection if it's not nil.
  • ListenAndServe implementation includes defer server.CloseConnection(), ensuring that the connection it creates is closed in the event of an interruption or error.

Implementation level

• Added the following fields to the Client struct:

  • laddrTCP *net.TCPAddr
  • networkProtocol NetworkProtocol

• Added the following fields to the Server struct:

  • udpConnection net.PacketConn
  • tcpListener net.Listener

• client.SetLocalAddr sets laddr or laddrTCP, depending on the protocol

• Added a server.ServeTCP(net.Listener) that works like server.Serve(net.PacketConn).

  • Most of the implementation is the same, so I factored it into a common server.serve(func() (Packet, error)) function.
  • The only thing that's different is the mechanism for reading a packet.

• Added a server.ReceiveTCPPacket(net.Listener) that works like server.ReceivePacket(net.PacketConn)

  • I had to do things slightly differently because we can't make assumptions about the size of the packets. So instead of making a 65535-byte array and reading data into it, I used ioutil.ReadAll to read all of the bytes (until EOF) into a byte array.
  • Added some tests involving a million-byte message (works with TCP only; UDP has a packet size limit).

• Adjustments to tests

  • Refactoring to enable reusing existing tests to test both UDP and TCP.
  • Added a randomly generated string argument to the OSC messages in order to test packets being of a certain size.
  • Handled errors in a couple of places where they were being ignored.
    • This unearthed an issue where tests fail when the server is stopped, because it was in the middle of trying to read a packet and we closed the connection. I added a workaround to get tests passing.

[daveyarwood]

I'll file a PR soon - it may be a week or two, as I'm working on testing my changes against a Java OSC library that also doesn't have TCP support yet.

Let me know if you have any feedback on the changes I outlined above!

[loffa]

I'm concerned about the future for this project as an upstream. I have a PR from almost 3 years back that have not been answered. Same situation for other PRs from earlier this year.

It seems like @hypebeast is not around anymore to answer for this repo. It might be possible to use one of the forks as the new repo for future usage.

[daveyarwood]

I noticed that, too. I'd be happy to maintain my fork as the new "official" one. In the process of working on TCP support, I learned quite a lot about the library, and I feel like I could do an OK job of reviewing and merging PRs, including the ones that have gotten stale here.

@hypebeast Any objections?

[glynternet]

I'd happily take on some of the maintenance, too, if this were to happen.

[glynternet]

If @hypebeast is not against it, we could breakout into a separate org

[daveyarwood]

I think creating a break-out org specifically for go-osc is a great idea. Let's give @hypebeast a few more days in case there is an objection, and then I'll go ahead and create the org. At that point, we can ideally transfer this repo there (would need to be done by @hypebeast), or we could create a new fork under that org.