<div dir="ltr">Sorry for the delayed response, I was travelling this week.<br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Oct 11, 2013 at 10:10 AM, Dan Williams <span dir="ltr"><<a href="mailto:dcbw@redhat.com" target="_blank">dcbw@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Thu, 2013-10-10 at 14:31 -0700, Prathmesh Prabhu wrote:<br>
> An unknown --> home (roaming) registration state change is not visible to the<br>
> plugin until mm_iface_modem_3gpp_reload_current_registration_info completes.<br>
> This causes the subscription state check during registration to miss the fact<br>
> that registration was successful.<br>
><br>
> load_subscription_state is intended to check subscription state<br>
> post-registration. As an initial implementation, mark the SIM provisioned in<br>
> absence of more information post-registration.<br>
<br>
</div>Ok, to clarify here, when update_registration_state() gets called in<br>
response to either a poll or an unsolicited registration state message,<br>
it only sets the new registration state *after* it calls<br>
mm_iface_modem_3gpp_reload_current_registration_info(). So the sequence<br>
is:<br>
<br>
something sets old registration state><br>
... (time passes)<br>
new registration state read<br>
update_registration_state() called<br>
if (home | roaming)<br>
mm_iface_modem_3gpp_reload_current_registration_info()<br>
loads operator code<br>
loads operator name<br>
loads subscription state<br>
<note that nothing here knows about new registration state><br>
update_registration_reload_current_registration_info_ready() called<br>
sets new registration state on skeleton<br>
updates 3GPP subsystem state<br>
<br>
So yeah, if a plugin overrides load_subscription_state, and the<br>
registration state changes to home|roaming, there's no way the plugin's<br>
load_subscription_state can figure out the new registration state.<br></blockquote><div>As Aleksander noted in a follow-up mail, load_subscription_state is called only if registration state is updated to HOME/ROAMING.</div>
<div>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.</div>
<div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I wonder why the new registration state isn't set immediately in<br>
update_registration_state() before loading the operator stuff? If we<br>
fail to load operator name/code/subscription, IMHO we should still set<br>
the correct registration state for the modem. We could delay the<br>
mm_iface_modem_update_subsystem_state() until the current registration<br>
information is loaded if modem state changes might make the call chain<br>
too complicated.<br>
<br>
Aleksander, what do you think? If this were done, Prathmesh, would this<br>
Altair patch still be needed?<br></blockquote><div> </div><div>If update_registration_state always updated the registration state before the asynchronous calls finish, this patch would not be needed. </div><div><br></div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
Dan<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> ---<br>
> plugins/altair/mm-broadband-modem-altair-lte.c | 46 ++++++++++++++++++++++++++<br>
> 1 file changed, 46 insertions(+)<br>
><br>
> diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c b/plugins/altair/mm-broadband-modem-altair-lte.c<br>
> index 64d4c51..9185193 100644<br>
> --- a/plugins/altair/mm-broadband-modem-altair-lte.c<br>
> +++ b/plugins/altair/mm-broadband-modem-altair-lte.c<br>
> @@ -1025,6 +1025,50 @@ modem_3gpp_load_operator_name (MMIfaceModem3gpp *self,<br>
> }<br>
><br>
> /*****************************************************************************/<br>
> +/* Subscription State Loading (3GPP interface) */<br>
> +<br>
> +static MMModem3gppSubscriptionState<br>
> +modem_3gpp_load_subscription_state_finish (MMIfaceModem3gpp *self,<br>
> + GAsyncResult *res,<br>
> + GError **error)<br>
> +{<br>
> + if (g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT (res), error))<br>
> + return MM_MODEM_3GPP_SUBSCRIPTION_STATE_UNKNOWN;<br>
> +<br>
> + return (MMModem3gppSubscriptionState) GPOINTER_TO_UINT (<br>
> + g_simple_async_result_get_op_res_gpointer (G_SIMPLE_ASYNC_RESULT (res)));<br>
> +}<br>
> +<br>
> +static void<br>
> +modem_3gpp_load_subscription_state (MMIfaceModem3gpp *self,<br>
> + GAsyncReadyCallback callback,<br>
> + gpointer user_data)<br>
> +{<br>
> + GSimpleAsyncResult *result;<br>
> +<br>
> + result = g_simple_async_result_new (G_OBJECT (self),<br>
> + callback,<br>
> + user_data,<br>
> + modem_3gpp_load_subscription_state);<br>
> +<br>
> + /* Reloading subscription state only occurs on a successfully registered<br>
> + * modem. (Although the 3GPP interface does not reflect this until after<br>
> + * loading operator information completes.)<br>
> + * We should set the subscription state to provisioned here, unless further<br>
> + * information from PCO says otherwise.<br>
> + * TODO: Use PCO value to check for subscription state post-registration.<br>
> + */<br>
> + mm_dbg ("Load subscription state: Marking the SIM as provisioned.");<br>
> + g_simple_async_result_set_op_res_gpointer (<br>
> + result,<br>
> + GUINT_TO_POINTER (MM_MODEM_3GPP_SUBSCRIPTION_STATE_PROVISIONED),<br>
> + NULL);<br>
> +<br>
> + g_simple_async_result_complete_in_idle (result);<br>
> + g_object_unref (result);<br>
> +}<br>
> +<br>
> +/*****************************************************************************/<br>
> /* Generic ports open/close context */<br>
><br>
> static const gchar *primary_init_sequence[] = {<br>
> @@ -1146,6 +1190,8 @@ iface_modem_3gpp_init (MMIfaceModem3gpp *iface)<br>
> iface->register_in_network_finish = modem_3gpp_register_in_network_finish;<br>
> iface->run_registration_checks = modem_3gpp_run_registration_checks;<br>
> iface->run_registration_checks_finish = modem_3gpp_run_registration_checks_finish;<br>
> + iface->load_subscription_state = modem_3gpp_load_subscription_state;<br>
> + iface->load_subscription_state_finish = modem_3gpp_load_subscription_state_finish;<br>
><br>
> /* Scanning is not currently supported */<br>
> iface->scan_networks = NULL;<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>