Add messages and protocol for Presentation API

[pthatcherg]

No description provided.

[tidoust]

Great! See inline comments for typos.

I'd prefer to use the same conformance classes in this document as in the Presentation API, unless there's a good reason not to.

It would also be good to link things together properly, e.g. to add proper links to relevant sections in the Presentation API, and to link back terms to definitions, but that's typically something that can be done later on (and which I'd be happy to do)

[pthatcherg]

I updated the terminology to use what I noticed was already in the "terminology" section at the start of the doc, which defines "controller" and "receiver" according to the terms in the API docs. So, I just used those shorter names everywhere.

I also added proper links to the API docs.

But I haven't yet learned how to do internal links in a markdown/bikeshed file (use HTML tags?). If you want to explain to me how to do that, I can add some internal links. Of course, if you're willing to do that later, I'd be happy to let you do it :).

[tidoust]

But I haven't yet learned how to do internal links in a markdown/bikeshed file (use HTML tags?). If you want to explain to me how to do that, I can add some internal links. Of course, if you're willing to do that later, I'd be happy to let you do it :).

Relevant documentation is in the Definitions and Autolinking sections of Bikeshed's documentation. Essentially, you'll need to use <dfn> tags to define terms, and use [=toto=] to reference them from other parts of the spec. That should be enough in most cases.

Bikeshed does not know anything about CDDL, so it won't be able to auto-dfn relevant terms in CDDL definitions. It won't be able to do syntax highlighting on CDDL blocks either, actually (CDDL does not seem to be supported by Pygments, used under the hoods).

Again, I'm happy to handle these editorial updates afterwards, so that you can focus on the important bits, meaning the actual definition of the protocol and mapping!

[pthatcherg]

OK, I split into 2 sections.

I'll take you up on your offer to add internal links. You can do so later, or you could make changes directly to my PR.

[btolsch]

I've been wondering if, for both close and terminate, we should exclusively use an event. Is there any reason for the controller to wait for a close-response/termination-response? Is there anything it should do with an error in one of those responses?

[pthatcherg]

The receiver could refuse to terminate for some reason (maybe it decides the controller isn't allowed to terminate that presentation). The controller can't assume it happened until it receives the response. The presentation API doesn't allow for a close() or terminate() method to fail asynchronously, though. So if we wanted to allow the JS to do something, like indicate to the user that close/terminate failed, it wouldn't have a way of knowing.

[mfoltzgoogle]

Yeah. close/terminate always creates a state change on the controller side. Since the controller can't assume it can communicate with the presentation (or the device it's on) after these are called, there's no way to communicate the outcome back to the JS.

[pthatcherg]

Couldn't we change the close() method to return a Promise to resolve when the close is acked?

[tidoust]

I'll take you up on your offer to add internal links. You can do so later, or you could make changes directly to my PR.

Deal! :) I propose to do it later, typically when pending PRs have been merged.

[chrisn]

A minor note, I wanted to read the changes as rendered HTML, but converting the bikeshed to HTML produced a few errors relating to indentation. I'd be happy to fix these up, but don't want to introduce whitespace only changes. LMK if you'd like me to go ahead.

[pthatcherg]

Sorry, I didn't bother to run bikeshed yet. I probably should. It would great if you wanted to fix that. Please, go ahead!

[mfoltzgoogle]

Overall looks good - minor comments / suggested edits

[mfoltzgoogle]

Nit: is an empty list valid here? Similar question for presentation-url-availability-event

[pthatcherg]

Sure, but the response would just be an empty list.

For the event, it would tell you nothing is available any more.

[mfoltzgoogle]

Why would a controller ever want to send an empty list?

Is the empty list behavior for the event documented?

[pthatcherg]

It wouldn't want to send an empty list. There's no reason. The behavior would have to be to return a response with an empty list.

I just added a "must not be empty" to make it clear this must never happen.

[pthatcherg]

This conversation/review has gotten complex enough that I'm having trouble tracking all of the things going on.

I think that we're close enough to being in a good state that we should merge the PR and then resolve anything unresolved with issues and or separate PRs.

What do you think?

[mfoltzgoogle]

LGTM to merge. Followup items can be handled in other PRs if you would like.