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.

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

Change links to forms in API #2806

Closed
benfrancis opened this issue Mar 8, 2021 · 32 comments
Closed

Change links to forms in API #2806

benfrancis opened this issue Mar 8, 2021 · 32 comments
Labels
task w3c-compliance wot-thing-description W3C WoT Thing Description specification
Milestone

Comments

@benfrancis
Copy link
Member

benfrancis commented Mar 8, 2021

Blocks: #2802

The gateway's API currently exposes links for each interaction affordance (property, action and event) which provide API endpoints for getting/setting properties, invoking actions etc. E.g.

  "properties": {
    "on": {
      "links": [{"href": "/things/switch/properties/on"}]
    }

The W3C WoT Thing Description specification specifies the use of forms rather than links for many of these use cases. E.g.

  "properties": {
    "on": {
      "forms": [{"href": "/things/switch/properties/on"}]
    }

I still do not like the forms approach of the W3C spec (see w3c/wot-thing-description#85 and w3c/wot-thing-description#88 for some previous discussions), but given for simple use cases the two are functionally equivalent, let's pick our battles and make this conformant with the W3C spec.

This includes:

  1. Changing instances of links to forms where appropriate
  2. Changing mediaType to contentType where used
  3. Removing rel (e.g. rel: "property") from links which become forms
  4. Adding an op member of a form where needed (this is not needed in all instances due to default value definitions)
  5. Updating the gateway's front end to expect this

This excludes:

  1. Changing the payloads of API requests/responses to match data schemas in the Thing Description (e.g. removing object wrappers), or changing data schemas in Thing Descriptions to exactly match payloads (e.g. adding object wrappers or response metadata). This will be handled separately.
  2. Representing action queues and event logs in forms, which is not currently supported in the W3C spec. For the purposes of this issue we should just provide a form with a href endpoint for these interaction affordances and continue to follow the conventions of the Web Thing API.
  3. The alternate link relation in the top level links member used to link to an HTML UI should stay as a link:
"links": [
  {
    "rel": "alternate",
    "mediaType": "text/html",
    "href": "/things/lamp"
  }
]

Open questions:

  1. Should a top level link to a WebSocket API endpoint be a link or a form? For example:
"links": [
  {
    "rel": "alternate",
    "href": "wss://mywebthingserver.com/things/lamp"
  }
]

could become:

"forms": [
  {
    "op": ["writeproperty", "invokeaction", "subscribeevent"],
    "href": "wss://mywebthingserver.com/things/lamp",
    "subprotocol": "webthing"
  }
]

but this is not technically W3C compliant as the spec says:

When the forms Array of a Thing instance contains Form instances, the string values assigned to the name op, either directly or within an Array, MUST be one of the following operation types: readallproperties, writeallproperties, readmultipleproperties, or writemultipleproperties.

and its not clear how to represent WebSocket API message types which are pushed from the client to the server such as propertyStatus, actionStatus and event.

  1. What should we do with top level links to properties, actions and events endpoints?
"links": [
  {
    "rel": "properties",
    "href": "/things/lamp/properties"
  },
  {
    "rel": "actions",
    "href": "/things/lamp/actions"
  },
  {
    "rel": "events",
    "href": "/things/lamp/events"
  }
]

The top level properties endpoint could become:

"forms": [
  {
    "op": "readallproperties",
    "href": "/things/lamp/properties"
  }
]

but there are currently no values of op which map well onto "get all recent events" or "invoke an action whose name is provided in the request payload". If we try to remove the rel from the actions and events endpoints, it will therefore no longer be possible to distinguish what they are for.

@benfrancis benfrancis added this to the 2.0 milestone Mar 8, 2021
relu91 added a commit to relu91/gateway that referenced this issue Mar 16, 2021
@relu91
Copy link
Collaborator

relu91 commented Mar 17, 2021

Implementation proposals for the current open points:

  1. Replicate the WebSocket endpoint in each affordance. This requires some changes on how forms (ex. links) are treated in the code base, but I think it's the most consistent way forward.
  2. To have a consistent pattern, I think it's best to specify these additional affordances on each WebThing:
    • properties (property affordance)
    • inovkeAction (action affordance)
    • events (property affordance)
      Possibly we should define also semantic tagging so that we can identify those easily.

Note the proposed solutions have as assumption that we cannot modify the current spec,

relu91 added a commit to relu91/gateway that referenced this issue Mar 17, 2021
relu91 added a commit to relu91/gateway that referenced this issue Mar 17, 2021
@benfrancis
Copy link
Member Author

benfrancis commented Mar 17, 2021

Note the proposed solutions have as assumption that we cannot modify the current spec,

I don't think we should assume that. If there are still things that can't be expressed in a W3C Thing Description then we need to flag that with the W3C, and in some cases it may be reasonable to add proprietary extensions for unsupported features, remove some features that can't be supported, or keep some features in the API but exclude them from the Thing Description if there's no way to describe them.

Replicate the WebSocket endpoint in each affordance. This requires some changes on how forms (ex. links) are treated in the code base, but I think it's the most consistent way forward.

I'd really rather not do this as it creates a lot of redundancy and may result in clients opening a WebSocket per interaction affordance which is highly impractical (see #34). (If anything we'd like to go in the other direction and offer a single WebSocket endpoint for the gateway shared by multiple devices).

I would prefer one the three possible options, in order:

  1. Change the W3C specification to allow any op type in a top level form. I have filed Allow more operations in a top level form w3c/wot-thing-description#1070 for this.
  2. Keep the WebSocket endpoint as a link (which has no such restrictions) until such a time as the above is supported
  3. Use a form with either the op: ["readallproperties", "writeallproperties", "readmultipleproperties", "writemultipleproperties"] or no op member at all (meaning clients have to rely on the URI scheme and subprotocol to find the WebSocket endpoint). The former is not really accurate right now but might be in the future. The W3C specification doesn't fully define how to create protocol bindings for WebSockets so we're really in uncharted territory here anyway.

To have a consistent pattern, I think it's best to specify these additional affordances on each WebThing:
properties (property affordance)
inovkeAction (action affordance)
events (property affordance)
Possibly we should define also semantic tagging so that we can identify those easily.

Sorry, but this seems like a horrible hack.

For properties I think it's already possible to define a top level form:

"forms": [
  {
    "op": "readallproperties",
    "href": "/things/lamp/properties"
  }
]

If we can't think of a way to implement the actions and events resources in a W3C compliant way, or a proposal to the W3C for how to make this possible in the specification, then we may just need to drop those features. They are really just convenience features which duplicate the individual action and event endpoints. This may be discussed further in #2807.

@relu91
Copy link
Collaborator

relu91 commented Mar 18, 2021

Note the proposed solutions have as assumption that we cannot modify the current spec,

I don't think we should assume that. If there are still things that can't be expressed in a W3C Thing Description then we need to flag that with the W3C, and in some cases it may be reasonable to add proprietary extensions for unsupported features, remove some features that can't be supported, or keep some features in the API but exclude them from the Thing Description if there's no way to describe them.

Thank you for sharing the intended direction. I was not aware of how you would like to handle this issue so I had to make a hypothesis. I thought that chasing a moving target would slow the convergence process and WebThings would follow the TD 1.0. It might take some time before a feature would be included in TD 1.1 and also, TD 1.1 must be backward compatible. As you said we could use property extensions while waiting for the standardization to followup but this would possibly create fragmentation (what if W3C never includes the extensions? are those extensions functional to the communication between a consumer and a WebThing? if so not WebThings clients would not be interoperable). Having said that I am not against following this path, but we should take some precautions (that I think you are well aware of).

About the proposal, as the hypothesis does not hold anymore, I think we could totally discard them. They were designed to strictly follow TD 1.0 so there's no point continuing the discussion on them.

Instead, about your proposals, I agree with the suggested order of possible solutions although I might have some comments/questions about the first one (let's discuss them in the issue you opened).

Sorry, but this seems like a horrible hack.

It was 😄, but again I assumed that we would NEED those features and we cannot modify the spec. So it would have been odd to use readallproperties AND define affordances for the other two. Now it's all different and if it possible to drop them or try to describe this use case in the TD spec.

@benfrancis
Copy link
Member Author

benfrancis commented Mar 18, 2021

I was not aware of how you would like to handle this issue so I had to make a hypothesis. I thought that chasing a moving target would slow the convergence process and WebThings would follow the TD 1.0.

Yes, sorry. Your solutions were completely reasonable within those constraints. I'm just conscious that whatever design decisions we make for the gateway's API will be probably be replicated in around 17 other WebThings implementations, so it needs to be something we're happy to live with long term, not just forcing W3C compliance for the sake of compliance. (No pressure 😉)

It might take some time before a feature would be included in TD 1.1 and also, TD 1.1 must be backward compatible.

If we think we can get better solutions into 1.1 then I think it's reasonable to pursue that rather than limit ourselves to what's possible with 1.0. Where there are conflicts I think we'll need to take it on a case by case basis as to whether we try to improve the W3C spec, drop a feature from WebThings, or implement it in a different way.

I assumed that we would NEED those features and we cannot modify the spec. So it would have been odd to use readallproperties AND define affordances for the other two. Now it's all different and if it possible to drop them or try to describe this use case in the TD spec.

We should try to find a solution to propose for TD 1.1, but if we can't then we may have to remove the actions resource and change the gateway front end to call individual action resources instead which shouldn't be too much of an issue.

Currently the gateway front end fetches the events resource to get a log of all events, that could be changed to fetch individual event resources but that would require merging multiple lists and would still require a way to describe an event log. Let's discuss that in #2807.

@relu91
Copy link
Collaborator

relu91 commented Mar 20, 2021

PRs in flight. Notice that the changes on the WebThings gateway depend on these two PRs, we have to wait until they are merged and published.

@benfrancis
Copy link
Member Author

@relu91 Thanks for sending these first PRs, very exciting!

This code has changed a lot since I last worked on it and after I left Mozilla Mike replaced nanomsg with a new WebSocket IPC mechanism, which we'd wanted to do for ages but I'm not familiar with. I was never that familiar with the implementation of the add-ons system in the first place as I mainly worked on the web layer.

I have to admit I wasn't expecting that changes would be necessary to the adapter IPC layer in order to change the format of Thing Descriptions exposed by the gateway's API, I had naively imagined the changes would be limited to the models and controllers of the web application. I clearly need to do some homework (including familiarising myself with TypeScript syntax!).

In the meantime I wonder whether @tim-hellhake can help here since he is the new module owner of the add-ons system and should probably review this code.

Now that I know the add-on IPC needs to change I'd like to better understand how this will impact add-ons. Will this break backwards compatibility for add-ons and mean add-on developers need to update their add-ons to use the new IPC? Or is this all handled within the node and python bindings provided by the gateway? I ask because I told people that add-ons wouldn't need to change and I'm concerned that I may have been mistaken.

Also, with regards to branching I'm thinking of creating a v1.x branch in the gateway repo so that we can continue to release 1.x releases without worrying about breaking changes on master. @tim-hellhake Do you have any thoughts on how to handle this in the gateway-addon-node, gateway-addon-python and gateway-addon-ipc-schema repos? How can we make sure that the right branch is pulled into 1.x and master (2.x) builds?

@relu91
Copy link
Collaborator

relu91 commented Mar 22, 2021

I have to admit I wasn't expecting that changes would be necessary to the adapter IPC layer in order to change the format of Thing Descriptions exposed by the gateway's API, I had naively imagined the changes would be limited to the models and controllers of the web application. I clearly need to do some homework (including familiarising myself with TypeScript syntax!).

I see. I am new to the code base so I might as well touched the wrong points. My process was to start from the model folder. There the Thing class imports PropertySchema, ActionSchema, and EventSchema types:

import {
Action as ActionSchema,
Event as EventSchema,
Property as PropertySchema,
Link,

Those are used to model the final thing description instance and are later "serialized" in a ThingDescription here. All of these types are defined as JSON schema definitions in gateway-addon-ipc-schema and later built to TS types in gateway-addon-node.

Al alternative approach (that I evaluated) would be to detach the Thing class from the schema definitions, but it would mean to have duplicated definitions. Not ideal in my opinion.

I saw that the list of the addons is quite long and it is not easy to predict the impact of this change. We can't even rely on TS-type checks. I picked addon at random and, as described in the gateway-addon, it does not declare it as a dependency. However, it redefines some of its types... I think in practice this describes the worst of scenarios:
tsc will pass hiding the fact that now Properties etc. will not have links but forms.

The same I think applies to python-based addons.
So the things do not look good, but I am not an expert here.

@tim-hellhake
Copy link
Member

I have to admit I wasn't expecting that changes would be necessary to the adapter IPC layer in order to change the format of Thing Descriptions exposed by the gateway's API

It depends.
There are 3 strategies:

  1. Don't change the IPC layer at all and convert all objects from the old spec into W3C compatible objects on-the-fly in the gateway
  2. Change the internals of the IPC layer to the W3C spec but keep the user-interface compatible
  3. Break compatibility with existing addons

I can't say for sure which is the best option because I don't know all the necessary changes.
I would prefer 3. because it reduces the overall complexity.
If it's only about renaming a few things, then 1. and 2. would also be an option.

  1. and 2. are also kind of awkward because you call setLinks in the addon but get forms from the API.
  2. has the advantage that you only need to change the gateway but you are limited to the old spec.
    If there are new features or structural changes you can't transport them over the IPC layer.
  3. is kind of hard because the adapter can access every internal field.
    You can only add new things which makes the code more complex and harder to maintain.

Imagine the old spec is your transport protocol.
The question is can it represent all the things you do with the W3C spec?
If not then 1. is out of the question.
Can you represent all features of the old spec in the W3C?
If yes then 2. might be an option.

Will this break backwards compatibility for add-ons and mean add-on developers need to update their add-ons to use the new IPC? Or is this all handled within the node and python bindings provided by the gateway?

The current PR changes the API so the addons will definitely break.
You will need to call setForms instead of setLink for example.

Also, with regards to branching I'm thinking of creating a v1.x branch in the gateway repo so that we can continue to release 1.x releases without worrying about breaking changes on master. @tim-hellhake Do you have any thoughts on how to handle this in the gateway-addon-node, gateway-addon-python and gateway-addon-ipc-schema repos?

I think we should pursue the same strategy in all addon repos.
This way it's consistent and you can directly see what belongs together.

How can we make sure that the right branch is pulled into 1.x and master (2.x) builds?

The CI pipeline will publish the correct version of the gateway-addon-node.
We just need to select the correct version in the package.json of the gateway.
I need to check if it's the same for the gateway-addon-python.
The gateway-addon-ipc-schema is pulled in by the gateway-addon-node, so this should be fine too.

@tim-hellhake
Copy link
Member

I see. I am new to the code base so I might as well touched the wrong points. My process was to start from the model folder. There the Thing class imports PropertySchema, ActionSchema, and EventSchema types:

You are right.
They are tightly coupled.

We can't even rely on TS-type checks. I picked addon at random and, as described in the gateway-addon, it does not declare it as a dependency. However, it redefines some of its types... I think in practice this describes the worst of scenarios:
tsc will pass hiding the fact that now Properties etc. will not have links but forms.

The gateway-addon-node was written in JS before.
The adapter you saw was not yet migrated to the new types.
You can check the wled-adapter as example.
It uses the gateway-addon as dev dependency.
In practice, you can never be absolutely sure what version you get because the gateway-addon-node is shipped with the gateway.
Ideally, the addon would require the gateway-addon-node but then you would need to cross-compile every addon because of the websocket package.

As it will be a new major release I would rather break things and do it right instead of maintaining technical debts.

@benfrancis
Copy link
Member Author

benfrancis commented Mar 23, 2021

@tim-hellhake wrote:

There are 3 strategies:
1. Don't change the IPC layer at all and convert all objects from the old spec into W3C compatible objects on-the-fly in the gateway
2. Change the internals of the IPC layer to the W3C spec but keep the user-interface compatible
3. Break compatibility with existing addons

This makes sense, thanks for explaining.

I share your instinct to choose option 3 to have a cleaner solution which can support all current and future W3C WoT Thing Description features. However, this may have significant implications because it could break many of the 100+ existing add-ons.

Changing the API exposed by the gateway was supposed to be the easy part, as I thought that would only impact the gateway's own web interface. And whilst it sounds like that is technically possible, it isn't a very future proof solution because the same schemas and terminology are used all the way down the stack and it would make sense to change those to the W3C compliant versions.

We already said that changing the API consumed by the thing-url-adapter would require further discussion as it means breaking compatibility with 16 web thing libraries. Therefore a change that could impact up to 100 add-ons definitely requires further discussion.

If we think that option 1 is a reasonable first step, then I think we could push ahead with that now. It could definitely fix this current issue of simply renaming "links" to "forms", but once we start more significant changes to the API and potentially adding support for W3C features which aren't in the Mozilla spec I suspect that is going to unravel.

If we want to go with option 3 then we need to hold a discussion with the community. @tim-hellhake @relu91 Are you both available to join the public monthly meeting on Thursday and explain these three options and their implications?

The decision on which approach to choose may also depend on the timing of when we release these changes. If it coincides with a replacement base OS which ends up requiring re-architecting the add-ons system around containers of some kind, then changing the API surface wouldn't be such a big deal as we may break compatibility anyway. But I wouldn't want to block releasing the W3C compliance features on a new base OS and add-ons architecture and create a gigantic 2.0 release plan that takes years to finish. We need to figure out an incremental release plan which allow us to continue with regular releases.

@madb1lly
Copy link

Hi all,

A thought on migrating gateways to this compatibility-breaking 2.x update:

If there is a common structure for branching as @tim-hellhake recommends then the 1.x gateway update code could check the installed add-ons to see if each has a new 2.x version which is available and inform the gateway user before updating, e.g. "You have 3 add-ons (list them) which will be incompatible with the new update, do you want to update?"

If "yes" then the gateway would then update the gateway and all add-ons for which a compatible 2.x version exists.

The update code would also ensure that add-ons are not updated to v2.x compatible version whilst the gateway is still 1.x.

Would that fly? 🤔

@benfrancis
Copy link
Member Author

@madb1lly I think we may want to do something along those lines regardless, so thank you for the suggestion. I know NextCloud does this kind of thing. When upgrading between versions it tells you which "apps" are compatible with the new version of the core and which will need to be disabled.

@tim-hellhake @relu91 I just talked over this issue with a mutual friend and we concluded that the impact of option 3 may not actually be that bad.

The only two instances we're aware of where add-ons manipulate the links array are:

  1. The thing-url-adapter
  2. To provide an alternate link relation pointing to an HTML UI

We can update the thing-url-adapter ourselves and the UI links will stay as links so don't need migrating.

With regards to other changes we want to make for W3C compliance:

  1. @context can just be set in the add-on libraries and shouldn't break anything (@tim-hellhake you may need to update the add-ons that you wrote in Rust too, since they don't use the libraries)
  2. Object wrappers should be handled entirely at the gateway level
  3. Backwards compatibility for mediaType vs. type (and link vs. form) could be provided by the gateway

All in all the impact actually seems minimal.

What do you think?

@relu91
Copy link
Collaborator

relu91 commented Mar 24, 2021

If we want to go with option 3 then we need to hold a discussion with the community. @tim-hellhake @relu91 Are you both available to join the public monthly meeting on Thursday and explain these three options and their implications?

I'll be there 👍🏻

@tim-hellhake @relu91 I just talked over this issue with a mutual friend and we concluded that the impact of option 3 may not actually be that bad.
The only two instances we're aware of where add-ons manipulate the links array are:

  1. The thing-url-adapter
  2. To provide an alternate link relation pointing to an HTML UI

We can update the thing-url-adapter ourselves and the UI links will stay as links so don't need migrating.

Well given this information I think we could really handle option 3.

  1. Backwards compatibility for mediaType vs. type (and link vs. form) could be provided by the gateway

Do you mean that the gateway should still load and TD with links instead of forms? but still, expose TDs with forms? I am asking because in the current changes that I have done I did not assume this backward compatibility. Of course, I can update it.

@benfrancis
Copy link
Member Author

Do you mean that the gateway should still load and TD with links instead of forms? but still, expose TDs with forms? I am asking because in the current changes that I have done I did not assume this backward compatibility. Of course, I can update it.

With mediaType vs. type we could support both at the Add-on IPC layer (maybe with a deprecation warning in the console whenever mediaType is used, but convert mediaType to type in the Web Thing API layer. Would that work?

In theory we could do the same for links vs. forms but in practice it might be trickier because some links are going to stay as links.

I've also just noticed another difference we'd need to handle in #2810.

@relu91
Copy link
Collaborator

relu91 commented Mar 24, 2021

With mediaType vs. type we could support both at the Add-on IPC layer (maybe with a deprecation warning in the console whenever mediaType is used, but convert mediaType to type in the Web Thing API layer. Would that work?

yeah, the translation of mediaType should be straightforward. I am a little bit worried about consistency from the add-on point of view. I mean in the new version we enforce link to be formed and only deprecate mediaType; either we choose to deprecate both or translate both. I know that for mediaType is just really a renaming operation, but still a little bit confusing. is it?

@benfrancis
Copy link
Member Author

either we choose to deprecate both or translate both

The difference is we can't deprecate links, because we still need them in W3C compliant Thing Descriptions, e.g.

  {
    "rel": "alternate",
    "mediaType": "text/html",
    "href": "/things/lamp"
  }

Not all links need changing into forms.

mediaType on the other hand is not W3C compliant.

@relu91
Copy link
Collaborator

relu91 commented Mar 24, 2021

I see, I thought we could do a "scoped" deprecation only for links in affordances. Still, since we are already in breaking backward compatibility mode, I don't think that it would make so much difference to maintain mediaType. It would make sense if most of the producers of links with mediaType don't manipulate forms at all. Otherwise, they need to be updated... so why don't ask to rename mediaType? Anyhow, I don't have a strong take here 😄 .

@tim-hellhake
Copy link
Member

If we want to go with option 3 then we need to hold a discussion with the community. @tim-hellhake @relu91 Are you both available to join the public monthly meeting on Thursday and explain these three options and their implications?

I'll be there too.

We can update the thing-url-adapter ourselves and the UI links will stay as links so don't need migrating.

Excellent.

The only two instances we're aware of where add-ons manipulate the links array are:
The thing-url-adapter
To provide an alternate link relation pointing to an HTML UI
We can update the thing-url-adapter ourselves and the UI links will stay as links so don't need migrating.

Well given this information I think we could really handle option 3.

I thought we don't need to break compatibility because we just add forms rather than replacing links with it?

@relu91
Copy link
Collaborator

relu91 commented Mar 26, 2021

Ok just to recap in light of what we have said during the last online meeting.

@benfrancis @tim-hellhake, green light?

The thing I want to be careful about is the impact on add-on developers. It seems like only three add-ons (which we maintain) should be impacted by links vs. forms, so that may not be a big deal. I have no idea how many add-ons may be impacted by mediaType vs. type, but our ImageProperty and VideoProperty schemas both use that so it seems likely there are add-ons which could be impacted. (Does that mean they might manually manipulate links in properties too?). Without auditing the source code of all add-ons I'm not really sure how to tell.

Just a note, ImageProperty and VideoProperty are used at affordance level (at least as I saw in the gateway code), so in my gateway fork their using contentType rather than mediaType. The UI is updated to expect this. I think it's reasonable to follow this approach since they were used in links inside a property (now they are forms).

Anyhow, I think it's beneficial to discuss the details on a PR. I'll open a PR to the gateway in a draft mode so that we can have something more concrete.

@benfrancis
Copy link
Member Author

benfrancis commented Mar 26, 2021

Rename links to forms breaking backward compatibility even in the IPC layer

Yes, on the assumption that it will only effect three add-ons which we can fix ourselves. Once we have a PR we can test this assumption by trying some add-ons.

Deprecate mediaType in links (this concerns issue #2808)

Yes, hopefully by allowing both type and mediaType (with a deprecation warning) at the add-on IPC layer, and always exposing type at the Web Thing API layer.

Update the impacted add-ons in this interaction

Yes. We may also need to automatically translate all of these differences inside the thing-url-adapter until we change them on the consumer side of that adapter as well.

Just a note, ImageProperty and VideoProperty are used at affordance level (at least as I saw in the gateway code), so in my gateway fork their using contentType rather than mediaType. The UI is updated to expect this. I think it's reasonable to follow this approach since they were used in links inside a property (now they are forms).

contentType is correct in this instance as far as W3C compliance is concerned¹. The problem is that the schema currently says "mediaType", so if there are add-ons which are manually generating that metadata that could be an issue.

Anyhow, I think it's beneficial to discuss the details on a PR. I'll open a PR to the gateway in a draft mode so that we can have something more concrete.

That would be great, thank you.

  1. I don't know why the W3C spec is inconsistent here, type seems like a bad choice since the same name is used elsewhere in the spec to mean a primitive data type like "string".

@benfrancis
Copy link
Member Author

1. I don't know why the W3C spec is inconsistent here, `type` seems like a bad choice since the same name is used elsewhere in the spec to mean a primitive data type like "string".

It's probably to be consistent with link elements in HTML...

@kgiori
Copy link
Contributor

kgiori commented Mar 26, 2021

* Rename links to forms breaking backward compatibility even in the IPC layer

MicroBlocks has a WebThings library that I am pretty sure will need to be updated. For Wi-Fi devices such as ESP8266 and ESP32 and others, it produces a "native" web thing description (TD) per the current webthings.io/schemas.

Currently my simple web doorbell TD looks like this (as loaded via "curl http://DEVICE_IP_ADDRESS"):
{"title":"doorbell","@context":"https://webthings.io/schemas/","@type":["PushButton"],"links":[{"rel":"events","href":"/events"},{"rel":"properties","href":"/properties"}],"properties":{},"events":{"pressed":{"description":"MicroBlocks event","@type":"PressedEvent"}}}

And a simple "smart light" (LED on/off) looks like this:
{"title":"Hello LED","@context":"https://webthings.io/schemas/","@type":["Light"],"links":[{"rel":"events","href":"/events"},{"rel":"properties","href":"/properties"}],"properties":{"on":{"links":[{"href":"/properties/on"}],"title":"On","type":"boolean","@type":"OnOffProperty","readOnly":false}},"events":{}}

What should the JSON TD look like to be compatible with the W3C spec, which if I understand correctly, the gateway's updated thing-url-adapter will be looking for? If these TD's didn't change, would the WebThings Gateway of the future be able to discover and manage these devices or not? If not, I assume everyone who has developed webthings.io/schemas devices will need to abruptly update deployed devices, plus fix all examples and documentation to use the updated spec.

@benfrancis
Copy link
Member Author

@kgiori This issue is only about the Web Thing API exposed by the gateway (phase 1 of the W3C compliance plan), not the Thing Descriptions consumed by the thing-url-adapter, so web thing libraries won't need to be changed. The backwards incompatibility at the add-ons IPC layer we're talking about will be handled automatically by the thing-url-adapter, which we will update as necessary (along with the other two add-ons we believe will be effected). The Thing Descriptions exposed by native web things (like the ones created with the MicroBlocks web thing library) won't need to change.

Once we get to phases 2 & 3 of the W3C compliance plan (changing the thing-url-adapter and web thing libraries to use W3C compliant Thing Descriptions), we're looking at ways of supporting both the legacy Web Thing API and W3C compliant API simultaneously (e.g. with two separate adapters), so that web things have plenty of time to change over.

Don't Panic ;)

@relu91
Copy link
Collaborator

relu91 commented Mar 29, 2021

Sorry, I got caught up in a meeting this Friday and I didn't have time to polish and submit the PR. Now is on 😄 .

Once we get to phases 2 & 3 of the W3C compliance plan (changing the thing-url-adapter and web thing libraries to use W3C compliant Thing Descriptions), we're looking at ways of supporting both the legacy Web Thing API and W3C compliant API simultaneously (e.g. with two separate adapters), so that web things have plenty of time to change over.

+1 for the legacy adapter it will decrease the "stress" on the WebThingsIO framework libraries.

What should the JSON TD look like to be compatible with the W3C spec, which if I understand correctly, the gateway's updated thing-url-adapter will be looking for

With the current changes the two TDs will not change too much:

/* No change here we have still to understand how to translate root links
* to forms. 
*/
{
    "title": "doorbell",
    "@context": "https://webthings.io/schemas/",
    "@type": [
        "PushButton"
    ],
    "links": [
        {
            "rel": "events",
            "href": "/events"
        },
        {
            "rel": "properties",
            "href": "/properties"
        }
    ],
    "properties": {},
    "events": {
        "pressed": {
            "description": "MicroBlocks event",
            "@type": "PressedEvent"
            // <-- Here we need a form. Still an open issue as said in the comment above
        }
    }
}
{
    "title": "Hello LED",
    "@context": "https://webthings.io/schemas/",
    "@type": [
        "Light"
    ],
    "links": [
        {
            "rel": "events",
            "href": "/events"
        },
        {
            "rel": "properties",
            "href": "/properties"
        }
    ],
    "properties": {
        "on": {
            "forms": [ // <-- changed to forms
                {
                    "href": "/properties/on"
                }
            ],
            "title": "On",
            "type": "boolean",
            "@type": "OnOffProperty",
            "readOnly": false
        }
    },
    "events": {}
}

@benfrancis
Copy link
Member Author

An update on the open questions...

  1. Should a top level link to a WebSocket API endpoint be a link or a form?

My conclusion from w3c/wot-thing-description#1070 is that this can be a form, and that we can simply omit the op member from that form altogether and rely on the protocol scheme and a subprotocol member to tell Consumers how to use it.

  1. What should we do with top level links to properties, actions and events endpoints?

My current thinking is that:

@relu91
Copy link
Collaborator

relu91 commented May 14, 2021

I wonder if it makes sense to create a second schema repo that contains the W3C types rather than the IPC types.
This way both server and client types from the schema.

@tim-hellhake about this I recently landed a PR that introduces the typescript definitions based on the W3C JSON schema. It is still not published yet, but maybe we could re-use it in our IPC types in the future.

@benfrancis
Copy link
Member Author

benfrancis commented Jul 20, 2021

My conclusion from w3c/wot-thing-description#1070 is that this can be a form, and that we can simply omit the op member from that form altogether and rely on the protocol scheme and a subprotocol member to tell Consumers how to use it.

See further discussion on this topic in w3c/wot-thing-description#878

My preference is still to provide a single top level Form for the WebSocket endpoint with no op member, since the alternative (also providing a Form for each action affordance with the same URL) creates a lot of mess and redundnacy. That would work for WebThings Gateway since it also exposes HTTP endpoints for each interaction affordance, but the same approach wouldn't work for a WebSocket-only device because the forms member of an InteractionAffordance is currently mandatory.

@benfrancis
Copy link
Member Author

I've just been testing #2811 and it's looking good. My understanding of what is still missing:

  1. Changing the WebSocket endpoint from a Link to a Form as discussed above (we need to decide whether a single top level form is enough, see Describing initial connection w3c/wot-thing-description#878 (comment))
  2. Changing Event affordances to provide a Server-Sent Events endpoint which we can then describe with a Form, rather than trying to describe the current historical log (blocked by Add support for Server Sent Events #2830)
  3. Changing the top level events endpoint from a Link to a Form, using the new subscribeallevents operation (Add subscribeallevents and unsubscribeallevents operations w3c/wot-thing-description#1082) (also blocked by Add support for Server Sent Events #2830)
  4. Remove "rel": "event" from the Forms of Event affordances
  5. Deciding what to do with the top level actions endpoint (I have raised the question of potential invokeanyaction/queryallactions operations at Is there a need for invokeanyaction/queryallactions operations? w3c/wot-thing-description#1200)

Since #2830 is now blocking this issue, I'm going to start looking into implementing Server-Sent Events support.

@benfrancis benfrancis added the wot-thing-description W3C WoT Thing Description specification label Jul 26, 2021
@relu91
Copy link
Collaborator

relu91 commented Jul 27, 2021

Changing the WebSocket endpoint from a Link to a Form as discussed above (we need to decide whether a single top level form is enough, see Describing initial connection w3c/wot-thing-description#878 (comment))

I agree with the direction described in the comment. Thanks to the fact that we already have a Rest API in place, TD consumers will still be able to interact with the Web Thing. Plus the TD will be valid according to the schema. What might be a little bit annoying is how we describe the situation in the TD spec:

  1. We describe the top-level form and we prescribe something on forms in affordances. For example: if a top-level form exist then all affordances MUST contain a form that (insert how the form should be)
  2. We describe the top-level form but we suggest something on forms in affordances: For example: if a top-level form exist then all affordances SHOULD contain a form that (insert how the form should be)

Option 2 would be optimal for our purposes because it will make our TD valid even on the assertion level.

Remove "rel": "event" from the Forms of Event affordances

Oh sorry, I miss that, I'll push an update soon.

relu91 added a commit to relu91/gateway that referenced this issue Sep 29, 2021
relu91 added a commit to relu91/gateway that referenced this issue Sep 29, 2021
relu91 added a commit to relu91/gateway that referenced this issue Sep 29, 2021
relu91 added a commit to relu91/gateway that referenced this issue Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
task w3c-compliance wot-thing-description W3C WoT Thing Description specification
Projects
Status: Done
Development

No branches or pull requests

5 participants