search

WebThingsIO / webthing-node

Node.js implementation of a Web Thing server
Mozilla Public License 2.0
232 stars 48 forks source link

Questions about contributing #149

Open 0815fox opened 3 years ago

0815fox commented 3 years ago

Hi,

I'd like to contribute to this project, as I use it in my own home system (with some self-made things, which I plan to release as free open source hardware + software as soon as I got a complete breakthrough).

I already provided a small pull request an hour ago. There are some things, which I'd like to discuss, before taking the effort.

What I noticed is for example, that the target files js/map/d.ts are transpiled/generated directly into the same location where the source files are. This makes navigation in code harder. Is this by intention? Would a PR be accepted, which puts the generated files into a bin folder (or whatever name you prefer?) instead?

Also I'd suggest to extract the SingleThing / MultipleThings classes into separate files to make server.ts easier to grasp. Handlers may be extracted e.g. into a folder lib/handlers if accepted.

I noticed that it is rather hard to integrate things with any kind of subscriber pattern. Those things would:

My "workaround" to integrate those devices will be, that my webthings bridge subscribes on all events when adding a thing, which however may overcrowd the limited bandwidth of the local CAN bus in cases where there are many things with many events. Another workaround would be to add an additional property for each event to be able to turn it on/off.

I know that's much information. I am willing to split this ticket up and rename it and also provide PRs depending on your feedback.

Thank you, Michael

0815fox commented 3 years ago

See PR #150 for a suggestion of a promisified variant of get/set on properties with a requestor callback for get.

tim-hellhake commented 3 years ago

Thanks for your contributions!

What I noticed is for example, that the target files js/map/d.ts are transpiled/generated directly into the same location where the source files are. This makes navigation in code harder. Is this by intention?

No, it isn't. See #152.

Also I'd suggest to extract the SingleThing / MultipleThings classes into separate files to make server.ts easier to grasp. Handlers may be extracted e.g. into a folder lib/handlers if accepted.

Good idea, go ahead!

the first one would need an extension on the WebThings specification: There is no addEventSubscription on the REST API (adding this would imply a session handling of course). The event description should contain an information on whether the event requires a subscription or if it just works. Also in general I am missing a removeEventSubscription in the WebSocket API. Where would be the best place for suggestions towards the WebThings specification?

Ping @benfrancis

0815fox commented 3 years ago

Part 1:

No, it isn't. See #152. :+1: well... thank you!

Part 2: I'll come up with another PR soon, which will suggest an improvement on the Single/Multiplethings topic. It'll be based on #151, will that change be accepted?

Part 3: I'd be glad to hear from you guys soon. ATM I'm about to prepare my components for publish as open source. It'll include the following:

The CANtspro protocoll supports subscriber logic to reduce bus load. Events will only be sent if there's at leads one subscriber. Of course that's only possible if the subscription logic is passed through all layers. Webthing-gateway would then only subscribe when there is intrest in the value. Beneficial in case of devices with many events, of which the user usually only needs a few. That's why I asked for that enhancement.

If you wish I could invite you to my namespace which is still private ATM (as there's hardware involved I want to avoid people buying stuff that doesn't yet work).

By the way: I'd be open to be involved in webthings.io if that's possible / desired.

benfrancis commented 3 years ago

@0815fox Thanks for your contributions!

Regarding event subscriptions, you are correct that there is no concept of subscribe/unsubscribe in the Web Thing REST API and currently no unsubscribe operation in the Web Thing WebSocket API.

In the case of the REST API this is intentional because currently the Events resource is just a historical log of events that you poll.

In the case of the WebSocket API we just never got around to implementing an unsubscribe message.

I don't really expect to add new features to the current Web Thing API because both are currently being standardised at the W3C and those standardised versions will replace the current ones.

The REST API is being standardised as the Core Profile in the WoT Profile specification.

The WebSocket API is being standardised as the Web Thing Protocol.

For the WoT Profile we decided to use Server-Sent Events to handle event subscriptions in the REST API, which I've now implemented in WebThings Gateway, see https://github.com/WebThingsIO/gateway/pull/2863. I'm also hoping to implement this in webthing-node within the next six months, but if you wanted to make a start yourself based on that Core Profile specification that would be most welcome.

For the WebSocket sub-protocol, unsubscribing from an event is already listed as a requirement, but it's going to take some time for it to turn into a specification and be implemented https://w3c.github.io/web-thing-protocol/requirements/#events-2 We could add a removeEventSubscription message to the Web Thing WebSocket API in the meantime, but as you say that would also need to be supported on the gateway.

By the way: I'd be open to be involved in webthings.io if that's possible / desired.

You already are!