-
Notifications
You must be signed in to change notification settings - Fork 25
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
BACnet support for events #379
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for wot-binding-templates ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
https://deploy-preview-379--wot-binding-templates.netlify.app/bindings/protocols/bacnet/index.html#event-mappings can be used for seeing the rendered version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good to me, although I haven't checked all the details in the new tables.
However, I think a few more points should be added to make this work within the conceptual framework of WoT. In general, WoT tries to abstract away from the exact protocols used; for a TD consumer, it should not matter whether they use BACnet or another underlying protocol; the drivers take care of the protocol-specific details.
This PR essentially defines a layer above the BACnet Protocol Binding that allows BACnet alarms to be distributed to TD consumers and for them to acknowledge these alarms. While I am not against the BACnet Protocol Binding and this higher layer being specified in the same document, they should be clearly separated and decoupled. Even though the higher layer will usually be used with the BACnet Protocol Binding, it could in principle also be used with the MQTT Protocol Binding, for example.
Let's say a TD consumer sees an event in a TD and it does not look at the contained form(s):
- How does the consumer recognize that this event delivers BACnet events?
- Should the consumer have the specified data schema hard-coded and compare it to the data schema in the TD, and if they are (almost?) identical, then it's BACnet events?
- How does the consumer recognize that it can/must acknowledge these events?
- How does the consumer know which action to use for confirmation?
- Should the consumer have the identifier of the action hard-coded?
Let's say I'm implementing a WoT/BACnet driver:
- How does the driver know which BACnet protocol element goes into which key-value pair in the JSON data schema? Is this purely based on the key names?
- I suppose the data schema might evolve with future revisions of the document. What should a driver do if there are newly specified key-value pairs in the data schema that it doesn't yet know about, or if pairs that are currently specified are missing?
Nit:
- You wrote that the terms "BACnet Event" and "BACnet Alarm" can be used interchangeably. I would suggest using "BACnet Alarm" throughout the document to avoid any confusion with WoT events.
@ektrah thanks for the review and sorry for late reply.
Fully agree that the logical payload should be decoupled from the underlying protocol. This is what I tried to express with: "Thus, it is necessary for the WoT consumer to work with a given logical data schema that is influenced by the BACnet standard. Still, this binding takes an approach to define the data schemata of the InteractionAffordances as independent as possible from BACnet, in order to avoid a direct dependency of WoT consumers to a specific protocol." I tried to handle the decoupling of the logical layer from the protocol binding in the table under the section 6.3, similarly to what we did before in section 6.2. If you have ideas how this can be improved, I'm glad to incorporate them.
I think the goal is that the consumer shouldn't recognize that these are actually BACnet events.
No. The consumer should just work with the schema. If there is an exceptional reason for the consumer to know it's actually BACnet, it should look in the forms.
Using the ackReq field.
There is only one action in the TD: AcknowledgeEvent. This could maybe be improved with a well-known
Not the id, but the title. Maybe the
Yes, the whole data model is well-known to the BACnet driver, hence it is in this protocol binding spec.
This is a general problem for all WoT protocol bindings. If protocol bindings introduce new fields, how is this versioned between the implementations and the binding spec? Is this already conceptualized somewhere?
That is a good idea, I did it now for the places where the term occurs in isolation. However, BACnet has also some phrases in its spec like "event notification", "event state", "event enrollment"; there it would cause more confusion to not use the standard phrase from BACnet, so I kept them. |
I think that @ektrah has many valid points. There are many concepts in this binding that do not exist in the abstraction of WoT. For example, acknowledging an event with an action is not something that we are able to express in a generic way. So saying I think this PR should be merged and should be taken as a primary example for manageable affordances discussion (or maybe some state machine modeling of Things). Until we solve that discussion, there will be no way to handle this in a generic fashion. And waiting for that discussion to be resolved is not going to help since that is how BACnet works and not supporting their eventing mechanism would simply mean that BACnet devices with events will not have events in their TD, thus they would be handled out-of-band. This is something WoT standards advocates against, so this is definitely an improvement. |
Call of 06.11:
|
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Discussion with @egekorkan and @ektrah today:
We will formulate this differently, because the payload is intended to be abstracted from BACnet, so also a WoT Consumer without knowledge of BACnet can work with events. However, some implicit knowledge is required to link
I will add these both for Example for note: https://github.com/w3c/wot-thing-description/blob/main/index.html#L1704 |
more understandable
…Affordances PR feedback
@egekorkan @ektrah I added the note now, as discussed yesterday, please have a look. |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Bring support for BACnet Events (used interchangeably with Alarms) to WoT BACnet Protocol Binding.
My general approach in this PR is to allow WoT Consumers to interact with EventAffordances in a certain way. I didn't try to cover all features and options of BACnet. I find it better to start simple and add features as they become necessary.
This PR bases on #377, so that one should be merged first.
Preview of rendered doc: https://deploy-preview-379--wot-binding-templates.netlify.app/bindings/protocols/bacnet/index.html#event-mappings