Hey there!

I was looking at Telemachus' source code and I noticed that you're using a custom HTTP server and hand-writing/parsing JSON. I've been making a few tools using Telemachus and have run into some rough edges with the servers:

• I've had the Websocket server refuse to send messages to new connections (maybe old connections aren't being pruned?). This happens when I'm refreshing a lot to test a tool
• Escaped query parameters aren't accepted

Unfortunately, I don't have specific examples :/

I was thinking that I could rewrite the server code to use JSON.net for JSON parsing and generation, and rewrite the servers themselves to use a standard HTTP server library (maybe HTTPServerClass). Using these libraries could reduce the amount of code that needs to be maintained, reduce the bugs people run into, and make the mod more stable.

I really like Telemachus and I want to keep building tools for it. Let me know if you'd like me to look into this. :)

@richardbunt How are things looking for peeking at some of the changes?

@ndevenish Thanks - JSON updates merged.

@ndevenish update, you might want to merge in tcannonfodder@bb6f9e0 to your branch, it adds support for POST requests with JSON bodies to the HTTP API :)

@ndevenish Hey man, I know you're working on an update to the websocket server's internals. It turns out Telemachus doesn't support POST requests, which need to be fixed up. Would you have some time to talk about how we should rewrite the internals of the HTTP API, especially to support POST requests?

An example of errors when using escaped query parameters:

GET /telemachus/datalink?ret=r.resource%5BElectricCharge%5D HTTP/1.1
Host: 127.0.0.1:8085
Connection: close

HTTP/1.0 500 Internal Server Error
Date: 7/26/2015 2:12:18 PM
Content-Length: 198
Content-Type: text/html
Server: Telemachus 1.4.30.0

Really a pity, since most HTTP clients nowadays automatically escape the query parameters.

@kleiram, thanks for posting one! I just didn't have one handy :)

Replacing parts of Telemachus with more robust solutions is certainly a good idea. The main reason this was not done in the first place, was that System.IO was locked out by SQUAD when Telemachus was written and as you can imagine this stopped pretty much all external libraries from working. However, the System.IO lock is no longer in effect and therefore there is nothing stopping a rewrite. Unfortunately I lack the time to work on a Telemachus 2.0, so if you do have some time then feel free to fork Telemachus/start from scratch and provide the KSP community with something more robust.

Awesome! I'm pretty busy too, but I think an overhaul of the server internals (without changing the interfaces to reduce the impact of the update) would be great for the overall health of the mod.

If you have some time, I'd love a quick summary of the mod's architecture, particularly the servers and the datalink with Telemachus. I browsed the code and didn't quite grok it at first glance

If you plan to simply swap out the servers for something else, it should be fairly simple to do as the Telemachus API layer is fairly well decoupled and can be found implemented in the KSPAPI class.

KSPAPI - This class essentially takes an API string and returns the corresponding value from KSP. It is used by both the web socket and HTTP servers and can conceivably be used by any other server to provide the Telemachus API.

It is instanced with a set of formatters (for spitting out the JSON), a VesselChangeDetector (used to provide information on whether there is a Telemachus antenna on the ship) and a ServerConfiguration (which is simply for providing the IP address to the API, so that it can be queried). Please find an example of instancing the KSPAPI here. Once the KSPAPI has been instanced it can be queried, of which there is an example here. It is basically 4 stage process:

1. Make a clone of DataSources for this query.
2. Use parseParams to strip off any API string arguments and store them in the DataSources copy.
3. Call process with the stripped API string and the DataSources clone, which grabs the data from the game.
4. Set the JSON variable name (setVarName) and return the result (format)

I've started having a tackle of this using the SimpleJSON library - see above patch - I just wanted something small and simple to drop in as a single file (Haven't found a websocket one yet). Also see e.g. ndevenish/Telemachus@8447e5b for another deperate attempt to wrangle sensible results out...

The straw was not being able to have : in variable names 😄

Edit: My case is Websockets, I misread, but it's similar lines

Thanks for taking a look at this @ndevenish, I'll merge this in when I am less snowed under with work. This message is just to let you know that I'm not ignoring you.

Dows this fix the issue of escaped query parameters?

@richardbunt Thanks for letting me know - I have more incoming also...

@kleiram not in this push, but I have a separate, mostly-complete branch that a) makes a load of tidying changes and b) uses websocket-sharp to handle all the serving/websockets, and it is fixed as part of that. If richard doesn't want to use it directly then I'll push out a separate fix, but it's non-trivial just to drop it in for now.

@kleiram So I looked at this again and it's actually not too bad, with one of my "Cleanup" commits, so I've merged that fix into it. It's at ndevenish/Telemachus@d486c530 . It requires the SimpleJSON fix to be merged first though.

Closed and fixed in #58