-
Notifications
You must be signed in to change notification settings - Fork 3
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
writeProperty message #38
base: master
Are you sure you want to change the base?
Conversation
Note that an error message can be sent in response if the Thing definitely knows the write was unsuccessful (I'm going to add error conditions to the readProperty and writeProperty in a follow-up PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. The only comment is about always confirming a writeProperty. I prefer to see a confirmation regardless whether it is readonly or not.
</p> | ||
<p class="note" title="Write-only Properties">In some cases it may not be possible for an underlying | ||
implementation to confirm that a value has been written. The <code>writeOnly</code> member of a | ||
PropertyAffordance being set to <code>true</code> may hint that this is the case. For this reason, a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me these are two different things. Write-only means that the value of the property cannot be read. The ability to confirm that a write was completed is a separate concern.
If the underlying hardware offers no such confirmation then a 'completed' response would mean that the write was applied. I would still like to see a confirmation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that a property that can't be read and a write that can't be confirmed could be argued to be subtly different things. However, unconfirmable writes do exist.
For example, in the X10 communication protocol some devices do not acknowledge the receipt of commands. You can send a command to switch a device on, but there's no way to tell whether that command was received.
My concern would be that if a Web Thing sends a confirmation message when a write may not actually have been completed or received, it may be misleading.
As far as I know a Thing Description has no way to distinguish a confirmable writeproperty
operation from a non-confirmable one (other than by using the writeOnly
member of a PropertyAffordance).
Therefore my suggestion would be that a Web Thing that can not confirm whether a write operation has succeeded (for whatever reason) should use the writeOnly
member as a hint, and a Consumer should not treat a lack of a response as an error condition in that case. An error message could be sent in response if the write is definitely known to have failed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I would really like to differentiate between propertyReading
and an acknowledgment that the write request or invokeAction request was processed using the best available method. Even if a write operation cannot be confirmed, the Thing provider should still be able to acknowledge that the request was received and acted upon to the best of its ability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's a need for a message type to acknowledge a propertyWrite
message without providing a property reading, then yes it does seem like that would need to be separate message type from propertyReading
. Adding a special "propertyWriteAcknowledgement" message type feels wrong though, so if this really is important then maybe this is further evidence for the need to split out operation name and message type.
I've filed #42 to explore this further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read through the proposal and I also find this point very sensitive. My main concern his how a Consumer should behave when writing a property. If the response "acknowledge" (either we decide to use propertyReading
or dedicated ack
) MIGHT arrive, then the best behaviour is to never wait for something, but rather request again a property read (if not write-only). If we don't have the ability to specify if a property write is acknowledgable by the TD, I think the default should be "there is no acknowledge".
I would explore the possibility to introduce the term acknowledge: true
as a custom protocol binding keyword.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My vote goes to Ben's proposal in #42:
Response
{
"thingID": "https://mythingserver.com/things/mylamp1",
"messageID": "79057736-3e0e-4dc3-b139-a33051901ee2",
"messageType": "response",
"operation": "writeproperty",
"name": "on",
"value": true,
"correlationID": "5afb752f-8be0-4a3c-8108-1327a6009cbd"
}
Note: To address the issue raised in https://github.com/w3c/web-thing-protocol/pull/38 about non-confirmable writes, a value could only be provided in a response if it is confirmed to have been written.
This addresses the concern I raised as there is always a response. The response can carry a value if the output is available, no value if the property is writeonly, or an error if it fails. This is deterministic and simple. The Affordance already states whether the property is writeonly so there is no guesswork involved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't catch up with the discussion. Then I definitely support the direction above!
OK, I will create a PR to split |
This PR defines a
writeProperty
message format.It is intended to fulfil requirement 5.2.2.2 from the Use Cases & Requirements document.
I think the only potentially controversial point with this PR might be the note regarding
writeOnly
properties which establishes that awriteProperty
message may not always receive a response.Let me know what you think.
Preview | Diff