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

feat: Add ADR for device functions #644

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs_src/design/TOC.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@
| [0018 Service Registry](./adr/0018-Service-Registry.md) | Service registry usage for EdgeX services |
| [0019 EdgeX-CLI V2](./adr/core/0019-EdgeX-CLI-V2.md) | EdgeX-CLI V2 Implementation |
| [0020 Delay start services (SPIFFE/SPIRE)](./adr/security/0020-spiffe.md) | Secret store tokens for delayed start services |
| [0021 Invoking Device Functions](./adr/device-service/0021-invoking-functions.md) | RPC-like actions for devices |
112 changes: 112 additions & 0 deletions docs_src/design/adr/device-service/0021-invoking-functions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# Invoking Device Functions


## Status

**Draft**

## Context
This ADR presents a mechanism for invoking functions on a device.

## Existing Behavior

Device access in EdgeX is focussed on 'Readings' and 'Settings'. A device may
present values which are readable, representing some measurement or status,
such as a temperature or vibration reading. Also a setting such as a the
required speed of a fan motor may be written to and read back.

What is not supported directly is 'Functions', where a device may be commanded
to do something. Examples could include perform a self-test, go into standby
mode for an hour, invalidate an access key.

While such operations may modelled using virtual resources in a device profile,
this is unintuitive.

## Decision

**Add a new section to device profiles describing functions**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd to describe functions in device profiles. Typically what we're trying to accomplish is allow use of some sort of functional protocol to communicate with devices, and the protocol itself doesn't change for different instances of the class of devices supported by the device service. For instance, the original device-camera service only supports ONVIF (which is a SOAP-based protocol), likewise our RFID device service only supports LLRP. I suppose if a service supported more than one function-based protocol, then defining the functions in profiles makes sense, but I think this is the exception, not the norm.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes for protocols like ONVIF which define available operations at the protocol level. But for eg a Modbus device where you may have a function "Reset latching alarms" which is invoked by writing a zero into register 0x16, that ought to be defined in a device profile.


```
{
"deviceFunctions":
[
{
"name": "Name by which the function is accessed",
"description": "Readable description of the function",
"attributes": { device-service-specific attributes which select this function },
"parameters":
[
{
"name": "Parameter name",
"description": "(optional) description of what the parameter controls",
"type": "Any of the usual EdgeX data types",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this include array, binary, and object types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see why not

"defaultValue": "(optional) value to use if param is not supplied",
"maximum": "(optional) for numerics, maximum allowed value",
"minimum": "(optional) for numerics, minimum allowed value"
}
],
"returnValues":
[
{
"name": "Name of returned value",
"description": "(optional) description of what the value indicates",
"type": "Any of the usual EdgeX data types"
}
]
}
]
}
```

Note: the `attributes` structure is analagous to `attributes` in a `deviceResource`. Each device service should document and implement a scheme of required attributes that will allow for selection of the relevant funtion.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the purpose of attributes are in this context? If extra data is required to make a function call, isn't this the purpose of parameters? I also don't understand what you mean by "a scheme of required attributes that will allow for selection of relevant function"?

sp: funtion

I


**Define MessageBus topics on which function call requests and replies are to be made**

`edgex/function-calls/device/[profile-name]/[device-name]/[function-name]`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why include the profile name? It's not included as a parameter in the standard core/device services command API.

Also would it make sense to model our topic names using the conventions we use for REST which prefix parameters with a name (e.g. edgex/function-calls/device/[device-name]/function/[function-name])?

Should the topic also somehow indicate that this message is destined for core command and/or a specific device instance?

Are these functions meant to proxied by core command (per the North/South Messaging ADR)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have changed these to reflect the content of the N/S messaging ADR


The payload for messages on these topics should be of the form
```
{
requestId: "184b894f-a7b7-4d6c-b400-99961d462419",
parameters: { (a map of parameter values keyed by parameter name) }
}
```
Copy link
Collaborator

@bnevis-i bnevis-i Dec 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can use an existing specification and add the requestId to it?

https://en.wikipedia.org/wiki/JSON-RPC#Version_2.0
https://www.jsonrpc.org/specification

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1000000

We should at least explore prior art before we invent something new...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have changed this so that it is using the same style of message body as proposed for N/S messaging


The `requestId` may be any string but UUIDs are recommended.

`edgex/function-responses/device/[profile-name]/[device-name]/[function-name]`

The device service will provide responses to function calls on this topic. The payload will be
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are messages sent with guaranteed delivery, or can messages be thrown away if the device is not there to receive them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QoS is configurable in the main messagebus options


```
{
requestId: "184b894f-a7b7-4d6c-b400-99961d462419",
status: 0,
returnValues: { (a map of return values keyed by value name) }
}
```

or if a call fails
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if a response is never sent or received?


```
{
requestId: "184b894f-a7b7-4d6c-b400-99961d462419",
status: (nonzero),
errorMessage "Message indicating the nature of the failure"
}
```

*Returned status codes*

| Status | Meaning
|--------|--------
| 0 | The operation was successful
| 1 | Parameters were missing, out of range or non-parsable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this include other errors with the request format?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a separate code for other format errors

| 2 | The Device is DOWN or DISABLED (OperatingState / AdminState)
| 3 | No such device or function
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto. Who is implementing the dead letter functionality?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added text on the SDKs responsibilities

| 100+ | Implementation-specific errors, defined for each Device Service

** Device SDK enhancement **

The device SDKs will handle the messagebus communcations and parameter marshalling. The generic errors defined above may be detected in this SDK code. The SDKs will define APIs for the individual device services to implement the function invocations.