Extend spec with ability to send data from client to server. #250
Conversation
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
Signed-off-by: Hans-Joachim Krauch <achim.krauch@gmail.com>
docs/spec.md
Outdated
|
|
||
| - `op`: string `"advertise"` | ||
| - `channels`: array of: | ||
| - `id`: number chosen by the client. The client may not reuse ids across multiple advertised channels. |
There was a problem hiding this comment.
We said this about channelId for server advertise:
The server may reuse ids when channels disappear and reappear, but only if the channel keeps the exact same topic, encoding, schemaName, and schema. Clients will use this unique id to cache schema info and deserialization routines.
Should we do something like this here too? Currently it doesn't sound like the same id is allowed to be reused even if it's unadvertised in between? Keeping the restriction as you have it is fine too, but might want to make it more clear that even unadvertising does not allow you to use the same id again.
There was a problem hiding this comment.
We can allow the client to reuse channel IDs if they have been unadvertised in between. I see no problem in that as the server will use the channel ID only to lookup published channels of the client
docs/spec.md
Outdated
|
|
||
| - `op`: string `"publish"` | ||
| - `channelId`: number. Channel ID corresponding to previous [Client Advertise](#client-advertise) | ||
| - `data`: object. JSON object |
There was a problem hiding this comment.
I think this should be a binary message for protocol simplicity. For this one example encoding of JSON a non-binary message works, but most encodings will require binary. It simplifies the server code if we don't need to handle client-published messages with two separate code paths.
There was a problem hiding this comment.
For this one example encoding of JSON
As currently written in the PR, JSON is the only allowed encoding. The thinking was that it would add a lot of complexity to ask clients to learn how to encode messages in multiple encodings, and somehow discover which encodings the server would support. How would you recommend we address these issues?
There was a problem hiding this comment.
add a lot of complexity to ask clients to learn how to encode messages in multiple encodings
I'm not suggesting clients should support multiple encodings. If a ws-protocol client is doing bag playback of a ROS 1 bag, it makes sense to advertise ros1-encoded topics and publish unprocessed message payloads rather than requiring the client to decode, transform to JSON, then publish. In this example, the client would only support publishing ros1-encoding and if the server did not support that encoding it would not be able to use the publish capability.
This does require the server to advertise which encodings it supports for client publishing. I'm not sure where that slots into the protocol best.
Public-Facing Changes
Extend the spec with ability to send data from client to server.
Description
This PR adds 3 new operations to the spec to allow sending data from the client to the server. Once we have agreed on the spec, I will update the example server implementation accordingly.
Partially fixes #38
Partially supersedes #158