Skip to content
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 server #4130

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

austinmilt
Copy link

@austinmilt austinmilt commented Dec 3, 2024

This PR adds a base websocket server implementation, to be used for two-way JSON-based communication with clients in both event broadcasting and request/response messaging, such as to send commands to the emulator. The implementation is meant to stand alongside the existing SocketServer (more performant, lower-level, less dev friendly) and the websocket "server" (renamed to WebSocketClient) which is actually a client implementation for connecting to and sending messages to external websocket servers.

Some notes on the implementation:

  • It uses a single channel for all communications and differentiates messages by a topic ID enum in request/response message wrappers. The primary advantage with this implementation is clients get the send/receive to/from a single URL.
  • The message wrappers use composition for topic-specific messages (i.e. the registration-specific message lives under the registration field of the wrapper). The primary advantage here is that clients can use a single implementation to serialize/deserialize all messages in a single pass over the JSON.
  • Clients may be registered for topics by registration requests or by force (for e.g. the error and registration topics).
  • Being registered for a topic does not preclude a request message being handled, e.g. a client can send an echo request without getting it echoed back.
  • Topics can be broadcast - goes to all clients registered for that topic - or client-specific - goes to a specific client based on some other message filtering or request/response. The implementation here does not use any broadcast topics. Those can be added in future topics.

Main Changes

  • renamed the prior WebSocketServer to WebSocketClient to match its usage, and make room for the new websocket server added here (WebSocketClient, CommApi)
  • added a websocket server implementation as described above including client registration and generic methods for messaging clients (WebSocketServer)
  • added messages for three topics - topic registration (Registration.cs), echo for debugging (Echo.cs), and generic errors (Error.cs) - which are wrapped in wrappers described above (MessageWrapper.cs) and described by their topic enum (MessageWrapper).
  • added CLI args for creating the websocket server (ArgParser, IMainFormForApi, ParsedCLIFlags, MainForm, CommApi) and use these to initialize the server (MainForm)

Check if completed:

// this could happen if, for instance, the client sent a registration request to the echo topic, such
// that we tried to access the wrong field of the wrapper
// TODO proper logging
Console.WriteLine("Error handling message {0}", e);
Copy link
Author

@austinmilt austinmilt Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would appreciate advice on how to properly do logging (both debug and otherwise)

: null
);
// TODO better place to put the start of the server?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would appreciate advice here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant