[RFC] protocol: Extend wayland seat with interfaces for sensor inputs.

[David Herrmann]

Hi

On Thu, Aug 1, 2013 at 10:39 AM, Stefan Schmidt <s.schmidt at samsung.com> wrote:
> Treating some specific sensors as input devices. In particular
> adding support for wl_compass, wl_gyroscope and wl_accelerometer here.
>
> We have requests to start and stop sensor event receiving as well as
> events to receive the different axis values for these sensors.
>
> Signed-off-by: Stefan Schmidt <s.schmidt at samsung.com>
> ---
>  protocol/wayland.xml |  123 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 123 insertions(+)
>
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index 3cfa953..066a718 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -1251,6 +1251,9 @@
>        <entry name="pointer" value="1" summary="The seat has pointer devices"/>
>        <entry name="keyboard" value="2" summary="The seat has one or more keyboards"/>
>        <entry name="touch" value="4" summary="The seat has touch devices"/>
> +      <entry name="compass" value="8" summary="The seat has compass devices"/>
> +      <entry name="gyroscope" value="16" summary="The seat has gyroscope devices"/>
> +      <entry name="accelerometer" value="32" summary="The seat has accelerometer devices"/>

If we add all kinds of devices here, we will very soon run out of
space for 32bit bitfields (which I think wayland uses for "uint"). I
don't have a solution, but we should keep this in mind.

>      </enum>
>
>      <event name="capabilities">
> @@ -1297,6 +1300,39 @@
>
>      <!-- Version 2 of additions -->
>
> +    <request name="get_compass" since="2">
> +      <description summary="return compass object">
> +        The ID provided will be initialized to the wl_compass interface
> +       for this seat.
> +
> +        This request only takes effect if the seat has the compass
> +        capability.
> +      </description>
> +      <arg name="id" type="new_id" interface="wl_compass"/>
> +    </request>
> +
> +    <request name="get_gyroscope" since="2">
> +      <description summary="return gyroscope object">
> +        The ID provided will be initialized to the wl_gyroscope interface
> +       for this seat.
> +
> +        This request only takes effect if the seat has the gyroscope
> +        capability.
> +      </description>
> +      <arg name="id" type="new_id" interface="wl_gyroscope"/>
> +    </request>
> +
> +    <request name="get_accelerometer" since="2">
> +      <description summary="return accelerometer object">
> +        The ID provided will be initialized to the wl_accelerometer interface
> +       for this seat.
> +
> +        This request only takes effect if the seat has the accelerometer
> +        capability.
> +      </description>
> +      <arg name="id" type="new_id" interface="wl_accelerometer"/>
> +    </request>
> +

This interface limits each seat to one device of each. For keyboards
or mouse we internally merge these into a single virtual device, but
we cannot do this for accelerometers or gyros. We had a discussion
about that recently, I am not sure about the conclusion, though. If
there was one, please include it in the commit message.

Besides, why are these devices bound to seats? Where are these devices
supposed to be? An accelerometer can be attached to an input device,
an HDD, a laptop, a tablet, ... If you look at this from a
smartphone/tablet view, it is obvious. But for desktops it really
isn't clear whose data an accelerometer device reports.
I guess these devices are meant to be attached to the input device
that this seat/user uses. In this case it makes sense to allow only a
single device and bind them to seats. But please clarify this in the
description. No one cares if the description is 1000 words long, so
feel free to be verbose.

>      <event name="name" since="2">
>        <description summary="unique identifier for this seat">
>         In a multiseat configuration this can be used by the client to help
> @@ -1605,6 +1641,93 @@
>      </event>
>    </interface>
>
> +  <interface name="wl_compass" version="2">
> +    <description summary="compass sensor device">
> +      The wl_compass interface represents a compass
> +      associated with a seat.
> +    </description>
> +
> +    <request name="start">
> +      <description summary="Start receiving events">
> +       Sent to enable event receiving for the compass sensor.
> +      </description>
> +    </request>
> +
> +    <request name="stop">
> +      <description summary="Stop receiving events">
> +       Sent to disable event receiving for the compass sensor.
> +      </description>
> +    </request>

So this is a shared device. Please note that down. A compositor has to
broadcast events to all interested clients, otherwise this API doesn't
work (or is inconsistent). Did you think of the implications of that?
Might be fine, but I don't see any discussion, so please elaborate.
What are the reasons to allow background applications to access
compass/accelerometer/gyro? That is, why didn't you follow the
enter/leave API?

I don't want change your API, but the short descriptions give me the
impression that you didn't think about these implications so I'd like
to see some explanations. And the longer the descriptions, the less
bikeshed you get.. as usual.

This actually poses a security risk, please have a look at:
  http://arstechnica.com/apple/2011/10/researchers-can-keylog-your-pc-using-your-iphones-accelerometer/

> +
> +    <event name="motion">
> +      <description summary="Motion event coming from the compass sensor">
> +       Updated sensor data available from compass
> +      </description>
> +      <arg name="x" type="fixed" summary="x-axis"/>
> +      <arg name="y" type="fixed" summary="y-axis"/>
> +      <arg name="z" type="fixed" summary="z-axis"/>
> +    </event>

I never worked with compass devices on linux and it isn't obvious to
me what these 3 axes represent? An explanation would really help here.
But maybe I am just missing some context..

> +
> +  </interface>
> +
> +  <interface name="wl_gyroscope" version="2">
> +    <description summary="gyroscope sensor device">
> +      The wl_gyroscope interface represents a gyroscope
> +      associated with a seat.
> +    </description>
> +
> +    <request name="start">
> +      <description summary="Start receiving events">
> +       Sent to enable event receiving for the gyroscope sensor.
> +      </description>
> +    </request>
> +
> +    <request name="stop">
> +      <description summary="Stop receiving events">
> +       Sent to disable event receiving for the gyroscope sensor.
> +      </description>
> +    </request>
> +
> +    <event name="motion">
> +      <description summary="Motion event coming from the gyroscope sensor">
> +       Updated sensor data available from gyroscope
> +      </description>
> +      <arg name="x" type="fixed" summary="x-axis"/>
> +      <arg name="y" type="fixed" summary="y-axis"/>
> +      <arg name="z" type="fixed" summary="z-axis"/>
> +    </event>

Here it is obvious what the 3 axes mean, but we should clarify that
these are randomly chosen. At least I am not aware of any definitions
which axes x, y and z are (for instance regarding the yaw/pitch/roll
definitions).

> +
> +  </interface>
> +
> +  <interface name="wl_accelerometer" version="2">
> +    <description summary="accelerometer sensor device">
> +      The wl_accelerometer interface represents a accelerometer
> +      associated with a seat.
> +    </description>
> +
> +    <request name="start">
> +      <description summary="Start receiving events">
> +       Sent to enable event receiving for the accelerometer sensor.
> +      </description>
> +    </request>
> +
> +    <request name="stop">
> +      <description summary="Stop receiving events">
> +       Sent to disable event receiving for the accelerometer sensor.
> +      </description>
> +    </request>
> +
> +    <event name="motion">
> +      <description summary="Motion event coming from the accelerometer sensor">
> +       Updated sensor data available from accelerometer
> +      </description>
> +      <arg name="x" type="fixed" summary="x-axis"/>
> +      <arg name="y" type="fixed" summary="y-axis"/>
> +      <arg name="z" type="fixed" summary="z-axis"/>
> +    </event>

Again, I think these are randomly chosen, right?

(copied from your reply)
> We started with many more sensors but brought it down to these three
> as they seem to make the most sense. I kept the proposal small
> regarding exposed requests and events to not over-engineer it from the beginning.

Would you mind listing some more devices? Just to get a feeling what
this discussion was all about.

I like this interface a lot. As I see it, it addresses the needs of
user-input. That is, it's targeted at games or apps for orientation.
It's not targeted at background daemons to protect your hard-drive.
Wasn't obvious to me right from the beginning, but maybe it should
have been? A short comment wouldn't hurt, anyway.

That said, it makes a lot of sense to provide this data attached to a
seat. We see a seat as a way to identify a user interacting with a
system. So we only stuff devices in a single seat that a user can use
in parallel. We identify the user this way. So it's immediately clear
what an compass attached to a seat reports. The orientation of the
user.
However, this API is strictly bound to smartphone/tablet input. For
instance what happens if the user has one input device in the left and
one in the right hand? Both might report movement data, but we
couldn't report it via this API because it assumes a single input
device. And this isn't even some fancy look into the future. With
Wii-Remotes we already have such input which can be greatly used. Just
thinking about it... But if we design this API in a way that multiple
devices can be attached to a seat but explicitly limit it to 1 device
in this revision, then we can extend this API in the future if we want
to support such setups. We might even tack your feet.. or head.. or..
ok, stop me!

All in all nice to see this being worked on. Thanks!

Cheers
David

> +
> +  </interface>
> +
>    <interface name="wl_output" version="2">
>      <description summary="compositor output region">
>        An output describes part of the compositor geometry.  The
> --
> 1.7.9.5
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel