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.

Sign up for GitHub

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

Jump to bottom

spec: Allow specifying service schema names & encodings #563

Merged
merged 3 commits into from
Nov 4, 2023
Merged

spec: Allow specifying service schema names & encodings #563

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9ee291a
spec: Allow specifying service schema names & encodings
achim-k Oct 25, 2023
b24e5ea
comments
achim-k Oct 26, 2023
d099476
improve field documentation
achim-k Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions docs/spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused about what it means to derive the schema encoding from supportedEncodings, because I thought those were message encodings, not schema encodings?

Copy link
Member

Choose a reason for hiding this comment

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

I guess we also need a response message encoding in addition to response schema encoding?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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:

https://github.com/foxglove/studio/blob/2c854ffb9c5a3676fb22818206d581bb1c08cf0f/packages/studio-base/src/players/FoxgloveWebSocketPlayer/index.ts#L606-L615

It's quite some magic. Maybe we just add messageEncoding to the objects that I have added in this PR?

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am in favor of the 1st option

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Applied the changes. OK for you like that @jtbandes @jhurliman @defunctzombie ?

- `name`: string
- `encoding`: string
- `data`: string
- `responseSchema`: 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 `_Response`) and the previously advertised [supportedEncodings](#server-info).
- `name`: string
- `encoding`: string
- `data`: string

#### Example

Expand All @@ -218,9 +224,16 @@ Informs the client about available services. Only supported if the server declar
{
"id": 1,
"name": "foo",
"type": "std_srvs/srv/Empty",
"type": "std_srvs/Empty",
"requestSchema": "",
"responseSchema": ""
},
{
"id": 2,
"name": "set_bool",
"type": "std_srvs/SetBool",
"requestSchema": { "name": "std_srvs/SetBool_Request", "encoding": "ros1msg", "data": "bool data" },
"responseSchema": { "name": "std_srvs/SetBool_Response", "encoding": "ros1msg", "data": "bool success\nstring message" }
}
]
}
Expand Down
Loading