spec: Allow specifying service schema names & encodings #563
Conversation
|
Nice, LGTM |
docs/spec.md
Outdated
| @@ -206,8 +206,14 @@ Informs the client about available services. Only supported if the server declar | |||
| - `id`: number. The server may reuse ids when services disappear and reappear, but only if the services keeps the exact same name, type, and schema. Clients will use this unique id to cache schema info and deserialization routines. | |||
| - `name`: string | |||
| - `type`: string | |||
| - `requestSchema`: string | |||
| - `responseSchema`: string | |||
| - `requestSchema`: string (schema data only) or object with the fields below. If given as string, schema name and encoding will be derived from `type` (by appending `_Request`) and the previously advertised [supportedEncodings](#server-info). | |||
There was a problem hiding this comment.
I'm confused about what it means to derive the schema encoding from supportedEncodings, because I thought those were message encodings, not schema encodings?
There was a problem hiding this comment.
I guess we also need a response message encoding in addition to response schema encoding?
There was a problem hiding this comment.
I'm confused about what it means to derive the schema encoding from supportedEncodings, because I thought those were message encodings, not schema encodings?
As serviceCallEncoding (message encoding) we pick one of the supported encodings from supportedEncodings and then derive the schema encoding from that:
It's quite some magic. Maybe we just add messageEncoding to the objects that I have added in this PR?
There was a problem hiding this comment.
Naming is hard... what do you think of:
request: {
encoding: string
schemaName: string
schemaEncoding: string
schema: string
}
or
requestEncoding: string
requestSchema: {
name: string
encoding: string
data: string
}
or something like that?
cc @defunctzombie for thoughts
There was a problem hiding this comment.
I am in favor of the 1st option
There was a problem hiding this comment.
Applied the changes. OK for you like that @jtbandes @jhurliman @defunctzombie ?
There was a problem hiding this comment.
Maybe it is documented elsewhere or obvious to others but I have no idea what any of the fields in the services array are suppose to be. id has some more detailed documentation but all the other fields only have a plain type (string, object). Reading request I've no idea what I am suppose to think about this field, how do I populate it, what is it for? The same is true for encoding. How is that different than schemaEncoding? What would I typically put in these? How does it relate to the schema field?
All of those questions I cannot answer reading the spec. I see an example which helps but whether I would be able to turn that into an equivalent for other systems I am not sure.
Related: what is "encoding"? Why is it important to note ros1 in the encoding? We already have a schemaEncoding of ros1msg. These are things that more descriptions on the fields might have made obvious but since there are not any I don't know what purpose this field serves that others have not already served.
|
Awesome! Thank you for adding the descriptions. This is all clear to me now. |
defunctzombie
deleted the
achim/fg-5280-service-calls-make-ros-assumptions-with-_request-and
branch
…7102) **User-Facing Changes** Remove ROS assumptions when using services with foxglove websocket **Description** Updates the websocket player to comply with the latest `ws-protocol` spec changes made in foxglove/ws-protocol#563. This allows to remove some ROS specific assumptions which makes it easier to advertise services from non-ROS systems. Fixes #6970 Resolves FG-5280
Public-Facing Changes
spec: Allow specifying service schema names & encodings
Description
Allows for customization of service schema names and schema encodings.
Partially fixes https://github.com/foxglove/studio/issues/6970
Partially resolves FG-5280