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.

Sign up for GitHub

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

Extended return type of invokeAction() #561

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Extended return type of invokeAction() #561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ab29ca
refactor: start experimenting with extending invokeAction
danielpeintner Aug 26, 2024
b73fdb1
refactor: improve initial text
danielpeintner Aug 26, 2024
bb69456
refactor: remove ActionStatus "pending"
danielpeintner Oct 14, 2024
54a2f50
Update index.html
danielpeintner Oct 15, 2024
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
69 changes: 66 additions & 3 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,70 @@ <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 a consumer implementation
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}}.
Copy link
Member

Choose a reason for hiding this comment

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

  • This **MAY** not offer is likely to be confused with **MAY NOT** offer, despite formatting, casing, etc.
  • Further, I'm not sure what sensible is contributing here.
  • I'm also left wondering whether there should be some indication of what other interactions (sensible or not) might be offered (if the action does offer more than those in {{InteractionOutput}})?

Perhaps this rephrasing is acceptable, for the first two points above? Something additional might be needed for the last.

Suggested change
Note: a synchronous action MAY not offer any <emph>additional</emph> sensible interaction beyond the ones from {{InteractionOutput}}.
Note: A synchronous action MAY offer only the interactions found in {{InteractionOutput}} and no others.

Copy link
Member

@JKRhb JKRhb Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The 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
Note: a synchronous action MAY not offer any <emph>additional</emph> sensible interaction beyond the ones from {{InteractionOutput}}.
Note: The output of a synchronous action MAY be limited to the functionality of a regular {{InteractionOutput}} object, e.g., invoking the `cancel()` method might not have an effect.

</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 {
"running",
"success",
"error"
};

[SecureContext, Exposed=(Window,Worker)]
interface ActionInteractionOutput : InteractionOutput {
readonly attribute object? error;
Promise&lt;ActionStatus&gt; status();
Promise&lt;undefined&gt; 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?">
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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..

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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...

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 status would be more powerful and useful. What do people think? Other ideas?

Copy link
Member

Choose a reason for hiding this comment

The 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 status would be more powerful and useful. What do people think? Other ideas?

Hmm, if I see it correctly, we could also treat the attribute as a getter and add more logic to that if needed, right?

Or could a script subscribe just for changes in status?

If we wanted to that, what would be the best way to model that? Adding a way for registering a callback function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mhh, maybe we should decide what we need/want from the status

  • is it likely that requesting the status may take a while? (or put differently do we need a Promise)
  • do we need a way to observe a status change (would be nice but adds complexity)

My take is that having a promise function for status() is fine while I am not 100% sure we should allow for listening status changes...

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:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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:
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:

<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
Expand Down Expand Up @@ -1323,7 +1386,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">
Expand Down Expand Up @@ -1386,7 +1449,7 @@ <h2>The <dfn>ConsumedThing</dfn> interface</h2>
/*Promise&lt;undefined&gt; writeAllProperties(
PropertyWriteMap valueMap,
optional InteractionOptions options = {});*/
Promise&lt;InteractionOutput&gt; invokeAction(DOMString actionName,
Promise&lt;ActionInteractionOutput&gt; invokeAction(DOMString actionName,
optional InteractionInput params = {},
optional InteractionOptions options = {});
Promise&lt;Subscription&gt; observeProperty(DOMString name,
Expand Down
12 changes: 10 additions & 2 deletions typescript/scripting-api/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ declare namespace WoT {
value(): Promise<DataSchemaValue>;
}

export enum ActionStatus { "running", "success", "error" }

export interface ActionInteractionOutput extends InteractionOutput {
error?: Error;
status(): Promise<ActionStatus>
cancel(): Promise<void>
}

export interface Subscription {
active:boolean,
stop(options?: InteractionOptions):Promise<void>
Expand Down Expand Up @@ -140,9 +148,9 @@ declare namespace WoT {
* Makes a request for invoking an Action and return the result.
* Takes as arguments actionName, optionally params and optionally options.
* It returns a Promise that resolves with the result of the Action represented
* as an InteractionOutput object, or rejects with an error.
* as an ActionInteractionOutput object, or rejects with an error.
*/
invokeAction(actionName: string, params?: InteractionInput, options?: InteractionOptions): Promise<undefined | InteractionOutput>;
invokeAction(actionName: string, params?: InteractionInput, options?: InteractionOptions): Promise<undefined | ActionInteractionOutput>;

/**
* Makes a request for Property value change notifications.
Expand Down