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

Questions about contributing #149

Open
0815fox opened this issue Oct 5, 2021 · 4 comments
Open

Questions about contributing #149

0815fox opened this issue Oct 5, 2021 · 4 comments

Comments

@0815fox
Copy link

0815fox commented Oct 5, 2021

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:

  • only notify about events when subscribed
  • as a special case of event: only notify about property changes when there is a subscriber - if they support this kind of notification at all -> read back from the thing itself in case of a get is often required.
    While the latter is an implementation thing of the webthings-node, for which I'd like to file a PR, which introduces a get callback function returning a promise on Value, 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?

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
Copy link
Author

0815fox commented Oct 6, 2021

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

@tim-hellhake
Copy link
Member

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
Copy link
Author

0815fox commented Oct 13, 2021

Part 1:

No, it isn't.
See #152.
👍 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:

  • Motor Valve
  • Pump controller
  • Room temperature unit
  • Wood oven air inlet valve (coming a bit later)
  • High current AC switching unit (for electric boilers etc.)
  • Central unit (on which webthings gateway runs)
  • a self-developed protocol based on CAN "CANtspro - CAN transmission and subscription protocol - really open" + C and typescript implementation; it features automatic address management, which makes it plug any play
  • cantspro-webthings-bridge (based on webthing-node)

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
Copy link
Member

@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 WebThingsIO/gateway#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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants