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

 

espy
 

On 4/9/19 2:56 PM, Trevor.Conn@... wrote:

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

I just added a comment to the above issue. Sorry for not chiming in sooner.

 

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)
}

The approach you outline here differs to what was outlined in the PR above.  In this case, the device service is still encoding the event twice, it's just now using an EventClient method to do it.

I think the correct place to be calling EventClient.MarshallEvent is in device-sdk-go/internal/handler/command.go in the functions execReadCommand. This is the rough logic (note this isn't working code, but it meant to express the idea):

       // push to Core Data
        event := &models.Event{Device: device.Name, Readings: readings}
        event.Origin = time.Now().UnixNano() / int64(time.Millisecond)

        data, _ := common.EventClient.MarshalEvent(event) //errors are not returned from commandFunc thus ignored here

        go common.SendEvent(data)

        return data, nil

Regards,
/tony



 

Trevor Conn

Technical Staff Engineer

Core Working Group Chair of EdgeX Foundry

Dell Technologies | IoT DellTech

Trevor.Conn@...

Round Rock, TX USA