Proposed change to ProtocolDriver.HandleOperation about device-sdk-go HOT 5 CLOSED

@iain-anderson Thanks for filing this issue! I'm still not convinced that we need to have two methods for handling ResourceOperations in the ProtocolDriver interface.

That said, the original design followed the Java implementation a bit too closely, and thus there was room for improvement. I've re-factored and re-named HandleOperations (although it may make sense to revert back to the original name, pluralized). Instead of executeCommand method calling HandleOperation for each ResourceOperation associated with device/command, I know build a slice of CommandRequest (good suggestion) structs and pass all at once to the ProtocolDriver, which builds a slice of CommandResults and returns synchronously. As each HTTP call to the REST endpoint is done in a separate goroutine, this should work just fine. I've removed a few parameters, as everything that's needed should be available from the ResourceOperation, DeviceObject, and Device. Please take a look at the WIP commit and let me know what you think.

from device-sdk-go.

I just made a minor change to drop models.Device from CommandRequest, as the device doesn't change for each ResourceOperation.

Further suggestions include:

• switching CommandRequest back to a ResourceOperation, and making the driver either keep a local copy of DeviceObjects, or always ask the Service for them by DevProfileName

• switching the models.Device parameter to a device.Name, again if the driver needs anything additional in the Device struct, it should ask the Service for it. The driver could for instance keep a map of device name to device profile (if multiple device profiles are supported).

• and finally, we should get the naming right. Should everything be Operations instead of Commands to better match the internal models (eg. OperationResult, HandleOperations)?

from device-sdk-go.

@tonyespy These changes look good to me so far. I'd be okay to stay with "Commands" in the naming as that is the context of what we are trying to implement.
I think if there is a possibility that the full device / deviceobject structures are going to be useful to an implementor we should continue to pass them down. Happy to look at this again in a few days when we have more implementation experience.
What I would suggest is that we do some processing of the command payload within the SDK. We ought to be able to pull individual values out of the JSON and pass those in the CommandResult.

from device-sdk-go.

@iain-anderson Thanks for the comment!

Re: the full device / deviceobject, let's break them down separately:

• Device - although there's only one passed per HandleCommands call, it still a large struct (note, it includes the entire DeviceProfile) which is passed by value. I think the DS implementation will be mostly interested in the DeviceObject, which contains the ValueProperty and Attributes (used to map the edgex device resource to the protocol specific require via uuid, protocol type, ...). I think we could get away with just passing device name, if we modify ProtocolDriver.Initialize() to take a devices.Service pointer. This way if the driver needs the full device, it can ask the Service for it. That said, if you really think it makes sense to pass the entire struct (vs. just the name), I'll leave it as is.

• DeviceObject - let's leave as is for now.

Re: the params, I think that's a good idea, but as we don't actually have POST/actuation commands working, let's leave params as is for now, and make the change when we implement POST.

from device-sdk-go.

• Device - I think if we just pass the Addressable this should contain what the protocol driver will need.

from device-sdk-go.