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

[cloud] RFC0007 - Mappings for Cloud Event Attributes to CoAP Options #8

Merged
merged 13 commits into from Aug 21, 2021

Conversation

pranav-bhatt
Copy link
Contributor

@pranav-bhatt pranav-bhatt commented May 21, 2021

Motivation

CoAP uses Options instead of the traditional headers we see in other protocols. These options are represented by option numbers ranging from 0-65536, with some options numbers having reserved definitions in the CoAP specification.

This document aims to associate certain Options to Cloud Event fields (usage defined by this RFC).

Please see this link to see the rendered spec for greater clarity.

Signed-off-by: Pranav <adpranavb2000@gmail.com>
Signed-off-by: Pranav <adpranavb2000@gmail.com>
Signed-off-by: Pranav <adpranavb2000@gmail.com>
Signed-off-by: Pranav <adpranavb2000@gmail.com>
@pranav-bhatt pranav-bhatt changed the title [cloud] RFC0007 - Mappings for Cloud Events and CoAP [cloud] RFC0007 - Mappings for Cloud Event Attributes to CoAP Options May 21, 2021
Signed-off-by: Pranav <adpranavb2000@gmail.com>
Copy link
Member

@lulf lulf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @pranav-bhatt! I think overall whats there so far looks good. Some comments:

Signed-off-by: Pranav <adpranavb2000@gmail.com>
@pranav-bhatt pranav-bhatt requested a review from lulf May 27, 2021 12:08
Signed-off-by: Pranav <adpranavb2000@gmail.com>
Signed-off-by: Pranav <adpranavb2000@gmail.com>
Copy link
Member

@lulf lulf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I just have a few comments/questions:

  • IMO the specversion should be a string in order to treat it as an opaque value in the endpoint
  • For the subject, I wonder if it would make sense to use the uri path like for HTTP for the channel?
  • I'm not sure if event type should be set by device, I think it is always set by endpoint to the "drogue event version" format the endpoint uses.

@pranav-bhatt
Copy link
Contributor Author

Nice work! I just have a few comments/questions:

* IMO the specversion should be a string in order to treat it as an opaque value in the endpoint

* For the `subject`, I wonder if it would make sense to use the uri path like for HTTP for the channel?

* I'm not sure if event type should be set by device, I think it is always set by endpoint to the "drogue event version" format the endpoint uses.

Thanks for the feedback!

  • Yeah I needed that clarification xD
  • So there are the following options defined by the spec that let you fill in path info. I was thinking those could be used for either source or subject, but I'm not exactly sure how to format them.
  • According to the core attributes table provided in RFC0003, the type can be provided by the device. That's why I set the values as such. I could change it if you want it to be set only by the endpoint.

@lulf
Copy link
Member

lulf commented May 27, 2021

Nice work! I just have a few comments/questions:

* IMO the specversion should be a string in order to treat it as an opaque value in the endpoint

* For the `subject`, I wonder if it would make sense to use the uri path like for HTTP for the channel?

* I'm not sure if event type should be set by device, I think it is always set by endpoint to the "drogue event version" format the endpoint uses.

Thanks for the feedback!

* Yeah I needed that clarification xD

* So there are the [following options defined by the spec](https://datatracker.ietf.org/doc/html/rfc7252#section-5.10.1) that let you fill in path info. I was thinking those could be used for either `source` or `subject`, but I'm not exactly sure how to format them.

For subject, if we compare with the http endpoint, I think I think Uri-Path would include "/v1/foo", and then in the endpoint you'd extract "foo" to be the subject/channel name (correct me if I'm wrong, @ctron). So, device uses the coap uri coap://example.com/v1/foo will set Uri-Path to "/v1/foo". You can probably test/check this with a coap client.

The way I'm interpreting source is that it is unrelated to the CoAP URI as such, so it should use a custom Option number.

* According to the [core attributes table](https://github.com/drogue-iot/rfcs/blob/main/active/0003-cloud-events-mapping.md#core-attributes) provided in RFC0003, the `type` can be provided by the device. That's why I set the values as such. I could change it if you want it to be set only by the endpoint.

Ok, it's probably fine to leave it as is. I got slightly confused with the column name, "Sent by device", when it's in fact "May be sent by device"

@ctron
Copy link
Member

ctron commented May 27, 2021

@lulf correct with the subject. I would put that on the path (if that is possible with CoAP) and not use an option for it.

Same for the other properties, which are generated by the endpoint. I don't think they should have options assigned.

@lulf
Copy link
Member

lulf commented May 27, 2021

@ctron For the properties that are generated by the endpoint, doesn't there need to be reverse mapping from cloud event to coap option in the event of command messages sent to the device, or are/should there be no such conversion?

Btw, the Url-Path option is the way CoAP communicates the path. Most coap clients will translate the "coap url" path and set this option.

Signed-off-by: Pranav <adpranavb2000@gmail.com>
Signed-off-by: Pranav <adpranavb2000@gmail.com>
Signed-off-by: Pranav <adpranavb2000@gmail.com>
Signed-off-by: pranav <adpranavb2000@gmail.com>
@pranav-bhatt pranav-bhatt marked this pull request as ready for review July 7, 2021 09:16
Signed-off-by: pranav <adpranavb2000@gmail.com>
@pranav-bhatt
Copy link
Contributor Author

@ctron @lulf any further changes?

@lulf
Copy link
Member

lulf commented Aug 21, 2021

I think its good to merge @pranav-bhatt

@lulf lulf merged commit 66af699 into drogue-iot:main Aug 21, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants