Re: [MM 0.6] Plumb up the SPN display-rule bits



On Thu, 2012-03-01 at 14:28 +0100, Aleksander Morgado wrote:
> Hey,
> 
> > This is required for properly displaying
> > local-operator-vs-home-provider information, per 3GPP
> > (31.102 section 4.1.2 on EF_SPN and 22.101 Annex A).
> > 
> 
> For anyone else checking, that's section 4.2.11 in release 11 of the
> document.
> 
> > Suggestions welcome on the long and somewhat unwieldy property/function names.
> > 
> >     - Nathan
> > 
> > 
> > 0001-Retrieve-and-plumb-up-the-SIM-SPN-bits-that-set-oper.patch
> > 
> > 
> > From 96ce78202344bd928bf602874a087a0adb383cbe Mon Sep 17 00:00:00 2001
> > From: Nathan Williams <njw chromium org>
> > Date: Wed, 29 Feb 2012 16:25:03 -0500
> > Subject: [PATCH] Retrieve and plumb up the SIM SPN bits that set operator
> >  name display rules.
> > 
> > BUG=chromium-os:21522
> > TEST=Insert SIMs from various operators (T-Mobile, MaxROAM), run mmcli -i 0
> > Varying conditions include no SPN (AT&T), all-FF SPN data (3 Italy),
> > SPN with neither display bit set (3 UK), SPN with a display bit set (MaxROAM)
> > 
> > Change-Id: Id83da0e094efe52922dcc21f1420d3f5db5883e3
> > ---
> >  cli/mmcli-sim.c                                    |   14 ++++--
> >  .../org.freedesktop.ModemManager1.Sim.xml          |   17 +++++++
> >  libmm-glib/mm-sim.c                                |   30 ++++++++++++
> >  libmm-glib/mm-sim.h                                |    2 +
> >  src/mm-sim.c                                       |   49 ++++++++++++++++++-
> >  src/mm-sim.h                                       |    2 +
> >  6 files changed, 106 insertions(+), 8 deletions(-)
> > 
> > diff --git a/cli/mmcli-sim.c b/cli/mmcli-sim.c
> > index aa39a91..89ceeed 100644
> > --- a/cli/mmcli-sim.c
> > +++ b/cli/mmcli-sim.c
> > @@ -161,14 +161,18 @@ print_sim_info (MMSim *sim)
> >      g_print ("SIM '%s'\n",
> >               mm_sim_get_path (sim));
> >      g_print ("  -------------------------\n"
> > -             "  Properties |          imsi : '%s'\n"
> > -             "             |            id : '%s'\n"
> > -             "             |   operator id : '%s'\n"
> > -             "             | operator name : '%s'\n",
> > +             "  Properties |                 imsi : '%s'\n"
> > +             "             |                   id : '%s'\n"
> > +             "             |          operator id : '%s'\n"
> > +             "             |        operator name : '%s'\n"
> > +             "             |    show PLMN at home : %s\n"
> > +             "             | show op name roaming : %s\n",
> >               VALIDATE (mm_sim_get_imsi (sim)),
> >               VALIDATE (mm_sim_get_identifier (sim)),
> >               VALIDATE (mm_sim_get_operator_identifier (sim)),
> > -             VALIDATE (mm_sim_get_operator_name (sim)));
> > +             VALIDATE (mm_sim_get_operator_name (sim)),
> > +             mm_sim_get_display_registered_network_name_at_home (sim) ? "yes" : "no",
> > +             mm_sim_get_display_operator_name_while_roaming (sim) ? "yes" : "no");
> >  }
> >  
> >  static void
> > diff --git a/introspection/org.freedesktop.ModemManager1.Sim.xml b/introspection/org.freedesktop.ModemManager1.Sim.xml
> > index 14f9077..5833838 100644
> > --- a/introspection/org.freedesktop.ModemManager1.Sim.xml
> > +++ b/introspection/org.freedesktop.ModemManager1.Sim.xml
> > @@ -99,5 +99,22 @@
> >      -->
> >      <property name="OperatorName" type="s" access="read" />
> >  
> > +    <!--
> > +       DisplayRegisteredNetworkNameAtHome
> > +
> > +       If true, the user interface is required to display the name of
> > +       the registered PLMN while registered on a home network (non-roaming).
> > +    -->
> > +    <property name="DisplayRegisteredNetworkNameAtHome" type="b" access="read" />
> 
> 
> Quoting from the TS:
> "b1=0: display of registered PLMN name not required when registered PLMN
> is either HPLMN or a PLMN in the service provider PLMN list.
> b1=1: display of registered PLMN name required when registered PLMN is
> either HPLMN or a PLMN in the service provider PLMN list."

So let me think this through...  bit 1 is about what to display when the
registered network is a "well-known" network, ie a PLMN known to the
SIM.

0 means that the phone does not need to show the registered network name
if it's "well-known" network.  They probably should have just said "0
means the opposite of 1", the terminology here is kinda confusing.

1 means that the registered network name is required to be shown if the
network is a well-known network.  I guess b1=1 would force showing a
roaming partner's name instead of the SPN if the roaming partner is
known to the SIM?

> I don't think that "DisplayRegisteredNetworkNameAtHome" covers
> everything the property is meant to say. Actually, it hides the main
> purpose of the property, which is the fact that the registered PLMN name
> should be displayed even if the network is not the home network, but a
> network allowed in the PLMN list. E.g. I have a card from the 'Yoigo'
> operator, and when using 2G my phone connects to the 'Movistar'network:
> but my phone still needs to display 'Yoigo' instead of 'Movistar'. The
> fact that the device should display the name of the registered PLMN when
> in the same home PLMN is quite obvious already.

AT&T in the US was notorious for playing games with the SPN, ie,
displaying "Cingular" (their old name) when you were registered with on
a roaming partner network.  Then they would terminate your contract for
using too much roaming data or talk minutes.  But since the phone always
said "Cingular" of course you never knew you were roaming.  It's both
(a) a nice way to make sure normal users aren't confused, and (b) a nice
way to cover up holes in your network.

> But, I can't think of a better or shorter name...
> "DisplayRegisteredNetworkNameAtHomeOrAllowed" is even longer... :-)

Perhaps combining both bits into a "RequiredNameDisplay" (?) property is
the right way to go?  We already know what PLMN we're registered with,
and we can access the PLMN list to figure out if the registered network
is, in fact, a well-known network or not.

> 
> > +
> > +    <!--
> > +       DisplayOperatorNameWhileRoaming
> > +
> > +       If true, the user interface is required to display the operator
> > +       name from the SIM (the OperatorName) when registered on a
> > +       non-home network (roaming).
> > +    -->
> > +    <property name="DisplayOperatorNameWhileRoaming" type="b" access="read" />
> > +
> 
> Quoting from the TS:
> "b2=0: display of the service provider name is required when registered
> PLMN is neither HPLMN nor a PLMN in the service provider PLMN list.
> b2=1: display of the service provider name is not required when
> registered PLMN is neither HPLMN nor a PLMN in the service provider PLMN
> list."

So here, when the network is not a well-known network, b2=0 means SPN
name display is required, while b2=1 means you can display the
registered network name.

If we did combine both bits into one property, then we'd get the
following logic:

enum {
    DISPLAY_UNRESTRICTED = 0,
    DISPLAY_REGISTERED = 1,
    DISPLAY_SPN = 2
} DisplayName;

display = DISPLAY_UNRESTRICTED
if (registered network is well-known) {
   if (b1 == 1)
      display = DISPLAY_REGISTERED
} else {
   if (b2 == 0)
      display = DISPLAY_SPN
}

And then the UI client can use that hint to figure out whether it is
required to display the SPN or the registered name.  Thoughts?

Dan

> The name "DisplayOperatorNameWhileRoaming" is also not fully clear.
> 'Roaming' means for the modem/phone 'not in the home network', and that
> covers both being in a network allowed in the list stored in the SIM
> card, or really roaming in another not-listed network. Following my
> example before, when I connect to the Movistar network with a Yoigo SIM
> card, I will be reported as being roaming, even if the network is one of
> the allowed networks in the list stored in the SIM card.
> 
> But again, I cannot thing of a better name without the WhileRoaming
> part... "DisplaySpnWhenNotAtHomeOrAllowed" is even longer.
> 
> >    </interface>
> >  </node>
> > diff --git a/libmm-glib/mm-sim.c b/libmm-glib/mm-sim.c
> > index 40bb93b..0645ef4 100644
> > --- a/libmm-glib/mm-sim.c
> > +++ b/libmm-glib/mm-sim.c
> > @@ -207,6 +207,36 @@ mm_sim_dup_operator_name (MMSim *self)
> >  }
> >  
> >  /**
> > + * mm_sim_get_display_registered_network_name_at_home:
> > + * @self: A #MMSim.
> > + *
> > + * Returns: %TRUE if the name of the registered network must be
> > + * displayed while registered on a home network, %FALSE otherwide.
> > + */
> > +gboolean
> > +mm_sim_get_display_registered_network_name_at_home (MMSim *self)
> > +{
> > +    g_return_val_if_fail (MM_GDBUS_IS_SIM (self), FALSE);
> > +
> > +    return mm_gdbus_sim_get_display_registered_network_name_at_home (self);
> > +}
> > +
> > +/**
> > + * mm_sim_get_display_operator_name_while_roaming:
> > + * @self: A #MMSim.
> > + *
> > + * Returns: %TRUE if the operator name from the SIM must be displayed
> > + * while registered on a non-home network, %FALSE otherwide.
> > + */
> > +gboolean
> > +mm_sim_get_display_operator_name_while_roaming (MMSim *self)
> > +{
> > +    g_return_val_if_fail (MM_GDBUS_IS_SIM (self), FALSE);
> > +
> > +    return mm_gdbus_sim_get_display_operator_name_while_roaming (self);
> > +}
> > +
> > +/**
> >   * mm_sim_send_pin:
> >   * @self: A #MMSim.
> >   * @pin: The PIN code.
> > diff --git a/libmm-glib/mm-sim.h b/libmm-glib/mm-sim.h
> > index 7301434..4205b9b 100644
> > --- a/libmm-glib/mm-sim.h
> > +++ b/libmm-glib/mm-sim.h
> > @@ -38,6 +38,8 @@ const gchar *mm_sim_get_identifier          (MMSim *self);
> >  const gchar *mm_sim_get_imsi                (MMSim *self);
> >  const gchar *mm_sim_get_operator_identifier (MMSim *self);
> >  const gchar *mm_sim_get_operator_name       (MMSim *self);
> > +gboolean mm_sim_get_display_registered_network_name_at_home (MMSim *self);
> > +gboolean mm_sim_get_display_operator_name_while_roaming     (MMSim *self);
> >  
> >  gchar *mm_sim_dup_path                (MMSim *self);
> >  gchar *mm_sim_dup_identifier          (MMSim *self);
> > diff --git a/src/mm-sim.c b/src/mm-sim.c
> > index 129584e..e468be7 100644
> > --- a/src/mm-sim.c
> > +++ b/src/mm-sim.c
> > @@ -1212,6 +1212,8 @@ load_operator_identifier (MMSim *self,
> >  
> >  static gchar *
> >  parse_spn (const gchar *response,
> > +           gboolean *display_reg_home,
> > +           gboolean *display_op_roaming,
> >             GError **error)
> >  {
> >      gint sw1;
> > @@ -1268,7 +1270,16 @@ parse_spn (const gchar *response,
> >          while (buflen > 1 && bin[buflen - 1] == (char)0xff)
> >              buflen--;
> >  
> > -        /* First byte is metadata; remainder is GSM-7 unpacked into octets; convert to UTF8 */
> > +        /* Bits in the first byte specify rules for name display */
> > +        if (bin[0] == '\xFF' && bin[1] == '\xFF') {
> > +            /* If we got all-FF, don't enable display */
> > +            *display_reg_home = FALSE;
> > +            *display_op_roaming = FALSE;
> > +        } else {
> > +            *display_reg_home = ((bin[0] & 1) == 1);
> > +            *display_op_roaming = ((bin[0] & 2) == 2);
> 
> You sure this is not the inverse? The documentation says that "b2=0"
> means "required" and "b1=0" means "not required".
> 
> 
> > +        }
> > +        /* Remainder is GSM-7 unpacked into octets; convert to UTF8 */
> >          utf8 = (gchar *)mm_charset_gsm_unpacked_to_utf8 ((guint8 *)bin + 1, buflen - 1);
> >          g_free (bin);
> >          return utf8;
> > @@ -1285,6 +1296,8 @@ parse_spn (const gchar *response,
> >  static gchar *
> >  load_operator_name_finish (MMSim *self,
> >                             GAsyncResult *res,
> > +                           gboolean *display_reg_home,
> > +                           gboolean *display_op_roaming,
> >                             GError **error)
> >  {
> >      const gchar *result;
> > @@ -1293,7 +1306,7 @@ load_operator_name_finish (MMSim *self,
> >          return NULL;
> >      result = g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (res));
> >  
> > -    return parse_spn (result, error);
> > +    return parse_spn (result, display_reg_home, display_op_roaming, error);
> >  }
> >  
> >  STR_REPLY_READY_FN (load_operator_name)
> > @@ -1418,6 +1431,37 @@ load_sim_identifier_ready (MMSim *self,
> >      interface_initialization_step (ctx);
> >  }
> >  
> > +static void
> > +load_operator_name_ready (MMSim *self,
> > +                          GAsyncResult *res,
> > +                          InitAsyncContext *ctx)
> > +{
> > +    GError *error = NULL;
> > +    gchar *val;
> > +    gboolean home, roaming;
> > +
> > +    val = MM_SIM_GET_CLASS (ctx->self)->load_operator_name_finish (self,
> > +                                                                   res,
> > +                                                                   &home,
> > +                                                                   &roaming,
> > +                                                                   &error);
> > +    mm_gdbus_sim_set_operator_name (MM_GDBUS_SIM (self), val);
> > +    mm_gdbus_sim_set_display_registered_network_name_at_home (MM_GDBUS_SIM (self),
> > +                                                              home);
> > +    mm_gdbus_sim_set_display_operator_name_while_roaming (MM_GDBUS_SIM (self),
> > +                                                          roaming);
> > +    g_free (val);
> > +
> > +    if (error) {
> > +        mm_warn ("couldn't load operator name: '%s'", error->message);
> > +        g_error_free (error);
> > +    }
> > +
> > +    /* Go on to next step */
> > +    ctx->step++;
> > +    interface_initialization_step (ctx);
> > +}
> > +
> >  #undef STR_REPLY_READY_FN
> >  #define STR_REPLY_READY_FN(NAME,DISPLAY)                                \
> >      static void                                                         \
> > @@ -1444,7 +1488,6 @@ load_sim_identifier_ready (MMSim *self,
> >  
> >  STR_REPLY_READY_FN (imsi, "IMSI")
> >  STR_REPLY_READY_FN (operator_identifier, "Operator identifier")
> > -STR_REPLY_READY_FN (operator_name, "Operator name")
> >  
> >  static void
> >  interface_initialization_step (InitAsyncContext *ctx)
> > diff --git a/src/mm-sim.h b/src/mm-sim.h
> > index cba15c0..09c470d 100644
> > --- a/src/mm-sim.h
> > +++ b/src/mm-sim.h
> > @@ -77,6 +77,8 @@ struct _MMSimClass {
> >                                   gpointer user_data);
> >      gchar * (* load_operator_name_finish) (MMSim *self,
> >                                             GAsyncResult *res,
> > +                                           gboolean *home_display,
> > +                                           gboolean *roam_display,
> >                                             GError **error);
> >  
> >      /* Change PIN (async) */
> 
> 




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]