Response of issueDeviceCommand is not providing CBOR Content-Type header #2272

FelixTing · 2020-01-10T06:04:33Z

🐞 Bug Report

Affected Services

The issue is located in: command

Is this a regression?

No

Description and Minimal Reproduction

Function issueDeviceCommand and issueDeviceCommandByNames is still applying the application/json Content-Type header when the response body is a CBOR event.

akramtexas · 2020-01-20T21:57:19Z

@FelixTing

As part of recreating (more on that in a minute) the bug that you've reported, I get to the point of issuing the following REST call, using my device name of "countcamera1":
http://localhost:48082/api/v1/device/name/countcamera1
And here is the response I get:
http://:0/api/v1/device/name/%7B%7BgetDeviceByName%7D%7D: dial tcp :0: connect: can't assign requested address
Can you please confirm that you're seeing the same response as well?
Can you please some detail into your setup where the response body, as you have noted, is a CBOR event? And how do you arrive at the conclusion that the functions issueDeviceCommand and issueDeviceCommandByNames are still applying the application/json Content-Type header in such a scenario (i.e. when the response body is a CBOR event)?

akramtexas · 2020-01-20T22:18:11Z

@FelixTing As I referred to in the previous comment, the steps I have following in my attempt to recreate the reported issue are these:

I start with creating reference information in EdgeX, making two calls to Core Metadata, to create the addressables for the Device Service.
I then define Value Descriptors by calls to Core Data.
Create a Device Profile via Core Metadata.
Use Core Metadata to create the Device Service.
Use Core Metadata to provision the device ("countcamera1").
Use Core Command to request a list of the commands supported by the device ("countcamera1") via the following API call:
http://localhost:48082/api/v1/device/name/countcamera1
And this is the point at which I get (as mentioned in previous comment) the following response:
http://:0/api/v1/device/name/countcamera1: dial tcp :0: connect: can't assign requested address

michaelestrin · 2020-01-21T11:20:59Z

And this is the point at which I get (as mentioned in previous comment) the following response:
http://:0/api/v1/device/name/countcamera1: dial tcp :0: connect: can't assign requested address

@akramtexas: this smacks of an uninitialized go-mod-core-contracts client (something @brandonforster is currently working on in edgexfoundry/go-mod-core-contracts#196). Unrelated to the underlying issue but something you'll need to understand (and overcome) to be able to reproduce the underlying issue.

akramtexas · 2020-01-21T12:59:48Z

@michaelestrin The input (in your comment above) is deeply appreciated 👍

akramtexas · 2020-01-21T14:25:53Z

Current status and learnings thus far:

First (and referring to the comment immediately above, which @michaelestrin kindly volunteered, and is much appreciated), I need to understand (unrelated to the underlying issue) the aspect of an uninitialized go-mod-core-contracts client.
As part of recreating the reported issue, I had started with creating reference information in EdgeX, making two calls to Core Metadata, to create the addressables for the Device Service.
TODO: Need to address the question of whether I actually have a process listening at the location specified by the addressables?
Regarding the above TODO, if there is no real device or device service, EdgeX doesn’t know that. But with the configuration and setup I have done, I should still be able to ask EdgeX to set the scan depth or set the snapshot duration to the camera, and have EdgeX attempt to perform the task (expecting EdgeX to ultimately responds with an error). So that is a start...
...and to follow up (and given that the addressable specifies the address of the device service managing the given devices, and it is not the address of the devices themselves), I could launch an instance of device-virtual-go, which would require:
(1) configuration.toml
to help with bootstrapping
(2) device-specific.yaml
to help with deviceResources
to help with deviceCommands
to help with coreCommands
Evidently, device-virtual-go currently does not support CBOR. It does support the following data formats:
Bool
Int8, Int16, Int32, Int64
Uint8, Uint16, Uint32, Uint64
Float32, Float64
As I understand it, here is the relevant PR open: (Support read operation of binary resource)
Again, as I understand it, that PR would enable Binary Data Ingestion via Core Data, which ties in with how Device Service creates event model representation (and the REST POST request is made specifying CBOR content type, with the request body being the whole event.)

akramtexas · 2020-01-21T16:35:17Z

I am now able to get past the (unrelated-but-impacting-the-recreation-of-this-issue) aspect tied in with the uninitialized go-mod-core-contracts client by (locally) building per PR-2313, which is a PR by @brandonforster that's currently under review.
Thus, the previously-failing step (i.e. the call issued to core-command regarding device countcamera1: http://localhost:48082/api/v1/device/name/countcamera1) now succeeds:
The next step I will now focus on is to look for a way to get (or create) a binary payload (to work with CBOR) and tweak device-virtual-go to use that payload (and thereby look at the interactions wherein @FelixTing come across the issue reported here.

akramtexas · 2020-01-21T22:58:39Z

I have a tweaked version of device-virtual-go that should be able to use a binary payload. With that (virtual) device service running, along with the core services, I can, for example, call the following core-metadata API: http://localhost:48081/api/v1/device - Response to the GET operation is as in following screenshot:
I initially leaned on a Dockerized version of (the tweaked) device-virtual-go, but concluded that it's just as easy (actually far simpler to work with) to just run all services natively (The example screenshot above is for all services running natively.)
Need to figure out why running core-command in Goland debugger-only is running into an (odd) issue when invoking the following API call: http://localhost:48082/api/v1/device/name/countcamera1. The goal is to step through the Go functions (in core-command that have been reported in this bug) and which I plan to test with a binary payload.

akramtexas · 2020-01-22T15:13:02Z

Squared away running the core-command service in debug-mode (Goland) without any issue.
Working forward from the test-data set up earlier, focus now is on creating a binary payload with which to test and recreate the reported scenario (regarding how the functions issueDeviceCommand and issueDeviceCommandByNames are still applying the application/json Content-Type header when the response body is a CBOR event.)

akramtexas · 2020-01-23T14:04:40Z

Tweaked the device.virtual.binary.yaml for the device service (device-virtual-go) with the goal of obtaining a binary payload to use for testing with this issue.
I confirmed that the device is registered and is among the devices known to Core Metadata.
By asking Core Command for the list of commands associated with the device (Random-Binary-Device), via a call to http://localhost:48082/api/v1/device/name/Random-Binary-Device, I obtained the following URL in particular: http://localhost:48082/api/v1/device/9b486859-1965-4719-a628-239196fb2bec/command/71433ddb-3e9f-47ff-b582-ba902609159b
Regarding the above, and based on the example in an EdgeX doc (Sending and Consuming Binary Data From EdgeX Device Services), invoking this URL returns a response that I think (I need to confirm this) is the CBOR representation of an event (I see that it is not human-readable).
I plan very shortly to dig into parsing CBOR-encoded events from the preceding CBOR representation (to understand better the underlying structure and make sure I am working toward the required binary payload).

akramtexas · 2020-01-23T18:57:32Z

I have succeeded in creating a binary payload, starting with a CBOR representation of an event.
Thus, in response to getting supported commands, via a call to Core Command at http://localhost:48082/api/v1/device/name/Random-Binary-Device), I found the needed path (in the "commands" element in the response for aforesaid API call. That path is: "path":"/api/v1/device/{deviceId}/Binary".
From there, I made a call (using my Device ID to populate the {deviceId} field) to Core Data at http://localhost:48080/api/v1/device/9b486859-1965-4719-a628-239196fb2bec/Binary.
The preceding API call resulted in the physical download of the event's CBOR representation, which I then ran through a small golang program that read in the CBOR data, and used a CBOR decoder to decode (the CBOR data) into raw, binary data, finally writing (that binary data) out to a file.
Next step: Using the newly-created binary payload to step through the Go functions (the ones in core-command that have been reported in this bug), seeing the interactions in conjunction with using the binary payload.

akramtexas · 2020-01-24T14:53:02Z

Gents @tsconn23 @michaelestrin, in sharing here an outline of the design choice(s) we have in order to address the reported behavior, I invite your thoughts in terms of preference on how to proceed:

At the level of edgex-go, we are currently hard-wired to use ContentTypeJSON, most pertinently to the behavior at hand in:
[1] func restGetCommandsByDeviceID(), as in w.Header().Set(clients.ContentType, clients.ContentTypeJSON)
[2] func issueDeviceCommandByNames() as in w.Header().Set(clients.ContentType, clients.ContentTypeJSON)
Short of (and this is clearly undesirable), and at the level of edgex-go, hard-wiring to use ContentTypeCBOR in the two functions above (instead of the currently hard-wired ContentTypeJSON).
Instead, we could conceivably make those calls contextual so they can respond with the appropriate Content-Type header (JSON, CBOR, etc.) - i.e. context to resolve would be CBOR vs JSON vs etc.
Thus, consider how, in response to requesting a list of supported commands, via a call to Core Command at http://localhost:48082/api/v1/device/name/Random-Binary-Device), we get in the response the following JSON sub-element nested under the "commands" element:

            "responses":[ 
               { 
                  "code":"200",
                  "expectedValues":[ 
                     "Binary"
                  ]
               },
               ...
               ...

At the level of edgex-go, in order to make those core-command calls contextual (so they can respond with the appropriate Content-Type header), we could introspect the command when the "context" gets invoked during their execution: i.e. Determine the appropriate Content-Type header (JSON, CBOR, etc.) by introspecting whether the context calls for CBOR, JSON, etc. That is, we look for the registered command.
At the level of device-virtual-go, the configuration (of the binary device) can either be of type "String" (which it currently is) or of type "Binary" (Referring here to the config in edgexfoundry/device-virtual-go/cmd/res/device.virtual.binary.yaml).
At the level of device-sdk-go, and tying in with the preceding observation, there currently does not appear to be any context for format encoding. I would need to confirm what the (default) format encoding is at the level of device-sdk-go. Related to this, there does not appear to be any config option in device-sdk-go for encoding the type, should we (separately) seek to make that configurable.

akramtexas · 2020-01-24T15:32:44Z

Adding some concrete details (screenshot below of stepping thru code) to what I had in mind when referring to making those core-command calls contextual so as to pick up the direction in which to respond, with the appropriate Content-Type header (JSON, CBOR, etc.):

michaelestrin · 2020-01-24T21:25:36Z

I'm not sure if this is an actual bug. It may be an enhancement request in disguise.

I'm not sure if the community agreed to implement CBOR for device commands.

I'm also not sure of the use case (though there must be one because this issue was opened).

We support CBOR for telemetry -- pushing binary events with readings from a device service through core, support, and appfunc services northward.

@tsconn23 @jpwhitemn did the community commit to implement CBOR on the command side?

tsconn23 · 2020-01-24T22:41:29Z

@FelixTing I have walked through this with Akram. There is a line in the Device SDK that calls this functionality out as a gap

https://github.com/edgexfoundry/device-sdk-go/blob/be9f41a4af74d5425ebf4b1f099130d6e4242ce6/internal/controller/restfuncs.go#L127

From this I understand your use case to be you are expecting a CBOR response to the REST client invoking a command via core-command that will return some kind of binary reading. Currently that functionality doesn't exist. It only sends an event of the appropriate encoding through core-data.

What use case are you implementing where you find that returning a CBOR response in this manner is useful? If you can provide more details, we can consider reclassifying this an an enhancement and potentially have Akram make the changes.

akramtexas · 2020-01-27T22:46:50Z

These are the results of stepping through and confirming that CBOR response is indeed being returned. Similarly, confirmed that core-command is handling CBOR.
Starting with the following GET command: http://localhost:48082/api/v1/device/48da70c2-8fc9-4b14-90ba-74640f92d48b/command/f24da50c-12d2-47df-a0f6-0919aaa4ba4a
I get CBOR-encoded event back:
I set the [Writable] LogLevel = 'DEBUG' to get more insight from logging:

level=DEBUG ts=2020-01-27T22:22:07.21532Z app=device-virtual source=utils.go:78 correlation-id=f2aec887-e2ed-45db-878b-ed7fadf0f043 msg="SendEvent: EventClient.MarshalEvent passed through encoded event"
level=INFO ts=2020-01-27T22:22:07.44027Z app=device-virtual source=utils.go:93 Content-Type=application/cbor correlation-id=f2aec887-e2ed-45db-878b-ed7fadf0f043 msg="SendEvent: Pushed event to core data"

Content-Type header, as we saw earlier, is:

(Reflecting the fact that the application/json Content-Type header is returned when the response body is a CBOR event.)

akramtexas · 2020-01-27T23:00:12Z

Adding that, as far as the PUT core-command is concerned, the func parseParams(){...}

func parseParams(params string) (paramMap map[string]string, err error) {
	err = json.Unmarshal([]byte(params), &paramMap)
...
...

currently unmarshals JSON only, with reference to the following call stack:

github.com/edgexfoundry/device-sdk-go/internal/handler.parseParams at command.go:410
github.com/edgexfoundry/device-sdk-go/internal/handler.parseWriteParams at command.go:359
github.com/edgexfoundry/device-sdk-go/internal/handler.execWriteCmd at command.go:311
github.com/edgexfoundry/device-sdk-go/internal/handler.CommandHandler at command.go:87
github.com/edgexfoundry/device-sdk-go/internal/controller.commandFunc at restfuncs.go:108
net/http.HandlerFunc.ServeHTTP at server.go:2007
github.com/edgexfoundry/device-sdk-go/internal/controller/correlation.OnRequestBegin.func1 at middleware.go:48
net/http.HandlerFunc.ServeHTTP at server.go:2007
github.com/edgexfoundry/device-sdk-go/internal/controller/correlation.OnResponseComplete.func1 at middleware.go:34
net/http.HandlerFunc.ServeHTTP at server.go:2007
github.com/edgexfoundry/device-sdk-go/internal/controller/correlation.ManageHeader.func1 at middleware.go:27
net/http.HandlerFunc.ServeHTTP at server.go:2007
github.com/gorilla/mux.(*Router).ServeHTTP at mux.go:162
net/http.serverHandler.ServeHTTP at server.go:2802
net/http.(*conn).serve at server.go:1890
runtime.goexit at asm_amd64.s:1357

akramtexas · 2020-01-28T16:55:50Z

This is regarding the scope of a possible solution, while investigating what it would take for Device SDK to have core-command process a CBOR-encoded event, treating it (properly) as data of application/cbor format. By tracing all the way through the Device SDK code, the Content-Type header is properly set to application/cbor and remains that way all the way to the MUX.
To compensate for that, one possibility is for core-command itself to infer that the command being executed is of type application/cbor, then that may be a way to infer the header's encoding type.
Thus, consider the expected value in the responses element when calling:

curl --location --request GET 'http://localhost:48082/api/v1/device/name/Random-Binary-Device' \
--data-raw ''

Where the relevant snippet in the response is:

    "commands": [
        {
            "created": 1580147003425,
            "modified": 1580147003425,
            "id": "f24da50c-12d2-47df-a0f6-0919aaa4ba4a",
            "name": "Binary",
            "get": {
                "path": "/api/v1/device/{deviceId}/Binary",
                "responses": [
                    {
                        "code": "200",
                        "expectedValues": [
                            "Binary"
                        ]
                    },

As far as tracing all the way through the Device SDK code itself, specifically around the area inside the func commandFunc(){...} where the following code-comment is located:

      // TODO: Resolve why this header is not included in response from Core-Command to originating caller (while the written body is).
      w.Header().Set(clients.ContentType, clients.ContentTypeCBOR)
      w.Write(event.EncodedEvent)
    } else {
      w.Header().Set(clients.ContentType, clients.ContentTypeJSON)
      json.NewEncoder(w).Encode(event)
    }
    // push to Core Data
    go common.SendEvent(event)
  }

The URL used is http://localhost:48082/api/v1/device/48da70c2-8fc9-4b14-90ba-74640f92d48b/command/f24da50c-12d2-47df-a0f6-0919aaa4ba4a
At the point where execution is exiting the function, where the goroutine is called (go common.SendEvent(event)), the Content-Type header has been properly set to application/cbor:

 w = {net/http.ResponseWriter} 
 *conn = {*net/http.conn} 
 *req = {*net/http.Request} 
...
...
 handlerHeader = {net/http.Header} 
  0 = Content-Type -> len:1, cap:1
   key = {string} "Content-Type"
   value = {[]string} len:1, cap:1
    0 = {string} "application/cbor"
 calledHeader = {bool} true

Where the logging confirms that the event push has taken place:

level=DEBUG ts=2020-01-28T16:07:26.69264Z app=device-virtual source=utils.go:78 correlation-id=102a2f66-fb31-4bda-8d8e-79fe9a28e787 msg="SendEvent: EventClient.MarshalEvent passed through encoded event"
level=INFO ts=2020-01-28T16:09:00.837645Z app=device-virtual source=utils.go:93 Content-Type=application/cbor correlation-id=102a2f66-fb31-4bda-8d8e-79fe9a28e787 msg="SendEvent: Pushed event to core data"

Further along in the execution, at the completion of the call to func OnResponseComplete() (in /device-sdk-go/internal/controller/correlation/middleware.go), the Content-Type header remains properly set to application/cbor:

w = {net/http.ResponseWriter} 
...
...
 handlerHeader = {net/http.Header} 
  0 = Content-Type -> len:1, cap:1
   key = {string} "Content-Type"
   value = {[]string} len:1, cap:1
    0 = {string} "application/cbor"
 calledHeader = {bool} true

Finally, farthest along in the execution, at the completion of the call to the Gorilla MUX, the Content-Type header remains properly set to application/cbor:

github.com/gorilla/mux.(*Router).ServeHTTP at mux.go:163
net/http.serverHandler.ServeHTTP at server.go:2802
net/http.(*conn).serve at server.go:1890
runtime.goexit at asm_amd64.s:1357

Where we have:

w = {net/http.ResponseWriter} 
 *conn = {*net/http.conn} 
 *req = {*net/http.Request} 
 reqBody = {io.ReadCloser} 
 cancelCtx = {context.CancelFunc} context.WithCancel.func1
 wroteHeader = {bool} true
 wroteContinue = {bool} false
 wants10KeepAlive = {bool} false
 wantsClose = {bool} false
 *w = {*bufio.Writer} 
 cw = {net/http.chunkWriter} 
 handlerHeader = {net/http.Header} 
  0 = Content-Type -> len:1, cap:1
   key = {string} "Content-Type"
   value = {[]string} len:1, cap:1
    0 = {string} "application/cbor"
 calledHeader = {bool} true

tsconn23 · 2020-01-28T20:43:03Z

@akramtexas
Regarding

To compensate for that, one possibility is for core-command itself to infer that the command being executed is of type application/cbor, then that may be a way to infer the header's encoding type.

We discussed on Friday 24-Jan that if the CBOR Content-Type header value is correctly coming back from the device service, which I understand that it is based on what you've written above, that it would be possible to modify core-command to

Check the header value on the response
If the value is CBOR then always converting the response body to a string is problematic.
We would need to float the raw bytes of the response up to an appropriate level where we make the determination of whether to convert to string. This decision is based on the Content-Type value, JSON vs CBOR
Since the execution of a core-command is effectively a pass-through, a JSON result can be passed back to the caller as a string. A CBOR result should be handed back as bytes. And the appropriate content-type needs to be sent to the caller as well.

akramtexas · 2020-01-28T23:05:17Z

@tsconn23 This is a great summary 👍 (I also really appreciate your pointing out in particular how the execution of a core-command function call is effectively a pass-through, with the allied implications.)

FelixTing · 2020-01-31T06:25:42Z

@akramtexas @tsconn23
My apologies for the belated reply. (I was on the Lunar new year vacation last few days.)

@akramtexas has reproduced what I encountered when trying to support read operation of binary resource in device-virtual-go:

Issue a GET command to retrieve a binary value
Get a CBOR encoded event
The content-Type header is application/json

Thanks for your help. I really appreciate it.

akramtexas · 2020-01-31T13:04:45Z

@FelixTing Welcome back from your Lunar calendar vacation. Thanks for confirming (using the link in your comment above) the specifics of the steps that lead to reported behavior. Is there anything else you would like to add in this connection, to elaborate what use case you are implementing, and where you find that returning a CBOR response in this manner is useful?

tsconn23 · 2020-01-31T15:42:02Z

@FelixTing We need to know the use case for the steps you've outlined above. How is this flow utilized in a customer oriented solution? What purpose does it solve? As described above this flow is not implemented and will be an enhancement. Understanding the customer use-case will help us to prioritize.

@iain-anderson @jpwhitemn for visibility

cloudxxx8 · 2020-02-02T14:10:28Z

@tsconn23 We don't have use cases so far. The purpose here is to implement the binary support in Virtual Device Service for user test, and we found out this limitation. We can tweak the Virtual Device Service to work only with the current supported behaviors on binary data.

michaelestrin · 2020-02-03T13:03:48Z

I've always viewed the command channel as a mechanism to actuate a device or provide an asynchronous prompt to a device service for it to take an action and return the relevant data via the normal manner (i.e. via core-data).

I may be misinterpreting; it sounds like you want to obtain data from a device service as a direct reply to a command request sent via core-command. If this is the case, this would effectively create a new data path within the system.

Given the design and intent of the system in general and core-command specifically, I'd be interested in @jpwhitemn's take on whether or not this is desirable.

The value of direct Core Command support comes into play when we need to handle event fusion, such as when sensors emit events that should trigger a command to pull nearly immediate binary data (such as an image snapshot from camera device in response to motion). This activity would likely be initiated by Rules Engine, so I suspect it may be more relevant/prioritized alongside other Rules Engine features.

@tobiasmo1 how would this work? In the scenario you describe, the rules engine -- based on a trigger -- would initiate a specific command to a specific device service to obtain some binary data. What would the rules engine to do with that binary data?

cloudxxx8 · 2020-02-03T15:46:12Z

A simple use case in my mind is that when a motion sensor detects something, Get commands would be invoked to retrieve videos or snapshots from the cameras in a specific area.

michaelestrin · 2020-02-03T16:07:20Z

A simple use case in my mind is that when a motion sensor detects something, Get commands would be invoked to retrieve videos or snapshots from the cameras in a specific area.

Why wouldn't the video/snapshot data be forwarded into core-data by the device service as usual?

cloudxxx8 · 2020-02-04T07:43:22Z

A simple use case in my mind is that when a motion sensor detects something, Get commands would be invoked to retrieve videos or snapshots from the cameras in a specific area.

Why wouldn't the video/snapshot data be forwarded into core-data by the device service as usual?

Since the binary data is large, users might not want to collect it constantly. Through rules engine and get command, users could retrieve the binary data when it is necessary.
@SteveOss @andyf1967 @iain-anderson could you please share your opinions or use cases you know?
Thanks for the effort, @michaelestrin @akramtexas @tsconn23 , we will implement the binary support in virtual device service with the current implementation first anyway.

michaelestrin · 2020-02-04T12:48:36Z

Since the binary data is large, users might not want to collect it constantly. Through rules engine and get command, users could retrieve the binary data when it is necessary.

I don't think I'm communicating clearly.

In your scenario, it could be the data is sent only when the rules engine triggers the command to instruct the device service to gather and send it. There's nothing about the existing data path that requires data to be sent constantly.

Assuming the rules engine -- based on a trigger -- initiated a specific command to a specific device service to obtain some binary data in-band, what would the rules engine to do with that binary data?

I'm still not clear on the value of creating a separate path (via core-command or otherwise) to receive and process data from device services.

cloudxxx8 · 2020-02-04T14:32:26Z

Why wouldn't the video/snapshot data be forwarded into core-data by the device service as usual?

I got your point now.
My understanding was that the usual way to send binary data to core-data is to use AutoEvent, and it will constantly send data to core-data.
It can work by ignoring the response received by the trigger.

iain-anderson · 2020-02-04T16:31:02Z

@SteveOss @andyf1967 @iain-anderson could you please share your opinions or use cases you know?

One suggestion that has been floated before is to allow control of the destinations for event data on a per-request basis by use of query parameters.
So the normal operation of the device service is to generate an event and post it to core-data, and to return the same event as the result of the http request (the latter then being passed-through by core-command). But either of these paths could be disabled, by appending something like ?ds-post-event=no or ?ds-return-event=no to your URL.
But this seems orthogonal to the issue here, which as I read it is that in the case of CBOR data the return path is being corrupted by using a wrong Content-Type

michaelestrin · 2020-02-04T16:41:07Z

But this seems orthogonal to the issue here, which as I read it is that in the case of CBOR data the return path is being corrupted by using a wrong Content-Type

I concur -- orthogonal.

the normal operation of the device service is to generate an event and post it to core-data, and to return the same event as the result of the http request (the latter then being passed-through by core-command)

With this additional clarity, it seems as if the initial CBOR implementation missed the real possibility of binary data being included in a command response. It sounds like this needs to be addressed. In your opinion (@jpwhitemn @iain-anderson ), is this a forward-facing change (i.e. in master only) or something we should fix in prior releases?

tobiasmo1 · 2020-02-04T19:26:14Z

“Why wouldn't the video/snapshot data be forwarded into core-data by the device service as usual? If there is no application subscribed to receive the data. I suspect most workflows will AutoEvent or manually trigger an event using a command. But this leaves the third flow we discussed and implemented; legacy command/response behavior that conforms to usual device service behavior for non-binary payloads. Assuming EdgeX will continue to support command/response for device services (to query and/or actuate a device), it is consistent to facilitate binary/CBOR data alongside other readings (numeric, strings, json, etc.). I suppose there are alternatives to routing the information back to the caller (e.g., using correlation ID, etc.), even to RESTful services that call a device service to “get” a response. I filed an issue during initial CBOR implementation that the command handling was “overloaded”. (ref: edgexfoundry/device-sdk-go#240). I also reported that Core Command wasn’t reflecting this Content Type header in the response; I don’t find a reference yet. Trevor may recall, I believe we decided to consider supporting this via Accept header. Toby From: Cloud Tsai <notifications@github.com> Sent: Monday, February 3, 2020 11:43 PM To: edgexfoundry/edgex-go <edgex-go@noreply.github.com> Cc: Mosby, Tobias <tobias.mosby@intel.com>; Mention <mention@noreply.github.com> Subject: Re: [edgexfoundry/edgex-go] Response of issueDeviceCommand is not providing CBOR Content-Type header (#2272) A simple use case in my mind is that when a motion sensor detects something, Get commands would be invoked to retrieve videos or snapshots from the cameras in a specific area. Why wouldn't the video/snapshot data be forwarded into core-data by the device service as usual? Since the binary data is large, users might not want to collect it constantly. Through rules engine and get command, users could retrieve the binary data when it is necessary. @SteveOss<https://github.com/SteveOss> @andyf1967<https://github.com/andyf1967> @iain-anderson<https://github.com/iain-anderson> could you please share your opinions or use cases you know? Thanks for the effort, @michaelestrin<https://github.com/michaelestrin> @akramtexas<https://github.com/akramtexas> @tsconn23<https://github.com/tsconn23> , we will implement the binary support in virtual device service with the current implementation first anyway. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#2272?email_source=notifications&email_token=AEAJEVKJSRGFJZ7HDG4YXKLRBEMBZA5CNFSM4KFC3UM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKWUTIQ#issuecomment-581781922>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEAJEVNPDH5NRJVM5HBUQ3TRBEMBZANCNFSM4KFC3UMQ>.

michaelestrin · 2020-02-13T16:14:21Z

Discussed in February 13th Core WG meeting; community decided to move ahead with correcting this in Geneva release. Re-categorized and assigned @akramtexas.

@tsconn23

akramtexas · 2020-02-17T13:35:41Z

Discussed in February 13th Core WG meeting; community decided to move ahead with correcting this in Geneva release. Re-categorized and assigned @akramtexas.

@tsconn23

@michaelestrin @tsconn23 I will work this for the Geneva release 👍

akramtexas · 2020-03-09T18:44:26Z

Following updates to restDevice.go (i.e. edgex-go/internal/core/command/restDevice.go) on a feature branch (WIP and for which a PR will be prepared and submitted), and for which unit testing has been created, a CBOR result is now handed back as bytes, and the appropriate content-type is sent to the caller as well. This complements the already-existing capability for handling a JSON result.
As for functional testing, I created another CBOR payload (much smaller than the one previously used, which was blowing the golang runtime stack, and for which an issue has been created).
Used this payload for the PUT command for core-command. Also tested with the GET commands for core-command. Using GoLand debugger, I traced through the command.go code (in device-sdk-go/internal/handler/) as well as the restfuncs.go code (in device-sdk-go/internal/controller/)
The need has been identified to update the following function in device-sdk-go, which is currently hard-coded to handle JSON only, and needs to have support added for handling CBOR as well:

func parseParams(params string) (paramMap map[string]string, err error) { ... }

However, for such an updated, we would also need to propagate the content type (JSON or CBOR) from the request header through a deep nesting of function calls:

github.com/edgexfoundry/device-sdk-go/internal/handler.parseParams at command.go:408
github.com/edgexfoundry/device-sdk-go/internal/handler.parseWriteParams at command.go:356
github.com/edgexfoundry/device-sdk-go/internal/handler.execWriteCmd at command.go:308
github.com/edgexfoundry/device-sdk-go/internal/handler.CommandHandler at command.go:87
github.com/edgexfoundry/device-sdk-go/internal/controller.commandFunc at restfuncs.go:108
net/http.HandlerFunc.ServeHTTP at server.go:2007

And which can be seen initiating with the call to the core-command function at the bottom of the call hierarchy below:

paramMap, err := parseParams(params)
in: func parseParams(params string) (paramMap map[string]string, err error) {

cvs, err := parseWriteParams(device.Profile.Name, ros, params)
in: func execWriteCmd(device *contract.Device, cmd string, params string) common.AppError {

appErr := execWriteCmd(&d, cmd, body)
in: func CommandHandler(vars map[string]string, body string, method string, queryParams string) (*dsModels.Event, common.AppError) {

event, appErr := handler.CommandHandler(vars, body, req.Method, req.URL.RawQuery)

func commandFunc(w http.ResponseWriter, req *http.Request) {

c.addReservedRoute(common.APIIdCommandRoute, commandFunc).Methods(http.MethodGet, http.MethodPut)
c.addReservedRoute(common.APINameCommandRoute, commandFunc).Methods(http.MethodGet, http.MethodPut)

After examining the edgex-go code-base, I see that a cleaner approach (which would avoid us having to propagate the content type from the request header through a deep nesting of function calls) could use (for core-command) the following function or something very similar, defined in edgex-go/internal/core/data/io.go:

// NewCborReader creates a new instance of cborReader.
func NewCborReader(configuration *config.ConfigurationStruct) cborReader {
  return cborReader{configuration: configuration}
}

// NewJsonReader creates a new instance of cborReader.
func NewJsonReader() jsonReader {
  return jsonReader{}
}

And which is used in the following switch statement, again in edgex-go/internal/core/data/io.go:

// NewRequestReader returns a BodyReader capable of processing the request body
func NewRequestReader(request *http.Request, configuration *config.ConfigurationStruct) EventReader {
  contentType := request.Header.Get(clients.ContentType)

  switch strings.ToLower(contentType) {
  case clients.ContentTypeCBOR:
    return NewCborReader(configuration)
  default:
    return NewJsonReader()
  }
}

And invoked in edgex-go/internal/core/data/router.go:
reader := NewRequestReader(r, configuration)

iain-anderson · 2020-03-10T10:15:39Z

Should we split out handling CBOR for write commands (enhancement) from this issue of missing content-type in result of read command (bug)

…2446) Fix edgexfoundry#2272 Update core-command to propagate `Content-type` header from device service when a GET command is executed. Previously this value was hard-coded to `application/json` and was ignoring the correct value that the device service was providing. Signed-off-by: Akram Ahmad <sftwr2020@gmail.com> Signed-off-by: Andre Srinivasan <andre@redislabs.com>

FelixTing added the bug Something isn't working label Jan 10, 2020

michaelestrin assigned akramtexas Jan 10, 2020

FelixTing mentioned this issue Jan 13, 2020

Support read operation of binary resource edgexfoundry/device-virtual-go#82

Closed

michaelestrin added geneva and removed unscoped Issues that are currently out of scope for all releases. labels Feb 13, 2020

michaelestrin assigned akramtexas Feb 13, 2020

michaelestrin added bug Something isn't working and removed enhancement New feature or request labels Feb 13, 2020

This was referenced Mar 20, 2020

Fix core-command response when handling a CBOR result. #2446

Merged

Update doc with steps for creating CBOR payload to use with PUT commands. edgexfoundry/edgex-docs#118

Merged

AnthonyMBonafide mentioned this issue Apr 9, 2020

Add Support for handling CBOR content for commands edgexfoundry/device-sdk-go#479

Closed

akramtexas mentioned this issue Apr 16, 2020

Handle both JSON request bodies as well as CBOR request bodies. edgexfoundry/device-sdk-go#488

Open

AnthonyMBonafide closed this as completed in f24d90c Apr 16, 2020

Response of issueDeviceCommand is not providing CBOR Content-Type header #2272

Response of issueDeviceCommand is not providing CBOR Content-Type header #2272

Comments

FelixTing commented Jan 10, 2020

🐞 Bug Report

Affected Services

Is this a regression?

Description and Minimal Reproduction

akramtexas commented Jan 20, 2020

akramtexas commented Jan 20, 2020

michaelestrin commented Jan 21, 2020

akramtexas commented Jan 21, 2020

akramtexas commented Jan 21, 2020

akramtexas commented Jan 21, 2020

akramtexas commented Jan 21, 2020

akramtexas commented Jan 22, 2020

akramtexas commented Jan 23, 2020 • edited Loading

akramtexas commented Jan 23, 2020

akramtexas commented Jan 24, 2020

akramtexas commented Jan 24, 2020

michaelestrin commented Jan 24, 2020

tsconn23 commented Jan 24, 2020

akramtexas commented Jan 27, 2020 • edited Loading

akramtexas commented Jan 27, 2020 • edited Loading

akramtexas commented Jan 28, 2020 • edited Loading

tsconn23 commented Jan 28, 2020

akramtexas commented Jan 28, 2020

FelixTing commented Jan 31, 2020

akramtexas commented Jan 31, 2020

tsconn23 commented Jan 31, 2020

cloudxxx8 commented Feb 2, 2020

michaelestrin commented Feb 3, 2020

cloudxxx8 commented Feb 3, 2020

michaelestrin commented Feb 3, 2020

cloudxxx8 commented Feb 4, 2020

michaelestrin commented Feb 4, 2020

cloudxxx8 commented Feb 4, 2020

iain-anderson commented Feb 4, 2020

michaelestrin commented Feb 4, 2020

tobiasmo1 commented Feb 4, 2020 via email

michaelestrin commented Feb 13, 2020 • edited Loading

akramtexas commented Feb 17, 2020

akramtexas commented Mar 9, 2020

iain-anderson commented Mar 10, 2020

akramtexas commented Jan 23, 2020 •

edited

Loading

akramtexas commented Jan 27, 2020 •

edited

Loading

akramtexas commented Jan 27, 2020 •

edited

Loading

akramtexas commented Jan 28, 2020 •

edited

Loading

michaelestrin commented Feb 13, 2020 •

edited

Loading