-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WebSocket: Define a communication protocol #6
Comments
I can implement this change if you @Sharlottes and @cardillan agree. |
I think this is a very good idea. I wanted to propose adding support for sending schematics to Mlog Watcher via WebSockets (and retire SchematicRefresher along the way) one day, but I don't have it all thought out yet. Do you think you could provide a sample client code for Javascript? I currently have a Java integration (in the command line compiler) and Javascript integration in an unreleased version of Mindcode. I can handle Java, but I don't do front-end and sometimes struggle with the web app. |
Sure, here is how the javascript code might look like. It uses a few adhoc constructs like the class JsonRpcConnection {
constructor(socket) {
// ids can be strings or numbers
// this is just for demonstration purposes
this.currentId = 0;
// store callbacks for pending requests
this.pending = new Map();
this.socket = socket;
// handle incoming messages
socket.addEventListener("message", (event) => {
const response = JSON.parse(event.data);
if (response.id) {
const resolve = this.pending.get(response.id);
if (resolve) {
resolve(response);
this.pending.delete(response.id);
}
}
});
}
send(method, params) {
const id = this.currentId++;
const message = JSON.stringify({
jsonrpc: "2.0",
id,
method,
params,
});
this.socket.send(message);
return new Promise((resolve) => {
// store the resolve function for later
this.pending.set(id, resolve);
});
}
}
const socket = new WebSocket("ws://localhost:9992");
const conn = new JsonRpcConnection(socket);
// jsonrpc version of what we currently have
function sendSelectedProcessorUpdate(conn, code) {
conn.send("updateSelectedProcessor", [code]);
}
// other possible methods we might want in the future
function sendProcessorUpdate(conn, position, code) {
conn.send("updateProcessor", [position, code]);
}
// stub for demonstration purposes
async function sendListProcessorsRequest(conn) {
const response = await conn.send("getProcessors", []);
// TODO: handle errors
if (response.error)
return null;
return response.result;
} |
I've checked the specs, and I don't think we need to use that library, because it requires some boilerplate code for generality. Also, while it's reasonable to validate payloads with each other in anticipation of more complex features at this point, I think it's overkill to prepare for unplanned RPCs. ...so it looks like we only need to format the JSON to go into the message value of @JeanJPNM's interface CustomPayload<T extends Record<string, unknown>> {
id: string;
data: T;
} // in JsonRpcConnection#send method
this.socket.send(JSON.stringify({
id,
// because it dosen't have to rpc, 'type' name will be naturely
type: 'CHECK_STATUS', // number could be lighter
data: { }
})) |
Agreed, I think it's good enough for this purpose, so now the only steps missing are:
|
The creation of #4 made me think about how the communication method used by this mod is going to become more complex as time goes on and more features are added. So I believe it may be necessary to use a communication protocol like json rpc to make the addition of new features more straightforward and backwards-compatible.
Of course, this is not required at the moment, but I think it may become relevant in the near future.
The text was updated successfully, but these errors were encountered: