-
Notifications
You must be signed in to change notification settings - Fork 63
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
Feedback on latest Editor's Draft #1324
Comments
Feedback part 2 (sections 6-11) below... Note: Apologies, this feedback ended up much longer than I had expected and now this issue is very long. Most of the major areas of feedback already have their own issues associated with them but I'm happy to break out other issues as requested. I haven't created separate issues for all the typos and grammatical errors I spotted since there are so many (you can find them all by searching for "->"), but hopefully they can all be fixed in one go (I can create a PR if I have time, but I may not have time before the end of the year). Obviously all of this feedback doesn't need to block publication of the next Working Draft and I am happy to re-purpose some of it as feedback on the published Working Draft later if preferred by the Editors, I just figured you'd rather receive it sooner rather than later. 6.3.1 Thing Root Object
As in 5.3.1, care needs to be taken here with backwards compatibility. This assertion is probably OK because it doesn't require that
However, the changes I proposed for section 5.3.1 would be needed in order for this to be valid. 6.3.2 Human-Readable MetadataIf this section contained a RECOMMENDATION that both the Thing Root Object and all properties, actions and events should include at least a 6.3.8 links"In the following examples are provide that uses different link relation values." (grammatically incorrect) -> "The examples provided below demonstrate the use of different link relation types." 6.3.9.3 responseAs I suggested under section 5.3.4.3, I propose that the 6.3.9.5 Top level forms
As with section 5.3.1, there is no information here about what the payloads of 6.3.10 Data Schemas
Presumably this should also now mention the
6.5 Validation
Reminder: The Editor's Note needs resolving before publication. 7.1 Semantic Annotations
7.1.3 Example III: Gelocation Annotations
Reminder: Issue 1 needs resolving before publication.
7.2 Adding Protocol Bindings
8.2 Data Schemas
A lot of the assertions in this section use the phrase "A Thing acting as a Consumer" where I think they should just say "A Consumer". This text dates back to 1.0 but I'm not sure why it was worded this way because it reads to me like an entity which is both a Consumer and a Producer, not just a Consumer. This language could potentially be simplified by formally defining the term "Producer" as I suggested under section 3, and referring to the two roles as "Producer" and "Consumer" rather than "Thing" and "Thing acting as a Consumer". 8.3 Protocol Bindings
I suggest adding text here to explain that a concrete protocol binding can now also be defined in a Profile. It may be helpful to distinguish between:
It's not clear what "Protocol binding document" refers to in this note (see #1326). 8.3.1 Protocol Binding based on HTTPSee #1259 for discussion regarding whether this section should be removed, or moved to another specification. 8.3.2 Other Protocol BindingsAs with section 8.3 it might be worth mentioning Profiles here. 10. Thing ModelI'm still very unclear on the intended use cases of the Thing Model feature, how Thing Models are intended to be used by Consumers and why it needs to be part of the Thing Description specification (vs. a templating engine implemented as a software helper library). The primary use case of defining classes of Things of which a Thing is an instance can already be achieved using semantic annotations which refer to capability schemas via a context extension, as we have been doing in WebThings for a number of years. I'm interested to see what take-up this feature gets. Perhaps it is a better approach and we should migrate the WebThings capability schema repository to using Thing Models via links rather than a context extension via semantic annotations, but currently I'm struggling to understand the rationale for its inclusion in the Thing Description specification. See #1078 for further discussion.
10.2 Thing Model Declaration
10.3 Modelling Tools
10.3.1 Versioning
10.3.2 Extension and Import
10.3.3 Composition
10.3.4 Required
10.3.5 Placeholder
10.4 Derivation to Thing Description Instances
11 IANA ConsiderationsIt has been suggested that additional file extensions should be registered with IANA for version 1.1, see #1214. |
@benfrancis thank you very much for this detailed review and feedback. I will start to create PRs to reflect your findings. |
I think, this is not needed. It is only highlighted here that properties have a special context since it is based on the terms of the data schema class and property affordance class.
This coming from the Security TF. @mmccool would be cool to have more background here.
The default value table only works for |
reported in comment w3c#1324 (comment) by "->"
FYI, I created a PR that covers all "->" issues, see #1335 |
Thanks @danielpeintner! @sebastiankb wrote:
Unfortunately In the meantime we will come up with a workaround for implementations in WebThings, see WebThingsIO/gateway#2882 (comment).
Actually now I think about it it isn't clear what the data schema of a In the example in w3c/wot-profile#91 I provide a separate data schema for the In the Core Profile the |
Call of 20.04:
|
Thanks for all of the hard work in addressing many of the points I raised.
Nit: I see that it's written in full now, but it should really be "Internet of Things", not "Internet of Thing".
Given this hasn't been resolved I think we'll have to work around it by either using Links for WebSocket endpoints or duplicating the same WebSocket endpoint in every interaction affordance.
I still can't find a definition for this in WoT Security or WoT Binding. Maybe just remove this row from the table?
I don't think this has been fixed, but it's a minor editorial point.
This still hasn't been addressed. From the discussion in #1341 it appears to have been postponed until 2.0. Please note that as far as I can tell this means that there's currently no way to describe the actions protocol binding from the HTTP Baseline Profile declaratively in a 1.1 Thing Description. I personally think it's OK for the concrete protocol bindings in profiles to go beyond what's possible to describe with declarative protocol bindings using protocol binding templates, but I know some people hold the view that profiles should stick to a sub-set of the features that can be described in a Thing Description, not a superset.
I don't think this was addressed but it was just a nice to have, not a blocker.
This has not been addressed. I've added a comment to #1469 to continue this discussion.
I don't think this was addressed but it's a minor point.
Still to do.
Still to do.
Not addressed, but fairly minor.
Not addressed.
The term "Producer" is now defined in WoT Architecture, but no changes have been made here.
This wasn't addressed, which means that the Thing Description specification currently gives the impression that protocol bindings can only be serialised as Forms using protocol binding templates. This isn't strictly true as profiles can now also define protocol bindings.
Not resolved, but being discussed in #1326.
Not resolved, but being discussed in #1259.
Not resolved, but being discussed in #1214. In conclusion, putting aside fairly minor proposed editorial changes, the main remaining issues are:
Whilst these are significant missing features, I think they can mostly be worked around with either very verbose Thing Descriptions or Profiles which go beyond what is possible in a Thing Description. It would be great to see some of the remaining editorial suggestions addressed, but otherwise I think it's safe to close this issue and continue discussion around the more major points in other issues. |
Following the call for review of the latest Editor's Draft of WoT Thing Description 1.1 with a view to CR transition, I've followed up on all the points I raised in this review to see whether they've been addressed. I think I've now filed separate issues for all of the remaining unresolved points, so I think this issue can be safely closed. Full explanation below.
Filed #1675.
This has now been resolved, but it isn't referenced from the Thing Description specification. I suggest adding "Producer" to the list at the start of section 3. Terminology. Filed #1668.
Filed #1670.
This has come up again in w3c/wot-profile#259 and I still think this is a major problem for describing asynchronous actions. There is no way to define a separate data schema for an action output and the response to an invokeaction operation. I think this is sufficiently tracked by #1053 and #1665, but it's currently marked as being deferred until 2.0. 2.5 Complex Interactions is a major part of the current charter that I don't think we've delivered on in Thing Description 1.1. There are now (at risk)
#1469 was closed but this was not fully addressed. I've filed #1665 with a more detailed review.
Filed #1671.
Filed #1672.
Filed #1673.
Filed #1671.
Still not resolved, but #1326 is tagged with 1.1.
Deferred to 2.0.
Still not resolved, but #1214 is tagged with 1.1.
This is being tracked in #1070 and has been deferred until 2.0.
I've covered this in more detail in #1665. |
Following @sebastiankb's email regarding an internal review of the WoT Thing Description 1.1 specification with a view to publishing the next working draft, please find my feedback below.
This is part 1 of my feedback covering sections 1-5. Part 2 covering sections 6-11 to follow!
Abstract
@context
URI (see below). It would be good to have an explanation somewhere of what exactly is meant by backwards compatibility (see Definition of Backwards Compatibility #1243).3 Terminology
[x] I note that the WoT Architecture document has a definition of a "Consumer", but not a "Producer".
5 TD Information Model
5.3.1 Thing
securityDefinitions
is missing from the table (there's a description but the actual term is missing).The solution I proposed in #1243 (comment) was:
https://www.w3.org/2022/wot/td/v1.1
must be first in the array, since both URIs can't be the first.op
being a mandatory member ofForm
causes a problem for top level WebSocket endpoints of Things which provide operations other than those listed here. See Allow more operations in a top level form #1070 and op does not have with default tag anymore #1192 (comment) for further discussion. I propose either lifting this constraint, or making theop
member optional.queryallactions
operation be a map of arrays, whose members follow the data schema for aqueryaction
operation (see Is there a need forinvokeanyaction
/queryallactions
operations? #1200 (comment))? Is there an equivalent of this for events?5.3.1.3 PropertyAffordance
5.3.2 Data Schema Vocabulary Definitions
5.3.4 Hypermedia Controls Vocabulary Definitions
5.3.4.1 Link
manifest
link relation type used for? The description "Point to a software implementation of the TD" is unclear. See What is the intended use of the manifest link relation? #1076. I'd suggest changing the description to something like: "Point to the web app manifest of a web application which provides a user interface with which a user can interact with the Thing." Note that this link relation type is defined in the Web App Manifest specification which could be provided as a reference.proxy-to
link relation type for? I can't find a definition in the (non-normative) WoT Security and WoT Binding Template documents cited as being the source.How do theEdit: I see that these link relation types are used in example Thing Models later in the specification.collection
anditem
link relation types relate to the Directory Service API in the WoT Discovery specification? I can imagine that acollection
link could point to the TDD of a directory, but what would theitem
link point to? Or is this a completely separate mechanism which can be combined with a directory? If so, would a directory list all of the "item" TDs directly, or only the "collection" TD?5.3.4.2 Form
op
member also mean "mandatory" if no default is provided? In particular I'm thinking about top level forms for which no defaults are defined. (See comments above about the issues withop
being mandatory for Form, discussed further in Allow more operations in a top level form #1070).I welcome the table with definitions of operation types!
5.3.4.3 ExpectedResponse
ExpectedResponse
should have aschema
member of typeDataSchema
, likeAdditionalExpectedResponse
does. See Consider adding DataSchema to response and additionalResponses #1053.The text was updated successfully, but these errors were encountered: