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

BACnet support for events #379

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Conversation

fennibay
Copy link
Contributor

@fennibay fennibay commented Jul 30, 2024

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

@fennibay fennibay requested a review from egekorkan July 30, 2024 13:23
Copy link

netlify bot commented Jul 30, 2024

Deploy Preview for wot-binding-templates ready!

Name Link
🔨 Latest commit c8c7882
🔍 Latest deploy log https://app.netlify.com/sites/wot-binding-templates/deploys/6736faaed6a7de000885de29
😎 Deploy Preview https://deploy-preview-379--wot-binding-templates.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@egekorkan egekorkan added enhancement bacnet related to bacnet protocol binding labels Jul 30, 2024
@egekorkan
Copy link
Contributor

https://deploy-preview-379--wot-binding-templates.netlify.app/bindings/protocols/bacnet/index.html#event-mappings can be used for seeing the rendered version

bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
Copy link
Member

@ektrah ektrah left a 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.

@fennibay
Copy link
Contributor Author

@ektrah thanks for the review and sorry for late reply.

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.

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.

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?

I think the goal is that the consumer shouldn't recognize that these are actually 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?

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.

  • How does the consumer recognize that it can/must acknowledge these events?

Using the ackReq field.

  • How does the consumer know which action to use for confirmation?

There is only one action in the TD: AcknowledgeEvent. This could maybe be improved with a well-known @type, but I'm not sure if that's necessary.

  • Should the consumer have the identifier of the action hard-coded?

Not the id, but the title. Maybe the @type

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?

Yes, the whole data model is well-known to the BACnet driver, hence it is in this protocol binding spec.

  • 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?

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?
BACnet binding would follow it.

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.

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.

@fennibay fennibay marked this pull request as ready for review October 29, 2024 11:21
@fennibay fennibay changed the title Draft: BACnet support for events BACnet support for events Oct 29, 2024
@egekorkan
Copy link
Contributor

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 Using the ackReq field. is correct but ackReq is not a generic WoT vocabulary for a Consumer to understand. How do we proceed though?

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.

@egekorkan egekorkan added Has Use Case Potential The use case can be extracted and explained Selected for Use Case labels Nov 6, 2024
@egekorkan
Copy link
Contributor

Call of 06.11:

  • It needs to be written in the document that this kind of mechanism require a "BACnet specific Consumer".
  • An annotation for the specific actions is preferred so that the Consumer can easily find such important actions related to the event.
  • Relationship between the both "source" keywords should be explained.
  • These explanations should be added as notes in the spec
  • People are fine with merging this PR but all agree that a generic mechanism is needed to cover this in the WoT abstraction level, which would work with more protocols.

bindings/protocols/bacnet/bacnet-model.ttl Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
@fennibay
Copy link
Contributor Author

Discussion with @egekorkan and @ektrah today:

  • It needs to be written in the document that this kind of mechanism require a "BACnet specific Consumer".

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 ackReq to AcknowledgeEvent, which is currently specified only in the BACnet protocol bindings. This will be documented in the EventAffordance short-term, and the long-term solution is "manageable affordances", see below.

  • An annotation for the specific actions is preferred so that the Consumer can easily find such important actions related to the event.
  • Relationship between the both "source" keywords should be explained.
  • These explanations should be added as notes in the spec

I will add these both for GetCurrentEvents and AcknowledgeEvent, and provide a link to manageable affordances

Example for note: https://github.com/w3c/wot-thing-description/blob/main/index.html#L1704

@fennibay
Copy link
Contributor Author

@egekorkan @ektrah I added the note now, as discussed yesterday, please have a look.

bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
bindings/protocols/bacnet/index.html Show resolved Hide resolved
bindings/protocols/bacnet/index.html Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bacnet related to bacnet protocol binding enhancement Has Use Case Potential The use case can be extracted and explained Selected for Use Case
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants