[PATCH weston-ivi-shell v4 4/9] A reference protocol of ivi hmi controller to set up IVI style UI.

[Pekka Paalanen]

On Mon, 17 Mar 2014 15:27:46 +0900
Nobuhiko Tanibata <NOBUHIKO_TANIBATA at xddp.denso.co.jp> wrote:

> The reference protocol is used between hmi-controller and
> hmi-controller-homescreen.

A one paragraph explanation on what hmi-controller and
hmi-controller-homescreen each are would be nice, maybe added to the
XML itself.

I'm a bit lost here now.

> 
> Signed-off-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA at xddp.denso.co.jp>
> ---
> 
> Changes for v2:
>  - squash Makefile to this patch
> 
> Changes for v3 and v4
>   - nothing. Version number aligned to the first patch
> 
>  protocol/Makefile.am            |   3 +-
>  protocol/ivi-hmi-controller.xml | 105 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 107 insertions(+), 1 deletion(-)
>  create mode 100644 protocol/ivi-hmi-controller.xml
> 
> diff --git a/protocol/Makefile.am b/protocol/Makefile.am
> index 9913f16..140aef5 100644
> --- a/protocol/Makefile.am
> +++ b/protocol/Makefile.am
> @@ -9,7 +9,8 @@ protocol_sources =				\
>  	wayland-test.xml			\
>  	xdg-shell.xml				\
>  	scaler.xml                              \
> -	ivi-application.xml
> +	ivi-application.xml			\
> +	ivi-hmi-controller.xml
>  
>  if HAVE_XMLLINT
>  .PHONY: validate
> diff --git a/protocol/ivi-hmi-controller.xml b/protocol/ivi-hmi-controller.xml
> new file mode 100644
> index 0000000..04e22f4
> --- /dev/null
> +++ b/protocol/ivi-hmi-controller.xml
> @@ -0,0 +1,105 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="ivi_hmi_controller">
> +
> +    <copyright>
> +    Copyright (C) 2013 DENSO CORPORATION
> +
> +    Permission is hereby granted, free of charge, to any person obtaining a copy
> +    of this software and associated documentation files (the "Software"), to deal
> +    in the Software without restriction, including without limitation the rights
> +    to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> +    copies of the Software, and to permit persons to whom the Software is
> +    furnished to do so, subject to the following conditions:
> +
> +    The above copyright notice and this permission notice shall be included in
> +    all copies or substantial portions of the Software.
> +
> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> +    AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> +    OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> +    THE SOFTWARE.
> +    </copyright>
> +
> +    <interface name="ivi_hmi_controller" version="1">
> +        <description summary="set up and control IVI style UI">
> +            Currently it's possible to set up surface for background, panel,
> +            buttons, and workspace.
> +        </description>
> +
> +        <request name="set_background">
> +            <description summary="set surface drawing background by surface ID"/>
> +            <arg name="srf_id" type="uint"/>

It is highly unusual for a Wayland extension work by ids like this. Is
the intention that one client creates the surface with a known
surface_id, and then another client using this interface says, that
that surface_id is actually a background?

Weren't the surface_ids determined in advance during the system design
phase, and then hardcoded constants in the software? Wouldn't
ivi-shell.so just read these constants from a file, or even hardcode
them?

That is to say, I do not understand the purpose of this interface just
by looking at the protocol specification. I feel the specification
should explain it.

If this is an attempt to replicate some features of the desktop_shell
extension, why not just use wl_surface instead of uint?

> +        </request>
> +
> +        <request name="set_panel">
> +            <description summary="set surface drawing panel by surface ID"/>
> +            <arg name="srf_id" type="uint"/>

What happens, if several set_* requests are made with the same srf_id?
What if they are different set_* requests with the same srf_id?
Does srf_id need to have assigned a surface already, or can it be done
later?

> +        </request>
> +
> +        <request name="set_button">
> +            <description summary="set surface drawing button by surface ID">
> +                Several buttons are regitered on panel by using arg; number.
> +            </description>
> +            <arg name="srf_id" type="uint"/>
> +            <arg name="number" type="uint"/>

What if the 'number' was already used?
What does 'number' mean, anyway?
What if a client uses an arbitrarily large or zero 'number'?

> +        </request>
> +
> +        <request name="set_home_button">
> +            <description summary="set surface drawing home button by surface ID"/>
> +            <arg name="srf_id" type="uint"/>
> +        </request>
> +
> +        <request name="set_workspacebackground">
> +            <description summary="set surface drawing background of workspace by surface ID"/>
> +            <arg name="srf_id" type="uint"/>

How do you know what size each surface should be?
Would you ever want to have output-specific assignments, like a
different background on each output?

> +        </request>
> +
> +        <request name="add_launchers">
> +            <description summary="set a list of surface drawing launchers by a list of surface ID">
> +                Per calling this request, a group of launchers are added.
> +            </description>
> +            <arg name="srf_ids" type="array"/>
> +            <arg name="icon_size" type="uint"/>

What is icon_size doing here?
Is it a request for whatever is providing the surfaces associated to
srf_ids, that they should be of this size? Or is it a request for the
compositor to scale all these surfaces to that size?

> +        </request>
> +
> +        <request name="workspace_control">
> +            <description summary="start controlling workspace by server">
> +                Give seat to the server to be controlled by server side.
> +            </description>
> +            <arg name="seat" type="object" interface="wl_seat"/>
> +            <arg name="serial" type="uint"/>

I cannot even guess what this does.

> +        </request>
> +
> +        <enum name="layout_mode">
> +            <entry name="tiling" value="0"/>
> +            <entry name="side_by_side" value="1"/>
> +            <entry name="full_screen" value="2"/>
> +            <entry name="random" value="3" />
> +        </enum>
> +
> +        <request name="switch_mode">
> +            <description summary="request mode switch of application layout"/>
> +            <arg name="layout_mode" type="uint"/>
> +        </request>
> +
> +        <enum name="home">
> +            <entry name="off" value="0"/>
> +            <entry name="on" value="1"/>
> +        </enum>
> +
> +        <request name="home">
> +            <description summary="request showing workspace or disable"/>
> +            <arg name="home" type="uint"/>

home? workspace? disable? ???

> +        </request>
> +
> +        <event name="workspace_end_control">
> +            <description summary="notify controlling workspace end"/>
> +            <arg name="is_controlled" type="int"/>

What?

> +        </event>
> +
> +    </interface>
> +
> +</protocol>

It seems I am missing the big picture here, the general idea of how
things should conceptually work. I think that could be part of the
extension's introduction documentation.


Thanks,
pq