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 media type #357

Closed
egekorkan opened this issue Apr 1, 2024 · 18 comments
Closed

BACnet media type #357

egekorkan opened this issue Apr 1, 2024 · 18 comments
Labels
bacnet related to bacnet protocol binding

Comments

@egekorkan
Copy link
Contributor

In the BACnet binding, there is no usage of contentType in the examples so the default mechanism would apply and we would get JSON payloads. However, BACnet is not using JSON afaik. How should we handle this @fennibay ?

@fennibay
Copy link
Contributor

fennibay commented Apr 2, 2024

I understand that the logical payload is always JSON-based. contentType defines the physical payload on the wire.

BACnet always uses the same ASN.1-based encoding on the wire. It is implemented in the available BACnet stacks.

So from BACnet POV there is not really a need for a contentType field.

But for the sake of completeness we could define one as a default in schema. I checked the media types on IANA, and I don't see anything for BACnet or ASN.1. So we would have to register one on IANA first.

There is application/bacnet-xdd+zip, but that one is not for BACnet data, but a special BACnet file format, used for exchanging device descriptions.

@egekorkan
Copy link
Contributor Author

So from BACnet POV there is not really a need for a contentType field.

The issue is that if we don't define one, a TD process MUST automatically insert application/json (see here).

The contentType in a TD form doesn't have to be registered in IANA first (at least for now) so it should be fine to define one in the binding spec for now.

@fennibay
Copy link
Contributor

fennibay commented Apr 2, 2024

Ok, so can we define it like this?

"contentType": "application/X-bacnet"

@danielpeintner
Copy link
Contributor

danielpeintner commented Apr 4, 2024

Minor: I usually see lower-case content-types only...

Even though looking into https://datatracker.ietf.org/doc/html/rfc7231#section-3.1.1.1 states some vague statements like

The type, subtype, and parameter name tokens are case-insensitive.
Parameter values might or might not be case-sensitive, depending on
the semantics of the parameter name.

@egekorkan
Copy link
Contributor Author

I propose to use "application/bacnet-x" to be aligned with lowercase throughout and starting with bacnet since the other bacnet media type starts like that too. Ideally, something should be registered to IANA. Any thoughts @ektrah and @mjkoster ?

@ektrah
Copy link
Member

ektrah commented Dec 3, 2024

If a media type is strictly needed, registering one with IANA would be the right way to go. No "x-" prefix or similar should be used (see RFC 6648). The requirements and registration process for a media type are described in RFC 6838. So we would need to have at least a fully completed registration template. Ideally, there should also be a detailed specification of the media type itself.

Overall, however, I'm not sure how much sense it makes to register the payload formats of all sorts of non-Internet protocols as Internet Media Types.

When a TD consumer invokes an action using HTTP, it needs to know how to serialize the action parameters in the HTTP message body. To do this, the TD needs to specify a media type to use. In the case of CoAP, the TD consumer needs to serialize the data in a CoAP content format, which is more than just a media type. In the case of BACnet, the TD consumer always serializes the data according to the BACnet specification.

This means that even though protocols and serialization formats can be considered orthogonal, I'd say the vocabulary to specify the serialization format to use is protocol specific. And for BACnet, no vocabulary is needed because it is always unambiguously clear.

@ektrah
Copy link
Member

ektrah commented Dec 3, 2024

In other words, instead of this:

before

... I think we should be doing this:

after

Unlike a "HTTP driver", a "BACnet driver" already knows how to convert data to and from the BACnet payload format and does not need to reach out to an orthogonal data serializer.

@egekorkan
Copy link
Contributor Author

I agree that it is redundant information in the case of BACnet but not indicating a contentType can confuse some parsers which should assume JSON as the default for all protocols. This is an annoying limitation of the TD which we can get rid of but we would still need a value for the contentType assuming that is still mandatory. We could add some WoT-specific value like "auto", in which case the protocol stack handles it.

Regarding the images:

  • Currently, TD Consumer code does not need to pass JSON or XML to a driver. Theoretically, the data structure of the programming language should be passed to the driver, which converts it to the required media type on the fly. If the media type needed is XML, there should be no need to have an intermediary media type like JSON. The programmer of that language should feel "at home" and not need any knowledge of media types.
  • I am not sure how this is currently done in implementations that are not JavaScript based. Their input would be much more valuable. @JKRhb could comment on dart_wot implementation.
  • I like the second image more than the first one but only relatively :)

@fennibay
Copy link
Contributor

fennibay commented Dec 6, 2024

I understand contentType is useful for example in an HTTP context when the consumer doesn't know if the data should be encoded as JSON or XML in the request/response body.

And I think @ektrah @egekorkan and I agree that we don't need such an indication in BACnet-context.

So the problem is reduced into: when the contentType is missing, the consumers default to JSON, and that is not correct for BACnet.

In practice this is not an issue, because no BACnet driver would try to send JSON over the wire. But we should be careful about the concept.

Question: Where is the defaulting to JSON specified? I couldn't find it in the section on contentType.

Solution ideas:

  1. Clarify the defaulting, if missing contentType would mean "left to the driver", we can just omit it.
  2. Use octet-stream? That just means "arbitrary binary data".

@ektrah
Copy link
Member

ektrah commented Dec 6, 2024

The default value for contentType is specified in Table 31 of the TD 1.1 spec.

My preference for TD 2.0 would be for contentType to become a protocol-specific vocabulary. That is, instead of contentType being shared by all protocols, HTTP would use a new htv:contentType vocabulary term; for CoAP there already exists cov:contentFormat; and for BACnet it wouldn't need anything.

For TD 1.1, application/octet-stream as a workaround doesn't sound bad to me. The Modbus binding template already seems to do the same thing.

@egekorkan
Copy link
Contributor Author

Agree on most points so far. BACnet can use octet-stream like Modbus for now and we can make this optional in the next version. There should be a note that explains that we are doing nothing new for BACnet drivers and they should do what they always did in terms of serialisation.

Regarding making the term protocol specific: While I see the reasoning, I think if the concept is the same, we should have a single word across protocols.

@ektrah
Copy link
Member

ektrah commented Dec 9, 2024

Protocol Concept TD 1.1 Proposal for TD 2.0
HTTP
  • "contentType"
  • "contentCoding"
  • "htv:fieldName": "Content-Type", "htv:fieldValue"
  • "htv:fieldName": "Content-Encoding", "htv:fieldValue"
  • "htv:fieldName": "Content-Language", "htv:fieldValue"

or

  • "htv:contentType"
  • "htv:contentCoding"
  • "htv:contentLanguage"
CoAP
  • content-format (combination of Internet Media Type and content coding)
  • "contentType" (must be the same information as "cov:contentFormat")
  • "contentCoding" (must be the same information as "cov:contentFormat")
  • "cov:contentFormat"
  • "cov:contentFormat"
MQTT
  • "contentType" (limited to Internet Media Types)
  • "mqv:payloadFormatIndicator"
  • "mqv:contentType" (may be any string)
Modbus
  • "contentType": "application/octet-stream"
  • "modv:type"
  • "modv:mostSignificantByte"
  • "modv:mostSignificantWord"
  • "modv:type"
  • "modv:mostSignificantByte"
  • "modv:mostSignificantWord"
BACnet
  • BACnet tag
  • "contentType": "application/octet-stream"
  • "bacv:hasDataType"
  • "bacv:hasDataType"

✨= new

@danielpeintner
Copy link
Contributor

danielpeintner commented Dec 10, 2024

Thank you @ektrah for the detailed table 👍

I think I understand why one might want to have (need?) protocol-specific terms. However, this means that you can no longer make any statements about the data without knowing the protocol. I am myself not sure whether I want that to happen.
I see (and understand) the argument that one needs to know the protocol anyway, but that may make the core TD become an abstract wrapper...

@ektrah
Copy link
Member

ektrah commented Dec 10, 2024

Protocol Assuming I know Internet Media Types,
can I understand the data without knowing the protocol in TD 1.1?
HTTP 🟩 Yes, because the protocol happens to use Internet Media Types
CoAP 🟨 Yes, at the cost of redundant information in the TD alongside the protocol-specific vocabulary
MQTT 🟨 Yes, at the cost of not being able to specify a payload format indicator or a content type that is not an Internet Media Type in a TD
Modbus 🟥 No, you need to understand the protocol-specific vocabulary
BACnet 🟥 No, you need to understand the protocol-specific vocabulary

@JKRhb
Copy link
Member

JKRhb commented Dec 10, 2024

  • Currently, TD Consumer code does not need to pass JSON or XML to a driver. Theoretically, the data structure of the programming language should be passed to the driver, which converts it to the required media type on the fly. If the media type needed is XML, there should be no need to have an intermediary media type like JSON. The programmer of that language should feel "at home" and not need any knowledge of media types.

  • I am not sure how this is currently done in implementations that are not JavaScript based. Their input would be much more valuable. @JKRhb could comment on dart_wot implementation.

As it is the case with node-wot, dart_wot implements the relevant parts of the Scripting API here, where the value of an InteractionOutput is mapped to one of the DataSchemaValue types null, boolean, number, string, array, or object. As you are further limited to the constraints set by JSON schema, you basically have JSON as an intermediary format when trying to deserialize a payload, which can only be circumvented if you use the binary output (via the arrayBuffer method) and deserialize it yourself (without using the codecs provided by node-wot/dart_wot). XML is the classic example for the discrepancy we have here, as it is not that well describable by JSON Schema.

Therefore, the schema information is unfortunately not that universally usable, which made me think whether payload bindings should rather each define their own schemas for validation (so the current DataSchemas in the core TD specification would only apply to JSON). As a consequence, the value of InteractionOutputs could be generic over the format-specific types of DataSchemaValues. All of this of course has the downside of potentially increasing complexity and reducing the content of the core TD, but maybe this is another aspect that could be discussed as part of a general overhaul of how media types are handled within TDs.

fennibay added a commit that referenced this issue Dec 16, 2024
Without specification it falls back to application/json, which is wrong
Now it will default to application/octet-stream, which means arbitrary binary data

BACnet media type #357
@fennibay
Copy link
Contributor

Therefore, the schema information is unfortunately not that universally usable, which made me think whether payload bindings should rather each define their own schemas for validation (so the current DataSchemas in the core TD specification would only apply to JSON).

We had this problem also in BACnet. Simple example: JSON has only one integer type, and BACnet differentiates between signed and unsigned. Solution was to allow BACnet define its own data model under the protocol binding, see bacv:hasDataType. Somewhat redundant, but solves the problem completely.

The contentType is a bit of a separate problem. Even when you have handled all type system differences, you might still need to distinguish between different formats: XML, JSON, YAML, CBOR... Thankfully we don't have this problem in BACnet.

@fennibay
Copy link
Contributor

I created now #390 to address this issue.

@egekorkan
Copy link
Contributor Author

fixed by #390

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
Projects
None yet
Development

No branches or pull requests

5 participants