-
Notifications
You must be signed in to change notification settings - Fork 28
Extended return type of invokeAction() #561
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
8ab29ca
b73fdb1
bb69456
54a2f50
c711244
63e84c8
4b40048
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1275,7 +1275,71 @@ <h2>The <dfn>InteractionOutput</dfn> interface</h2> | |||||||||
</ol> | ||||||||||
</section> | ||||||||||
</section> | ||||||||||
<section> <h3>Using {{InteractionInput}} and {{InteractionOutput}}</h3> | ||||||||||
|
||||||||||
<section data-dfn-for="ActionInteractionOutput"> | ||||||||||
<h2>The <dfn>ActionInteractionOutput</dfn> interface</h2> | ||||||||||
<p> | ||||||||||
Belongs to the <a>WoT Consumer</a> conformance class. | ||||||||||
An {{ActionInteractionOutput}} object is always created by the implementations | ||||||||||
and exposes functionality to interact with long running (asynchronous) actions.<br /> | ||||||||||
Note: a synchronous action MAY not offer any <emph>additional</emph> sensible interaction beyond the ones from {{InteractionOutput}}. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Perhaps this rephrasing is acceptable, for the first two points above? Something additional might be needed for the last.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tried to cover the third aspect that you've mentioned by rephrasing the sentence as follows – hope that it is a bit clearer than it was before:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not against it, if its accurate, but this revision (which would cover all of my bullets) seems to have a very different meaning than the original. |
||||||||||
</p> | ||||||||||
<p> | ||||||||||
This interface exposes an action status object. Its implementation | ||||||||||
will allow cancelling asynchronous actions and report the status of a long running action. | ||||||||||
</p> | ||||||||||
<pre class="idl"> | ||||||||||
enum ActionStatus { | ||||||||||
/* "pending", Profile has pending. Not sure what it would mean? */ | ||||||||||
danielpeintner marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
"running", | ||||||||||
"success", /* Profile uses completed? */ | ||||||||||
"error" /* Profile uses failed? */ | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we coordinate in some way with the Profile TF to align the terms that are being used? Or is it no problem if they differ, since they are operating on a different level anyway? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Once we feel comfortable with our choices we should definitely reach out. Most of the status terms are 1:1 matches. What seems different is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: Profile has 4 terms
What can we do with aligning the terms? Use the ones from profile or talk to them? Any opinion? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, their terminology actually sounds quite good to me, so from my side we could also adopt what they are using. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am fine withe the Profile terms also. The question remains if it is fine dropping "pending" then... (same same but different 🙃) |
||||||||||
}; | ||||||||||
|
||||||||||
[SecureContext, Exposed=(Window,Worker)] | ||||||||||
interface ActionInteractionOutput : InteractionOutput { | ||||||||||
readonly attribute object? error; | ||||||||||
Promise<ActionStatus> status(); | ||||||||||
Promise<undefined> cancel(); | ||||||||||
}; | ||||||||||
</pre> | ||||||||||
<p> | ||||||||||
The <dfn>error</dfn> property represents a possible error, initially `null`. | ||||||||||
</p> | ||||||||||
<p class="ednote" title="Should state be a function or an attribute only?"> | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started with a function but I start thinking an attribute should be sufficient.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An action is supposed to be always polled? Or could a script subscribe just for changes in status? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think observing state changes would be nice but I am not sure whether that is actually necessary... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mhh, I tend to think now that a a function for reporting the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hmm, if I see it correctly, we could also treat the attribute as a getter and add more logic to that if needed, right?
If we wanted to that, what would be the best way to model that? Adding a way for registering a callback function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mhh, maybe we should decide what we need/want from the status
My take is that having a promise function for Are there opinions/proposals? |
||||||||||
|
||||||||||
</p> | ||||||||||
<p class="ednote" title="Additional action object functions needed?"> | ||||||||||
Should we allow pause/resume also? TD has no notion of it. | ||||||||||
</p> | ||||||||||
<section><h3>The <dfn>status()</dfn> function</h3> | ||||||||||
Reports the status of an <a>Action</a> (one of <code>running</code>, <code>success</code> or <code>error</code>) or rejects on error. The method MUST run the following steps: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
<ol> | ||||||||||
<li> | ||||||||||
Return a {{Promise}} |promise:Promise| and execute the next steps | ||||||||||
[=in parallel=]. | ||||||||||
</li> | ||||||||||
<li> | ||||||||||
TODO | ||||||||||
</li> | ||||||||||
</ol> | ||||||||||
</section> | ||||||||||
<section><h3>The <dfn>cancel()</dfn> function</h3> | ||||||||||
Cancels a running WoT <a>Action</a> or rejects on error. The method MUST run the following steps: | ||||||||||
<ol> | ||||||||||
<li> | ||||||||||
Return a {{Promise}} |promise:Promise| and execute the next steps | ||||||||||
[=in parallel=]. | ||||||||||
</li> | ||||||||||
<li> | ||||||||||
TODO ... applicable when the state is running | ||||||||||
</li> | ||||||||||
</ol> | ||||||||||
</section> | ||||||||||
|
||||||||||
</section> | ||||||||||
|
||||||||||
<section> <h3>Using {{InteractionInput}}, {{InteractionOutput}} and {{ActionInteractionOutput}}</h3> | ||||||||||
<p> | ||||||||||
As illustrated in the next pictures, the {{InteractionOutput}} interface | ||||||||||
is used every time implementations provide data to scripts, while | ||||||||||
|
@@ -1323,7 +1387,7 @@ <h2>The <dfn>InteractionOutput</dfn> interface</h2> | |||||||||
<p> | ||||||||||
When a {{ConsumedThing}} invokes an <a>Action</a>, it provides the | ||||||||||
parameters as {{InteractionInput}} and receives the output of the | ||||||||||
<a>Action</a> as an {{InteractionOutput}} object. | ||||||||||
<a>Action</a> as an {{ActionInteractionOutput}} object. | ||||||||||
</p> | ||||||||||
<p> | ||||||||||
An {{ExposedThing}} <a href="#the-actionhandler-callback"> | ||||||||||
|
@@ -1386,7 +1450,7 @@ <h2>The <dfn>ConsumedThing</dfn> interface</h2> | |||||||||
/*Promise<undefined> writeAllProperties( | ||||||||||
PropertyWriteMap valueMap, | ||||||||||
optional InteractionOptions options = {});*/ | ||||||||||
Promise<InteractionOutput> invokeAction(DOMString actionName, | ||||||||||
Promise<ActionInteractionOutput> invokeAction(DOMString actionName, | ||||||||||
optional InteractionInput params = {}, | ||||||||||
optional InteractionOptions options = {}); | ||||||||||
Promise<Subscription> observeProperty(DOMString name, | ||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.