Hi Aleksander, that's weird. By the way I should be able to test it this afternoon with a LE910 or HE910 and let you know.
BR, Carlo On Wed, 15 Mar 2017 at 00:38 Aleksander Morgado <aleksan...@aleksander.es> wrote: > It will allow us to avoid totally or partially the after-sim-unlock > explicit wait time that we're forcing. > > https://bugs.freedesktop.org/show_bug.cgi?id=100206 > --- > > Hey Carlo & Daniele, > > I tried to enable #QSS=2 to see if I could get "#QSS: 3" indications: > 3 - SIM INSERTED and READY (SMS and Phonebook access are possible). > > But looks like the HW I have doesn't emit any #QSS indication for some > reason, any idea why? E.g. even if I remove the SIM and insert it again I > get no "#QSS: 0" or "#QSS: 1" indication. Any idea why? > > Could you also review and test the patch with a device that supports > "#QSS: 3" indications? I'm still not very convinced whether this is worth > it to avoid the 1s timeout in the after-sim-unlock step, truth be told, but > maybe if this setup works (i.e. we get a #QSS notification as soon as the > SIM is completely ready) we could extend the timeout to a value a bit > bigger just in case, like 5s? > > What do you think? > > --- > plugins/telit/mm-broadband-modem-telit.c | 106 > ++++++++++++++++++++++++++----- > plugins/telit/mm-broadband-modem-telit.h | 2 + > 2 files changed, 91 insertions(+), 17 deletions(-) > > diff --git a/plugins/telit/mm-broadband-modem-telit.c > b/plugins/telit/mm-broadband-modem-telit.c > index 8bc9d1e5..76bcb1d4 100644 > --- a/plugins/telit/mm-broadband-modem-telit.c > +++ b/plugins/telit/mm-broadband-modem-telit.c > @@ -42,40 +42,83 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit, > mm_broadband_modem_telit, MM_TYPE > G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM, > iface_modem_init) > G_IMPLEMENT_INTERFACE (MM_TYPE_IFACE_MODEM_3GPP, > iface_modem_3gpp_init)); > > +struct _MMBroadbandModemTelitPrivate { > + /* Whether the SIM ready notification has been received */ > + gboolean sim_ready; > + GTask *sim_ready_task; > + guint sim_ready_timeout_id; > +}; > > > > /*****************************************************************************/ > /* After Sim Unlock (Modem interface) */ > + > +/* A short delay is necessary with some SIMs when they have just been > + * unlocked. Using up to 1 second as secure margin. */ > +#define SIM_READY_TIMEOUT_SECS 1 > + > static gboolean > modem_after_sim_unlock_finish (MMIfaceModem *self, > GAsyncResult *res, > GError **error) > { > - return TRUE; > + return g_task_propagate_boolean (G_TASK (res), error); > } > > static gboolean > -after_sim_unlock_ready (GSimpleAsyncResult *result) > +after_sim_unlock_ready (MMBroadbandModemTelit *self) > { > - g_simple_async_result_complete (result); > - g_object_unref (result); > + g_assert (self->priv->sim_ready_timeout_id); > + g_assert (self->priv->sim_ready_task); > + > + self->priv->sim_ready_timeout_id = 0; > + > + g_task_return_boolean (self->priv->sim_ready_task, TRUE); > + g_clear_object (&self->priv->sim_ready_task); > + > return G_SOURCE_REMOVE; > } > > static void > -modem_after_sim_unlock (MMIfaceModem *self, > - GAsyncReadyCallback callback, > - gpointer user_data) > +modem_after_sim_unlock_cancel (MMBroadbandModemTelit *self) > { > - GSimpleAsyncResult *result; > + /* Cancel the after-sim-unlock wait timeout */ > + if (self->priv->sim_ready_timeout_id) { > + g_source_remove (self->priv->sim_ready_timeout_id); > + self->priv->sim_ready_timeout_id = 0; > + } > > - result = g_simple_async_result_new (G_OBJECT (self), > - callback, > - user_data, > - modem_after_sim_unlock); > + /* Complete after-sim-unlock wait task with error */ > + if (self->priv->sim_ready_task) { > + g_task_return_new_error (self->priv->sim_ready_task, > MM_CORE_ERROR, MM_CORE_ERROR_FAILED, "SIM removed"); > + g_clear_object (&self->priv->sim_ready_task); > + } > +} > + > +static void > +modem_after_sim_unlock (MMIfaceModem *_self, > + GAsyncReadyCallback callback, > + gpointer user_data) > +{ > + MMBroadbandModemTelit *self; > + > + self = MM_BROADBAND_MODEM_TELIT (_self); > + > + g_assert (!self->priv->sim_ready_task); > + g_assert (!self->priv->sim_ready_timeout_id); > + > + /* Create new task that we store in the private info, so that it can > be > + * completed early when unsolicited messages arrive. */ > + self->priv->sim_ready_task = g_task_new (self, NULL, callback, > user_data); > > - /* A short delay is necessary with some SIMs when > - they have just been unlocked. Using 1 second as secure margin. */ > - g_timeout_add_seconds (1, (GSourceFunc) after_sim_unlock_ready, > result); > + /* If SIM ready already reported, we're done */ > + if (self->priv->sim_ready) { > + g_task_return_boolean (self->priv->sim_ready_task, TRUE); > + g_clear_object (&self->priv->sim_ready_task); > + return; > + } > + > + /* Wait for SIM readiness */ > + self->priv->sim_ready_timeout_id = g_timeout_add_seconds > (SIM_READY_TIMEOUT_SECS, (GSourceFunc) after_sim_unlock_ready, self); > } > > > > /*****************************************************************************/ > @@ -94,6 +137,10 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port, > switch (qss) { > case 0: > mm_info ("QSS: SIM removed"); > + > + /* Flag as not ready */ > + self->priv->sim_ready = FALSE; > + modem_after_sim_unlock_cancel (self); > break; > case 1: > mm_info ("QSS: SIM inserted"); > @@ -102,7 +149,8 @@ telit_qss_unsolicited_handler (MMPortSerialAt *port, > mm_info ("QSS: SIM inserted and PIN unlocked"); > break; > case 3: > - mm_info ("QSS: SIM inserted and PIN locked"); > + mm_info ("QSS: SIM inserted and Ready"); > + self->priv->sim_ready = TRUE; > break; > default: > mm_warn ("QSS: unknown QSS value %d", qss); > @@ -203,9 +251,12 @@ modem_setup_sim_hot_swap (MMIfaceModem *self, > if (!port) > port = mm_base_modem_peek_port_primary (MM_BASE_MODEM (self)); > > + /* We enable QSS=2 so that we get notifications not only of SIM > inserted or > + * not inserted, but also when the SIM is unlocked and ready for SMS > and > + * phonebook access */ > mm_base_modem_at_command_full (MM_BASE_MODEM (self), > port, > - "#QSS=1", > + "#QSS=2", > 3, > FALSE, > FALSE, /* raw */ > @@ -1259,6 +1310,22 @@ mm_broadband_modem_telit_new (const gchar *device, > static void > mm_broadband_modem_telit_init (MMBroadbandModemTelit *self) > { > + /* Initialize private data */ > + self->priv = G_TYPE_INSTANCE_GET_PRIVATE (self, > MM_TYPE_BROADBAND_MODEM_TELIT, MMBroadbandModemTelitPrivate); > +} > + > +static void > +finalize (GObject *object) > +{ > + MMBroadbandModemTelit *self = MM_BROADBAND_MODEM_TELIT (object); > + > + /* The after sim unlock step takes a full reference of the modem > object in > + * the GTask, and therefore, the object will never be unref-ed while > this > + * operation is pending */ > + g_assert (!self->priv->sim_ready_timeout_id); > + g_assert (!self->priv->sim_ready_task); > + > + G_OBJECT_CLASS (mm_broadband_modem_telit_parent_class)->finalize > (object); > } > > static void > @@ -1304,4 +1371,9 @@ iface_modem_3gpp_init (MMIfaceModem3gpp *iface) > static void > mm_broadband_modem_telit_class_init (MMBroadbandModemTelitClass *klass) > { > + GObjectClass *object_class = G_OBJECT_CLASS (klass); > + > + g_type_class_add_private (object_class, sizeof > (MMBroadbandModemTelitPrivate)); > + > + object_class->finalize = finalize; > } > diff --git a/plugins/telit/mm-broadband-modem-telit.h > b/plugins/telit/mm-broadband-modem-telit.h > index 50e6365f..f68465e7 100644 > --- a/plugins/telit/mm-broadband-modem-telit.h > +++ b/plugins/telit/mm-broadband-modem-telit.h > @@ -29,9 +29,11 @@ > > typedef struct _MMBroadbandModemTelit MMBroadbandModemTelit; > typedef struct _MMBroadbandModemTelitClass MMBroadbandModemTelitClass; > +typedef struct _MMBroadbandModemTelitPrivate MMBroadbandModemTelitPrivate; > > struct _MMBroadbandModemTelit { > MMBroadbandModem parent; > + MMBroadbandModemTelitPrivate *priv; > }; > > struct _MMBroadbandModemTelitClass{ > -- > 2.12.0 >
_______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel