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

Add gamepad light indicator extension #143

Open
wants to merge 14 commits into
base: gh-pages
Choose a base branch
from
+116 −0
Open

Add gamepad light indicator extension #143

Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e6cfd46
Add gamepad light indicator extension
fernando-sony Oct 29, 2020
2a7d14d
Update extensions.html
fernando-80 Oct 30, 2020
16fb917
Merge branch 'gh-pages' into light-indicator
fernando-80 Oct 30, 2020
2044611
Update extensions.html
fernando-80 Oct 30, 2020
56a5576
Merge branch 'gh-pages' into light-indicator
fernando-80 Jan 12, 2021
2c92bb9
Update extensions.html
fernando-80 Mar 2, 2021
e8f4dd0
Update extensions.html
fernando-80 Mar 2, 2021
12c2ca7
Update extensions.html
fernando-80 Mar 2, 2021
d988007
Update extensions.html
fernando-80 Mar 2, 2021
e839fae
Update extensions.html
fernando-80 Mar 2, 2021
af0ddeb
Update extensions.html
fernando-80 Mar 2, 2021
de4aade
Update extensions.html
fernando-80 Mar 2, 2021
d7c79f6
Update extensions.html
fernando-80 Mar 2, 2021
313072d
Apply suggestions from code review
marcoscaceres Apr 8, 2021
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
117 changes: 117 additions & 0 deletions extensions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,117 @@ <h2><dfn>GamepadPose</dfn> Interface</h2>
</dd>
</dl>
</section>

<section>
<h2><dfn>GamepadLightIndicatorType</dfn> Enum</h2>
<p>
This enum defines the type of light indicators supported by gamepads.
</p>

<pre class="idl">
enum GamepadLightIndicatorType {
"on-off",
"rgb"
};
</pre>

<dl data-dfn-for="GamepadLightIndicatorType">
<dt>"<dfn>on-off</dfn>"</dt>
<dd>
Light indicator that supports a single color.
</dd>

<dt>"<dfn>rgb</dfn>"</dt>
<dd>
Light indicator that supports multiple colors.
</dd>
</dl>
</section>

<section>
<h2><dfn>GamepadLightColor</dfn></h2>
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved
<p>
This represents the color of a light indicator. The default value should
be set to 0 for red, green, and blue.
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved
</p>

<pre class="idl">
dictionary GamepadLightColor {
Copy link
Member

Choose a reason for hiding this comment

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

We should investigate what existing color objects we could use from the platform here.

required octet red;
required octet green;
required octet blue;
};
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved
</pre>

<dl data-dfn-for="GamepadLightColor">
<dt><dfn>red</dt></dfn>
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved
<dd>
Red component of the light color, or non-zero value for an
<code>on-off</code> light indicator that indicates ON.
</dd>

<dt><dfn>green</dt></dfn>
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved
<dd>
Blue component of the light color, or non-zero value for an
<code>on-off</code> light indicator that indicates ON.
</dd>

<dt><dfn>blue</dt></dfn>
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved
<dd>
Green component of the light color, or non-zero value for an
<code>on-off</code> light indicator that indicates ON.
</dd>
</dl>
</section>

<section>
<h2><dfn>GamepadLightIndicator</dfn></h2>
<p>
This interface defines a light indicator.
</p>

<pre class="idl">
[Exposed=Window, SecureContext]
interface GamepadLightIndicator {
readonly attribute GamepadLightIndicatorType type;

Promise&lt;long&gt; setColor(GamepadLightColor color);
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved
};
</pre>

<dl data-dfn-for="GamepadLightIndicator">
<dt><dfn>type</dt></dfn>
<dd>
Type of light indicator supported by the gamepad.
</dd>

<dt><dfn>setColor</dt></dfn>
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved
<dd>
<p>
Sets the color of a light indicator, or sets the light indicator to ON
or OFF.
</p>

<p>
For <code>on-off</code> light indicators, one or more non zero values
denotes the light indicator is ON, whereas a zero value for all
components indicates the light indicator is OFF.
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved
</p>

<p>
For RGB light indicators, one or more non zero values specify the
color of the light indicator, whereas all zero values indicate the
light indicator is OFF.
</p>

<p>
The returned Promise will resolve <code>true</code> when setting the
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if this can be guaranteed to succeed. @nondebug, do you have an opinion about this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We typically don't get a signal that the color was changed successfully so we shouldn't claim that in the spec. I think we should reword this so we resolve when the LED color change command is sent and reject if the command couldn't be sent.

Copy link
Member

@marcoscaceres marcoscaceres Mar 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, so yeah... it shouldn't resolve with true either.

We probably need to rewrite this whole section. I'd suggest having a look at:
https://w3c.github.io/web-share/#share-method

Basically, we need to do a few checks that can cause the promise to reject... the controller has been disconnected/no longer available, etc. There might be a SecurityError if the permission has been changed before .setColor() is called. Also, what should happen if:

while (true) {
   x.setColor(randomRGB());
}

Rate limit? Drop requests on the floor?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We discussed this and couldn't come up with any compelling reasons to keep the Promise. It's unlikely that we'll receive any error indication from the device and there isn't any way we can lose permission for a Gamepad. Let's remove the promise and treat setColor as fire-and-forget.

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
The returned Promise will resolve <code>true</code> when setting the
The returned Promise will resolve `undefined` when setting the

color has succeeded, and will reject the device API error code on
failure.
Comment on lines +441 to +442
Copy link
Member

Choose a reason for hiding this comment

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

This needs to reject with a an actual DOM Exception of some kind. I think this is overly broad/underspecified tho...

</p>
</dd>
</dl>
</section>

<section data-dfn-for="GamepadTouch">
<h2><dfn>GamepadTouch</dfn> Interface</h2>
Expand Down Expand Up @@ -397,6 +508,7 @@ <h2>Partial <dfn>Gamepad</dfn> Interface</h2>
readonly attribute GamepadHand hand;
readonly attribute FrozenArray&lt;GamepadHapticActuator> hapticActuators;
readonly attribute GamepadPose? pose;
readonly attribute FrozenArray&lt;GamepadLightIndicator>? lightIndicators;
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved
readonly attribute FrozenArray&lt;GamepadTouch>? touchEvents;
};
</pre>
Expand All @@ -423,6 +535,11 @@ <h2>Partial <dfn>Gamepad</dfn> Interface</h2>
<code>null</code>.
</dd>

<dt><dfn>lightIndicators</dfn></dt>
<dd>
A list of all light indicators in the gamepad. <code>null</code> if
the gamepad does not have or does not support a light indicator.
fernando-80 marked this conversation as resolved.
Show resolved Hide resolved

<dt><dfn>touchEvents</dfn></dt>
<dd>
A list of touch events generated from all touch surfaces. <code>null</code>
Expand Down