search

WebThingsIO / webthing-arduino

Simple server for ESP8266, ESP32, Ethernet, or WiFi101-compatible boards compliant with Mozilla's proposed WoT API
Mozilla Public License 2.0
207 stars 78 forks source link

Migration to W3C WoT standard #140

Closed Citrullin closed 3 years ago

Citrullin commented 3 years ago

A follow up for #139. Only contains the actual migration of the library without example.

Citrullin commented 3 years ago

Close, since the team doesn't want to support the Web of Things TD 1.0 standard defined by the W3C.

benfrancis commented 3 years ago

Hi @Citrullin,

I can see that you are frustrated, but please don't be disheartened by requests for changes in a code review, this is a normal part of the contribution process.

I can assure you that the team does want WebThings to comply with W3C WoT specifications, it is currently the primary focus of the 2.0 release and I have personally been working on this nearly full time for a number of months now. Please understand that webthing-arduino is just one of 16 libraries, 2 gateway implementations and over 100 add-ons which all rely on the same Web Thing API, so any changes to this API require significant co-ordination. And also that WebThings is maintained by a group of volunteers who mostly contribute in their spare time and may not have time to review large patches with no accompanying explanation or prior discussion.

As you hopefully know, @relu91 and I have been working hard over many months to discuss the best approach to make WebThings W3C WoT compliant, which has included many long meetings with three different W3C task forces to introduce new features to W3C specifications to make it possible. See https://github.com/WebThingsIO/gateway/issues/2802 for a high level summary of that discussion.

I'm afraid the approach in this pull request of simply removing any features which can not be described by the existing Thing Description 1.0 specification is not acceptable in my view.

Ideally it would have been better for you to participate in the wider discussion and co-ordinate your work with the changes made to the gateway API. However, if you are interested in continuing to contribute then I'd be happy to set up a call to co-ordinate our work and discuss how you can contribute more effectively.

Thank you for the time you have already invested in the project.

Citrullin commented 3 years ago

@benfrancis I don't have the time to have such a discussion if keeping the Mozilla RFC is more important than being compliant to the W3C recommendation. From my point of view this topic is pretty simple. WebThings isn't a finished and usable product. It uses the Mozilla RFC which isn't an industry standard. So, going for the industry standard, even if it isn't ideal should have the highest priority. I understand that some features are missing. But those can be added later in another iteration, instead of practically standing still. It's software, you can still add those features.

I need a library I can use now and I can advertise now with. I am not going to wait 2 years or whatever until I can actually use that library and market it.

Ideally it would have been better for you to participate in the wider discussion and co-ordinate your work with the changes made to the gateway API. However, if you are interested in continuing to contribute then I'd be happy to set up a call to co-ordinate our work and discuss how you can contribute more effectively.

Discussions are not necessary here and it also doesn't make sense to discuss. You made your point clear. You don't want to be WoT TD 1.0 compliant and I can't change that. Therefore there isn't anything to coordinate.

rzr commented 3 years ago

Take it easy friends, this project is under community maintenance... What I can suggest to you is to try to split into smaller efforts then it will be easier for us to be integrated (and yes it can take time, we wont stop you to maintain a fork but it would be in community interest to avoid that)

More hints can help:

https://rzr.github.io/rzr-presentations/docs/flows

My 2c

benfrancis commented 3 years ago

@Citrullin wrote:

if keeping the Mozilla RFC is more important than being compliant to the W3C recommendation.

This is a misrepresentation.

  1. We are not trying to continue to be compliant with our old Web Thing Description specification, we are trying to keep features of the API which our existing consumers depend on.
  2. It is not possible to describe the whole existing API using a WoT Thing Description 1.0 compliant Thing Description because the specification is missing key features.
  3. It is also not necessary to remove features from the API in order to make the Thing Descriptions exposed by devices W3C compliant (even WoT Thing Description 1.0 compliant), you can just choose not to describe those features in the Thing Description until Thing Descriptions gain the semantics to describe them.

Discussions are not necessary here and it also doesn't make sense to discuss.

If you are not willing to engage in discussion, then you are welcome to go elsewhere.

Citrullin commented 3 years ago

@Citrullin wrote:

if keeping the Mozilla RFC is more important than being compliant to the W3C recommendation.

This is a misrepresentation.

1. We are not trying to continue to be compliant with our old [Web Thing Description](https://webthings.io/api/#web-thing-description) specification, we are trying to keep features of the API which our existing consumers depend on.

I don't understand how this should effect a library which doesn't consume anything, but only produces a TD. If your gateway is able to read the WoT TD 1.0 this shouldn't even effect this library at all.

2. It is not possible to describe the whole existing API using a WoT Thing Description 1.0 compliant Thing Description because the specification is missing key features.

It doesn't need to. This is a library which doesn't act as a client.

3. It is also not necessary to remove features from the API in order to make the Thing Descriptions exposed by devices W3C compliant (even WoT Thing Description 1.0 compliant), you can just choose not to describe those features in the Thing Description until Thing Descriptions gain the semantics to describe them.

Well, I tested it and removed it for a reason. I kept all the other code I don't use. I don't remember the details, but it probably broke something. Besides that, the library has generally not the highest code quality. Everything is just in two header files. Besides that, the structure can improve a lot. So, I don't even understand all the discussions for a library which should get a major refactoring anyways.

Discussions are not necessary here and it also doesn't make sense to discuss.

If you are not willing to engage in discussion, then you are welcome to go elsewhere.

That's why I just closed the PR. I made the library WoT TD 1.0 compliant and removed everything that was in the way of getting there.

benfrancis commented 3 years ago

Can you list your must haves in order to make it mergeable? I am getting really confused, because there are so many things you mentioned.

OK, to summarise:

  1. Remove the rel member from Forms since it isn't needed
  2. Do not prepend the string "uri:" to the id member (if you want to add a check that the ID is valid URI, that's fine)
  3. Remove .well-known/wot-thing-description from the path of Thing Description(s) and mDNS broadcast. We'll implement the WoT Discovery specification later and make this an alias of the canonical Thing Description URL(s)
  4. Keep the top level endpoints for properties and actions using top level Forms with readallpropertiesand queryallactions operations. These meta operations are an important feature of the web thing (e.g. see https://github.com/WebThingsIO/thing-url-adapter/issues/106) which should not be removed. (See below for a note about events)
  5. You may need to modify the payload of an invokeaction operation to remove the object wrapper, like we did in WebThings Gateway, so that the payload matches the input data schema
  6. I am personally OK with removing the feature that enables a single device to expose multiple web things at a /things endpoint, however the module owner (@rzr) has the final decision. Note that only the resources at /things/{id} are actually Thing Descriptions which need to be compliant with the W3C WoT Thing Description 1.1 specification. The resource at /things provides an array of multiple things. If this is kept then it could either be made compliant with the WoT Discovery Directory Service API, or just left as it is since it's not doing any harm (not every resource served by a web server has to be a Thing Description)
  7. Rename variable names from "link" to "form" where appropriate
  8. Replace the timestamp in events
  9. Replace the webthings.io context. I suggest including both the W3C and WebThings contexts in an array ("@context": ["https://www.w3.org/2019/wot/td/v1", "https://webthings.io/schemas/"]) since this won't require changes in WebThings Gateway. If WoT Consumers say this is an invalid TD because the webthings.io/schemas resource doesn't resolve to a valid JSON-LD document then I suggest that's a bug in the Consumer. However, we will separately work on creating an appropriate JSON-LD resource and changing the hosting solution to fix this.
  10. Change the security member back to a string
  11. Remove the commented out line on line 740

Finally, we need to decide what to do about events. There is no way to describe the current Event and Events resources in the current Web Thing REST API using a W3C Thing Description (even in v1.1) because they provide a list of past events rather than an event stream which can be subscribed/unsubscribed to.

In WebThings Gateway the solution we came up with for events was to implement Server-Sent Events endpoints at /events and /events/{id} using HTTP content negotiation so that a client can choose either the existing JSON resource or an event stream. If you are interested in implementing that in webthing-arduino too then that's great. If not, then then only solution I can think of is to simply omit references to events in the Thing Description altogether, but leave the existing events API in place in case someone wants to use it until we get around to implementing SSE. Note that events can still be subscribed to using the WebSocket endpoint, which can be described by a top level Form.

As the module owner, @rzr may have further comments.

@Citrullin @sebastiankb I hope this helps.

rzr commented 2 years ago

May I orphan this module since my involvement in webthings has declined since I am busy with oniroproject ..

benfrancis commented 2 years ago

@rzr I've sent you a message on Matrix

ameerhazo commented 2 years ago

Hello @benfrancis, are there any plans to include the usage of different ontologies such as iotschema.org in the future? The WoT implementation that my team is developing has been using this ontology to describe different kinds of smart devices such as our smart meters.

benfrancis commented 2 years ago

@ameerhazo Yes, see https://github.com/WebThingsIO/gateway/issues/2929

There's nothing to stop you using other ontologies in your Things today via semantic annotations on the Producer side, but on the Consumer side WebThings Gateway won't yet do anything special with them.