[PATCH V2] altair-lte: mark SIM provisioned in load_subscription_state

[Prathmesh Prabhu Chromium]

Sorry for the delayed response, I was travelling this week.


On Fri, Oct 11, 2013 at 10:10 AM, Dan Williams <dcbw at redhat.com> wrote:

> On Thu, 2013-10-10 at 14:31 -0700, Prathmesh Prabhu wrote:
> > An unknown --> home (roaming) registration state change is not visible
> to the
> > plugin until mm_iface_modem_3gpp_reload_current_registration_info
> completes.
> > This causes the subscription state check during registration to miss the
> fact
> > that registration was successful.
> >
> > load_subscription_state is intended to check subscription state
> > post-registration. As an initial implementation, mark the SIM
> provisioned in
> > absence of more information post-registration.
>
> Ok, to clarify here, when update_registration_state() gets called in
> response to either a poll or an unsolicited registration state message,
> it only sets the new registration state *after* it calls
> mm_iface_modem_3gpp_reload_current_registration_info().  So the sequence
> is:
>
> something sets old registration state>
> ... (time passes)
> new registration state read
> update_registration_state() called
> if (home | roaming)
>    mm_iface_modem_3gpp_reload_current_registration_info()
>      loads operator code
>      loads operator name
>      loads subscription state
> <note that nothing here knows about new registration state>
> update_registration_reload_current_registration_info_ready() called
> sets new registration state on skeleton
> updates 3GPP subsystem state
>
> So yeah, if a plugin overrides load_subscription_state, and the
> registration state changes to home|roaming, there's no way the plugin's
> load_subscription_state can figure out the new registration state.
>
As Aleksander noted in a follow-up mail, load_subscription_state is called
only if registration state is updated to HOME/ROAMING.
On the other hand, if a plugin overrides the update_registration_state
sequence of calls, chaining up the mm_broadband_modem implementation of the
function, the plugin implementation fails to notice the successful
registration update.

This is the case because the whole sequence of update_registration_state
finishes without updating the registration state to home (roaming), as the
reload_registration_info calls are still outstanding.


> I wonder why the new registration state isn't set immediately in
> update_registration_state() before loading the operator stuff?  If we
> fail to load operator name/code/subscription, IMHO we should still set
> the correct registration state for the modem.  We could delay the
> mm_iface_modem_update_subsystem_state() until the current registration
> information is loaded if modem state changes might make the call chain
> too complicated.
>
> Aleksander, what do you think?  If this were done, Prathmesh, would this
> Altair patch still be needed?
>

If update_registration_state always updated the registration state before
the asynchronous calls finish, this patch would not be needed.


> Dan
>
> > ---
> >  plugins/altair/mm-broadband-modem-altair-lte.c | 46
> ++++++++++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >
> > diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c
> b/plugins/altair/mm-broadband-modem-altair-lte.c
> > index 64d4c51..9185193 100644
> > --- a/plugins/altair/mm-broadband-modem-altair-lte.c
> > +++ b/plugins/altair/mm-broadband-modem-altair-lte.c
> > @@ -1025,6 +1025,50 @@ modem_3gpp_load_operator_name (MMIfaceModem3gpp
> *self,
> >  }
> >
> >
>  /*****************************************************************************/
> > +/* Subscription State Loading (3GPP interface) */
> > +
> > +static MMModem3gppSubscriptionState
> > +modem_3gpp_load_subscription_state_finish (MMIfaceModem3gpp *self,
> > +                                           GAsyncResult *res,
> > +                                           GError **error)
> > +{
> > +    if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT
> (res), error))
> > +        return MM_MODEM_3GPP_SUBSCRIPTION_STATE_UNKNOWN;
> > +
> > +    return (MMModem3gppSubscriptionState) GPOINTER_TO_UINT (
> > +        g_simple_async_result_get_op_res_gpointer
> (G_SIMPLE_ASYNC_RESULT (res)));
> > +}
> > +
> > +static void
> > +modem_3gpp_load_subscription_state (MMIfaceModem3gpp *self,
> > +                                    GAsyncReadyCallback callback,
> > +                                    gpointer user_data)
> > +{
> > +    GSimpleAsyncResult *result;
> > +
> > +    result = g_simple_async_result_new (G_OBJECT (self),
> > +                                        callback,
> > +                                        user_data,
> > +
>  modem_3gpp_load_subscription_state);
> > +
> > +   /* Reloading subscription state only occurs on a successfully
> registered
> > +    * modem. (Although the 3GPP interface does not reflect this until
> after
> > +    * loading operator information completes.)
> > +    * We should set the subscription state to provisioned here, unless
> further
> > +    * information from PCO says otherwise.
> > +    * TODO: Use PCO value to check for subscription state
> post-registration.
> > +    */
> > +    mm_dbg ("Load subscription state: Marking the SIM as provisioned.");
> > +    g_simple_async_result_set_op_res_gpointer (
> > +        result,
> > +        GUINT_TO_POINTER (MM_MODEM_3GPP_SUBSCRIPTION_STATE_PROVISIONED),
> > +        NULL);
> > +
> > +    g_simple_async_result_complete_in_idle (result);
> > +    g_object_unref (result);
> > +}
> > +
> >
> +/*****************************************************************************/
> >  /* Generic ports open/close context */
> >
> >  static const gchar *primary_init_sequence[] = {
> > @@ -1146,6 +1190,8 @@ iface_modem_3gpp_init (MMIfaceModem3gpp *iface)
> >      iface->register_in_network_finish =
> modem_3gpp_register_in_network_finish;
> >      iface->run_registration_checks = modem_3gpp_run_registration_checks;
> >      iface->run_registration_checks_finish =
> modem_3gpp_run_registration_checks_finish;
> > +    iface->load_subscription_state = modem_3gpp_load_subscription_state;
> > +    iface->load_subscription_state_finish =
> modem_3gpp_load_subscription_state_finish;
> >
> >      /* Scanning is not currently supported */
> >      iface->scan_networks = NULL;
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/modemmanager-devel/attachments/20131018/5bbd036d/attachment.html>