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

Able to send query parameters in GET command requests to devices #1564

Closed
pregit opened this issue Jul 18, 2019 · 9 comments
Closed

Able to send query parameters in GET command requests to devices #1564

pregit opened this issue Jul 18, 2019 · 9 comments
Labels
Milestone

Comments

@pregit
Copy link

pregit commented Jul 18, 2019

🚀 Feature Request

Relevant Package

core-command

Description

Should be able to pass query parameters in command GET requests to devices.
Example: "/api/v1/device/{deviceID}/command/{commandID}?param1=value&param2=value"

@pregit pregit added the enhancement New feature or request label Jul 18, 2019
@lenny-goodell
Copy link
Member

FYI, @rsdmike and I did a quick POC to see what the API impact would be. This can be accomplished without breaking REST API. Device SDK would have to pass the query parameters to HandleReadCommands without changing the function signature, so we put the the query parameters in CommandRequest.Attributes. @rsdmike will submit the PR when he returns from vacation next week.

@tsconn23
Copy link
Member

What is a practical example for param1=value&param2=value that is not satisfied by the current API?

@pregit
Copy link
Author

pregit commented Jul 19, 2019

command: "get_inventory" where facility=backstock

@tsconn23
Copy link
Member

tsconn23 commented Jul 19, 2019

command: "get_inventory" where facility=backstock

That's not enough context to understand your use case. What do inventory and backstock mean in the case of an IOT solution? What is "facility"?

@antoniomtz
Copy link

antoniomtz commented Jul 19, 2019

Hello,

To expand more on this feature request. We have a device that manages multiple sensors and exposes methods through jsonrpc . We were able to register some of those methods that don't require parameters as EdgeX commands and using the mqtt-device-service to communicate with the device.

For example:

Request:

{
 "jsonrpc" : "2.0",
 "method" : "sensor_get_device_ids"
}

Response:

{
 "jsonrpc" : "2.0",
 "result" : [ "sensor001", "sensor002"]
}

Using the current EdgeX Commands API, we are able to execute such command by making a request to the registered GET command endpoint:

http://edgex-core-command:48082/api/v1/device/{deviceID}/command/{commandID}

However, some of the exposed methods from our device require parameters, for example:
Request:

{
 "jsonrpc" : "2.0",
 "method" : "sensor_get_basic_info",
 "params" : {
   "device_id" : "sensor001"
  }
 }

Response:

{
 "jsonrpc" : "2.0",
 "result" : {
    "device_id" : "sensor001",
    "status": "something",
    "x" : "y"
 }

And for @pregit example, the device supports that method as well:
Request:

{
 "jsonrpc" : "2.0",
 "method" : "get_inventory",
 "params" : {
   facility" : "backstock"
 }
}

To give you a more general practical example, let's say you have an audio sensor with multiple microphones attached to it in different rooms, and that sensor exposes a GET command which is:

GET /microphones?microphoneID={id}&status={value}

This cannot be accomplished by the current EdgeX commands API since we are not able to pass either URL params or Query params to the Edgex GET command endpoints. Only PUT command endpoints support params in the body, but they do not return a response back which is the expected behavior.

Our request is to support user input params to GET command endpoints. As @lenny-intel mentioned, they did a quick proof of concept to accomplish this behavior.

Thanks,
Antonio

@iain-anderson
Copy link
Member

I think the main impact of allowing parameters in the device get requests is that the readings that are sent out to core-data and the rest of EdgeX become decontextualized. We'd have to at least add the parameters to the Event structure (maybe Reading?) and the export mechanisms would have to be aware of them?

@tonyespy
Copy link
Member

I think the idea here is that these query parameters are being used to provide hints as to how a device service should gather readings. For instance if we decide to actually implement a readings cache in the SDKs, a query parameter could be used to force the DS to bypass the cache. @tobiasmo1 and I also discussed query parameters that would control whether a DS sends a response back to a caller or just returns a status code. Likewise you could also envision a GET call w/a query parameter that informed the DS not to push the result to Core Data. In these cases, the parameters wouldn't be passed on to the ProtocolDriver, but instead handled by the SDK itself.

That said, in the use case put forth by @antoniomtz makes sense as well. They essentially want to support a command which requires multiple parameters and returns the Events/Readings as a response to the caller like would happen with a GET command.

I suppose one other way to fix this would be to add PUT command support for query parameter which triggered an actual response body to be returned to the caller like what happens with GET commands.

@iain-anderson
Copy link
Member

The examples seem to support selecting different devices ("device_id", "microphone_id") or selecting different information from a device ("facility"). This will mean that the information attached to a reading (device id, resource name) isn't going to fully identify the source. Is there any way we can extend our modelling of devices and the resources attached to them to accommodate these scenarios ?

rsdmike added a commit to rsdmike/edgex-go that referenced this issue Aug 5, 2019
This pull request is an example/POC that @lenny-intel and I put together of how edgexfoundry#1564 could be accomplished. This is only 1/2 of the equation as the device-sdk would need to be updated to handle the query parameters. The good thing with this is that it wouldn't be a breaking change as the device-sdk would simply ignore the query parameters that are sent on.

Addresses edgexfoundry#1564

Signed-off-by: Mike Johanson <michael.johanson@intel.com>
jim-wang-intel pushed a commit to jim-wang-intel/edgex-go that referenced this issue Aug 8, 2019
…ces (edgexfoundry#1571)

Fix edgexfoundry#1564

This pull request is an example/POC that @lenny-intel and I put together of how edgexfoundry#1564 could be accomplished. This is only 1/2 of the equation as the device-sdk would need to be updated to handle the query parameters. The good thing with this is that it wouldn't be a breaking change as the device-sdk would simply ignore the query parameters that are sent on.

Signed-off-by: Mike Johanson <michael.johanson@intel.com>
@iain-anderson
Copy link
Member

note for the record that we agreed in the device services wg meeting of 5th August that this would be implemented, that the device services would handle the query parameters by passing all of them as a single additional attribute to the device driver, and that query parameters in PUT requests would not be permitted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants