Re: [PATCH 8/8] api: Let MM_MODEM_MODE be a bitfield, and new PreferredMode property

[Dan Williams <dcbw redhat com>]

On Fri, 2011-09-30 at 15:01 +0200, Aleksander Morgado wrote:
> Supported and Allowed modes are modified to be bitmasks of MM_MODEM_MODE values,
> and preference of a specific mode is now given in the new PreferredMode
> property and as an extra argument to the SetAllowedModes() call.
> 
>  * Supported Modes: bitmask specifying which modes are supported by the specific
> hardware. For example, a modem may only support 1G/2G/3G connections (not 4G).
> 
>  * Allowed Modes: bitmask specifying which modes, of the ones Supported by the
> modem, are allowed to use. For example, a modem may support 1G/2G/3G connections
> but only 1G and 2G connections are allowed by the user as 3G involves more
> expensive data rates.
> 
>  [Allowed] ⊆ [Supported]
> 
>  * Preferred Mode: specific mode which is preferred among the ones defined in
> the Allowed modes bitmask. For example, a modem may allow 1G/2G/3G connections
> but the user would like that if possible 2G be used, as 3G consumes too much
> battery. If 2G is not possible, 3G can be used.
> 
>  [Preferred] ∈ [Allowed]

I don't have a huge objection to this, but I'm not sure I see the
benefit of having the Preferred/Allowed split versus the complexity.
Basically, if Allowed were an enum where we enumerated the preference
there are 4 items to choose from (4G, 3G, 2G, empty) and 3 slots in the
preference order (since empty doesn't get a slot, just a single enum).
Thats a total of 25 combinations, but some like 2G>4G don't really make
sense, so we have somewhere under 25.  32-bits gives us a lot of range
there if it's an enum not a bitfield.  The downside is that it has no
relationship with the MM_MODEM_MODE flags.  My worry is just that it's
added complexity (3 properties to check instead of 2) that may be just a
bit more work for clients.

Dan

> ---
>  new/org.freedesktop.ModemManager1.Modem.xml |   56 ++++++++++++++-------------
>  1 files changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/new/org.freedesktop.ModemManager1.Modem.xml b/new/org.freedesktop.ModemManager1.Modem.xml
> index 55a52aa..8da5680 100644
> --- a/new/org.freedesktop.ModemManager1.Modem.xml
> +++ b/new/org.freedesktop.ModemManager1.Modem.xml
> @@ -111,11 +111,16 @@
>        </tp:docstring>
>        <annotation name="org.freedesktop.DBus.GLib.Async" value=""/>
>        <annotation name="org.freedesktop.DBus.GLib.CSymbol" value="impl_modem_set_allowed_mode"/>
> -      <arg name="mode" type="u" tp:type="MM_MODEM_MODE">
> +      <arg name="allowed" type="u" tp:type="MM_MODEM_MODE">
>          <tp:docstring>
>            Bitmask of all the modes allowed in the modem.
>          </tp:docstring>
>        </arg>
> +      <arg name="preferred" type="u" tp:type="MM_MODEM_MODE">
> +        <tp:docstring>
> +	  Specific mode preferred among the ones allowed, if any.
> +        </tp:docstring>
> +      </arg>
>      </method>
>  
>      <method name="SetAllowedBands">
> @@ -296,6 +301,13 @@
>        </tp:docstring>
>      </property>
>  
> +    <property name="PreferredMode" type="u" access="read" tp:type="MM_MODEM_MODE">
> +      <tp:docstring>
> +        The preferred access technology (eg 2G/3G/4G), among the ones defined in
> +        the allowed modes. Only one or none values must be given.
> +      </tp:docstring>
> +    </property>
> +
>      <property name="SupportedModes" type="u" access="read" tp:type="MM_MODEM_MODE">
>        <tp:docstring>
>          Access technology selection modes supported by the device.  For POTS
> @@ -492,38 +504,28 @@
>  
>      <tp:enum name="MM_MODEM_MODE" type="u">
>        <tp:docstring>
> -        Describes the device's current access mode preference; ie the specific
> -        technology preferences the device is allowed to use when connecting to
> -        a network.  Also used as a bitfield to indicate which allowed modes
> -        the modem supports when setting the mode preference.
> +        Bitfield to indicate which access modes are supported, allowed or
> +        preferred in a given device.
>        </tp:docstring>
> -      <tp:enumvalue suffix="ANY" value="0x0000">
> -	<tp:docstring>
> -	  Any mode can be used (only this value allowed for POTS modems)
> -	</tp:docstring>
> -      </tp:enumvalue>
> -      <tp:enumvalue suffix="2G_PREFERRED" value="0x0001">
> -	<tp:docstring>Prefer 2G (GPRS or EDGE)</tp:docstring>
> -      </tp:enumvalue>
> -      <tp:enumvalue suffix="3G_PREFERRED" value="0x0002">
> -	<tp:docstring>Prefer 3G (UMTS or HSxPA)</tp:docstring>
> +      <tp:enumvalue suffix="NONE" value="0x00">
> +	<tp:docstring>None</tp:docstring>
>        </tp:enumvalue>
> -      <tp:enumvalue suffix="4G_PREFERRED" value="0x0004">
> -	<tp:docstring>Prefer 4G (LTE)</tp:docstring>
> +      <tp:enumvalue suffix="1G" value="0x01">
> +        <tp:docstring>CSD, GSM</tp:docstring>
>        </tp:enumvalue>
> -      <tp:enumvalue suffix="2G_ONLY" value="0x0100">
> -	<tp:docstring>Use only 2G (GPRS or EDGE)</tp:docstring>
> +      <tp:enumvalue suffix="2G" value="0x02">
> +        <tp:docstring>GPRS, EDGE</tp:docstring>
>        </tp:enumvalue>
> -      <tp:enumvalue suffix="3G_ONLY" value="0x0200">
> -	<tp:docstring>Use only 3G (UMTS or HSxPA)</tp:docstring>
> +      <tp:enumvalue suffix="3G" value="0x04">
> +        <tp:docstring>UMTS, HSxPA</tp:docstring>
>        </tp:enumvalue>
> -      <tp:enumvalue suffix="4G_ONLY" value="0x0400">
> -	<tp:docstring>Use only 4G (LTE)</tp:docstring>
> +      <tp:enumvalue suffix="4G" value="0x08">
> +        <tp:docstring>LTE</tp:docstring>
>        </tp:enumvalue>
> -      <!-- FIXME: what about 3G/4G only? or 2G/3G only?  Should this be a
> -           bitfield?  If so, do we need to indicate what combinations the
> -           modem supports?
> -        -->
> +      <tp:enumvalue suffix="ANY" value="0xFFFFFFFF">
> +	<tp:docstring>
> +	  Any mode can be used (only this value allowed for POTS modems)
> +	</tp:docstring>
>      </tp:enum>
>  
>      <tp:flags name="MM_MODEM_BAND" value-prefix="MM_MODEM_BAND" type="u">