Topics

Update on Core-Contracts Issue #62

Trevor.Conn@...
 

https://github.com/edgexfoundry/go-mod-core-contracts/issues/62

 

This is the proposal that came from this past Monday’s Device Service call wherein it was requested to expose the result of an event’s marshaling for a particular flow in which a command invoked on the device service needs to return the generated event to its caller – for example a GET command to return the current state of a device. That originating issue is here:

 

https://github.com/edgexfoundry/device-sdk-go/issues/233

 

Per the proposal, I created a feature branch that contains what I think are the required changes to accomplish this. I’d like to get some feedback from those asking for this before creating a PR. My feature branch is here:

 

https://github.com/tsconn23/go-mod-core-contracts/tree/issue_62

 

If this is correct, then the logic called out in issue 233 in device-sdk-go/internal/controller/restfuncs.go::commandFunc() will look like something this:

 

event, appErr := handler.CommandHandler(vars, body, req.Method)
 
if appErr != nil {
        http.Error(w, fmt.Sprintf("%s %s", appErr.Message(), req.URL.Path), appErr.Code())
} else if event != nil {
        data, _ := common.EventClient.MarshalEvent(event) //errors are not returned from commandFunc thus ignored here
        if event.HasBinaryValue() { // Assumes device-sdk-go PR #232 has been merged
               w.Header().Set(clients.ContentType, clients.ContentTypeCBOR)
        } else {
               w.Header().Set(clients.ContentType, clients.ContentTypeJSON)
        }
        w.Write(data)
}

 

If there are other places where an event is returned directly, then this also applies to those paths. Locations which are already using the common.EventClient.Add() function to POST new events to Core-Data will need to be refactored.

 

Please review and let me know if this is what you are looking for.

 

Trevor Conn

Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech

Trevor.Conn@...

Round Rock, TX USA

 

Tobias Mosby
 

Hi Trevor,

 

This looks functional and I could refactor PR233 to incorporate this as-is.

 

However, I want to confirm we are being intentional about establishing go-mod-core-contracts as the home for business logic; such as existence of one or more readings supplied as a binary value being used to trigger CBOR encoding of an event data model within the MarshalEvent method.

 

It seems to me that this new MarshalEvent method should add context as a parameter and perform marshaling based on content type (e.g., “application/json” vs “application/cbor”). This enables caller to apply whatever business logic to determine content type without altering the contract.

 

Toby

 

From: EdgeX-TSC-Core@... [mailto:EdgeX-TSC-Core@...] On Behalf Of Trevor.Conn@...
Sent: Tuesday, April 9, 2019 11:56 AM
To: edgex-tsc-device-services@...; EdgeX-TSC-Core@...
Subject: [Edgex-tsc-core] Update on Core-Contracts Issue #62

 

https://github.com/edgexfoundry/go-mod-core-contracts/issues/62

 

This is the proposal that came from this past Monday’s Device Service call wherein it was requested to expose the result of an event’s marshaling for a particular flow in which a command invoked on the device service needs to return the generated event to its caller – for example a GET command to return the current state of a device. That originating issue is here:

 

https://github.com/edgexfoundry/device-sdk-go/issues/233

 

Per the proposal, I created a feature branch that contains what I think are the required changes to accomplish this. I’d like to get some feedback from those asking for this before creating a PR. My feature branch is here:

 

https://github.com/tsconn23/go-mod-core-contracts/tree/issue_62

 

If this is correct, then the logic called out in issue 233 in device-sdk-go/internal/controller/restfuncs.go::commandFunc() will look like something this:

 

event, appErr := handler.CommandHandler(vars, body, req.Method)
 
if appErr != nil {
        http.Error(w, fmt.Sprintf("%s %s", appErr.Message(), req.URL.Path), appErr.Code())
} else if event != nil {
        data, _ := common.EventClient.MarshalEvent(event) //errors are not returned from commandFunc thus ignored here
        if event.HasBinaryValue() { // Assumes device-sdk-go PR #232 has been merged
               w.Header().Set(clients.ContentType, clients.ContentTypeCBOR)
        } else {
               w.Header().Set(clients.ContentType, clients.ContentTypeJSON)
        }
        w.Write(data)
}

 

If there are other places where an event is returned directly, then this also applies to those paths. Locations which are already using the common.EventClient.Add() function to POST new events to Core-Data will need to be refactored.

 

Please review and let me know if this is what you are looking for.

 

Trevor Conn

Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech

Trevor.Conn@...

Round Rock, TX USA

 

Trevor.Conn@...
 

Toby – That would mean a client could specify Content-Type “application/json” for an event with binary content. Wouldn’t break anything but it’s also not what we’ve discussed. We designed the current solution to specifically avoid this in fact due to the expected performance concerns.

 

Also, there’s no context currently in use by the code path below. If we’re starting to go down that route, then I would ask why wouldn’t the device service sdk provide a client with a similar operational footprint to those found in core-contracts so that request assembly/handling isn’t in-lined…?

 

Trevor

 

From: EdgeX-TSC-Device-Services@... <EdgeX-TSC-Device-Services@...> On Behalf Of Tobias Mosby
Sent: Tuesday, April 9, 2019 4:06 PM
To: Conn, Trevor; edgex-tsc-device-services@...; EdgeX-TSC-Core@...
Subject: Re: [Edgex-tsc-device-services] Update on Core-Contracts Issue #62

 

[EXTERNAL EMAIL]

Hi Trevor,

 

This looks functional and I could refactor PR233 to incorporate this as-is.

 

However, I want to confirm we are being intentional about establishing go-mod-core-contracts as the home for business logic; such as existence of one or more readings supplied as a binary value being used to trigger CBOR encoding of an event data model within the MarshalEvent method.

 

It seems to me that this new MarshalEvent method should add context as a parameter and perform marshaling based on content type (e.g., “application/json” vs “application/cbor”). This enables caller to apply whatever business logic to determine content type without altering the contract.

 

Toby

 

From: EdgeX-TSC-Core@... [mailto:EdgeX-TSC-Core@...] On Behalf Of Trevor.Conn@...
Sent: Tuesday, April 9, 2019 11:56 AM
To: edgex-tsc-device-services@...; EdgeX-TSC-Core@...
Subject: [Edgex-tsc-core] Update on Core-Contracts Issue #62

 

https://github.com/edgexfoundry/go-mod-core-contracts/issues/62

 

This is the proposal that came from this past Monday’s Device Service call wherein it was requested to expose the result of an event’s marshaling for a particular flow in which a command invoked on the device service needs to return the generated event to its caller – for example a GET command to return the current state of a device. That originating issue is here:

 

https://github.com/edgexfoundry/device-sdk-go/issues/233

 

Per the proposal, I created a feature branch that contains what I think are the required changes to accomplish this. I’d like to get some feedback from those asking for this before creating a PR. My feature branch is here:

 

https://github.com/tsconn23/go-mod-core-contracts/tree/issue_62

 

If this is correct, then the logic called out in issue 233 in device-sdk-go/internal/controller/restfuncs.go::commandFunc() will look like something this:

 

event, appErr := handler.CommandHandler(vars, body, req.Method)
 
if appErr != nil {
        http.Error(w, fmt.Sprintf("%s %s", appErr.Message(), req.URL.Path), appErr.Code())
} else if event != nil {
        data, _ := common.EventClient.MarshalEvent(event) //errors are not returned from commandFunc thus ignored here
        if event.HasBinaryValue() { // Assumes device-sdk-go PR #232 has been merged
               w.Header().Set(clients.ContentType, clients.ContentTypeCBOR)
        } else {
               w.Header().Set(clients.ContentType, clients.ContentTypeJSON)
        }
        w.Write(data)
}

 

If there are other places where an event is returned directly, then this also applies to those paths. Locations which are already using the common.EventClient.Add() function to POST new events to Core-Data will need to be refactored.

 

Please review and let me know if this is what you are looking for.

 

Trevor Conn

Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech

Trevor.Conn@...

Round Rock, TX USA

 

Trevor.Conn@...
 

I think between this thread, Monday’s Device Service meeting and Toby’s recent questions in the #device_services Slack channel, there is a previously unaccounted for use case emerging w/r/t CBOR. That use case has to do with some northside service calling core-command which calls a device service in order to pull a reading off some device

 

  • Could be a thermostat (JSON)
  • Could be a camera (CBOR)

 

And the issue is that we do not currently have a way to know before the fact what the reading is going to contain so that we can send the appropriate Content-Type header to the device service.

 

Up to now this flow simply hasn’t been in scope for Edinburgh. Only event ingestion through core-data/Add has been in scope. We are talking about increasing scope.

 

There is currently no actuation capability that I’m aware of for a device from the north side of EdgeX. We do want this at some point, but is Edinburgh the right timeframe? Or is this a Fuji thing?

 

There is also no practical use that I’m aware of for having the rules engine pull a reading in this manner. It could be done, I suppose, but to what end? The most common use of the rules engine is to make decisions based on events coming from export-distro which it evaluates and then issues PUT commands to southside devices. Correct me if I’m wrong. But regardless, right now this all sounds like a previously unaccounted for increase in scope.

 

Trevor

 

From: Conn, Trevor
Sent: Tuesday, April 9, 2019 4:46 PM
To: 'Tobias Mosby'; edgex-tsc-device-services@...; EdgeX-TSC-Core@...
Subject: RE: [Edgex-tsc-device-services] Update on Core-Contracts Issue #62

 

Toby – That would mean a client could specify Content-Type “application/json” for an event with binary content. Wouldn’t break anything but it’s also not what we’ve discussed. We designed the current solution to specifically avoid this in fact due to the expected performance concerns.

 

Also, there’s no context currently in use by the code path below. If we’re starting to go down that route, then I would ask why wouldn’t the device service sdk provide a client with a similar operational footprint to those found in core-contracts so that request assembly/handling isn’t in-lined…?

 

Trevor

 

From: EdgeX-TSC-Device-Services@... <EdgeX-TSC-Device-Services@...> On Behalf Of Tobias Mosby
Sent: Tuesday, April 9, 2019 4:06 PM
To: Conn, Trevor; edgex-tsc-device-services@...; EdgeX-TSC-Core@...
Subject: Re: [Edgex-tsc-device-services] Update on Core-Contracts Issue #62

 

[EXTERNAL EMAIL]

Hi Trevor,

 

This looks functional and I could refactor PR233 to incorporate this as-is.

 

However, I want to confirm we are being intentional about establishing go-mod-core-contracts as the home for business logic; such as existence of one or more readings supplied as a binary value being used to trigger CBOR encoding of an event data model within the MarshalEvent method.

 

It seems to me that this new MarshalEvent method should add context as a parameter and perform marshaling based on content type (e.g., “application/json” vs “application/cbor”). This enables caller to apply whatever business logic to determine content type without altering the contract.

 

Toby

 

From: EdgeX-TSC-Core@... [mailto:EdgeX-TSC-Core@...] On Behalf Of Trevor.Conn@...
Sent: Tuesday, April 9, 2019 11:56 AM
To: edgex-tsc-device-services@...; EdgeX-TSC-Core@...
Subject: [Edgex-tsc-core] Update on Core-Contracts Issue #62

 

https://github.com/edgexfoundry/go-mod-core-contracts/issues/62

 

This is the proposal that came from this past Monday’s Device Service call wherein it was requested to expose the result of an event’s marshaling for a particular flow in which a command invoked on the device service needs to return the generated event to its caller – for example a GET command to return the current state of a device. That originating issue is here:

 

https://github.com/edgexfoundry/device-sdk-go/issues/233

 

Per the proposal, I created a feature branch that contains what I think are the required changes to accomplish this. I’d like to get some feedback from those asking for this before creating a PR. My feature branch is here:

 

https://github.com/tsconn23/go-mod-core-contracts/tree/issue_62

 

If this is correct, then the logic called out in issue 233 in device-sdk-go/internal/controller/restfuncs.go::commandFunc() will look like something this:

 

event, appErr := handler.CommandHandler(vars, body, req.Method)
 
if appErr != nil {
        http.Error(w, fmt.Sprintf("%s %s", appErr.Message(), req.URL.Path), appErr.Code())
} else if event != nil {
        data, _ := common.EventClient.MarshalEvent(event) //errors are not returned from commandFunc thus ignored here
        if event.HasBinaryValue() { // Assumes device-sdk-go PR #232 has been merged
               w.Header().Set(clients.ContentType, clients.ContentTypeCBOR)
        } else {
               w.Header().Set(clients.ContentType, clients.ContentTypeJSON)
        }
        w.Write(data)
}

 

If there are other places where an event is returned directly, then this also applies to those paths. Locations which are already using the common.EventClient.Add() function to POST new events to Core-Data will need to be refactored.

 

Please review and let me know if this is what you are looking for.

 

Trevor Conn

Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech

Trevor.Conn@...

Round Rock, TX USA