[PATCH] altair-lte: prevent connect while processing sim refresh
Dan Williams
dcbw at redhat.com
Wed May 28 13:56:35 PDT 2014
On Wed, 2014-05-28 at 13:36 -0700, Thieu Le wrote:
> I think I prefer having a more generic busy error code versus a
> separate busy property due to the racy nature of properties:
>
> Eg.
> 1. Client queries busy property.
> 2. Modem indicates not busy.
> 3. Client sends request.
> 4. Modem became busy before receiving client request.
You've convinced me :) It turns out we actually already have
MM_CORE_ERROR/MM_CORE_ERROR_RETRY (Operation cannot yet be performed,
retry later.) which I think would work well for this. Would you agree?
Dan
>
> On Wed, May 28, 2014 at 1:00 PM, Dan Williams <dcbw at redhat.com> wrote:
> > On Tue, 2014-05-27 at 14:27 -0700, Thieu Le wrote:
> >> This patch modifies the Altair LTE plugin to fail connect requests while the
> >> modem has detached from the network when processing sim refresh.
> >
> > So the patch looks fine to me, but I wonder if we shouldn't have a
> > generic ModemManager error code for "try this again later". Or do
> > people think SIM_BUSY is good enough for that? My thought is that if
> > something else is blocking connect for some reason (like maybe a network
> > scan is happening, and we need to wait for it to finish) that SIM_BUSY
> > isn't appropriate for that reason, but a generic "wait" error would be.
> > Just trying to make it easier to clients to handle this case, since they
> > shouldn't really care that the SIM is busy vs. something else being
> > busy.
> >
> > Alternatively, we could add another modem property to indicate
> > busy/not-busy, that clients could use to know when they could send a new
> > request?
> >
> > Dan
> >
> >> ---
> >> plugins/altair/mm-broadband-bearer-altair-lte.c | 14 ++++++++++++++
> >> plugins/altair/mm-broadband-modem-altair-lte.c | 19 ++++++++++++++++++-
> >> plugins/altair/mm-broadband-modem-altair-lte.h | 2 ++
> >> 3 files changed, 34 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/plugins/altair/mm-broadband-bearer-altair-lte.c b/plugins/altair/mm-broadband-bearer-altair-lte.c
> >> index 4594d32..46e8da4 100644
> >> --- a/plugins/altair/mm-broadband-bearer-altair-lte.c
> >> +++ b/plugins/altair/mm-broadband-bearer-altair-lte.c
> >> @@ -196,6 +196,20 @@ connect_3gpp (MMBroadbandBearer *self,
> >> return;
> >> }
> >>
> >> + /* Don't allow a connect while we detach from the network to process SIM
> >> + * refresh.
> >> + * */
> >> + if (mm_broadband_modem_altair_lte_is_sim_refresh_detach_in_progress (modem)) {
> >> + mm_dbg ("Detached from network to process SIM refresh, failing connect request");
> >> + g_simple_async_report_error_in_idle (G_OBJECT (self),
> >> + callback,
> >> + user_data,
> >> + MM_MOBILE_EQUIPMENT_ERROR,
> >> + MM_MOBILE_EQUIPMENT_ERROR_SIM_BUSY,
> >> + "Detached from network to process SIM refresh, can't connect.");
> >> + return;
> >> + }
> >> +
> >> data = mm_base_modem_peek_best_data_port (MM_BASE_MODEM (modem), MM_PORT_TYPE_NET);
> >> if (!data) {
> >> g_simple_async_report_error_in_idle (G_OBJECT (self),
> >> diff --git a/plugins/altair/mm-broadband-modem-altair-lte.c b/plugins/altair/mm-broadband-modem-altair-lte.c
> >> index 77e2137..8240185 100644
> >> --- a/plugins/altair/mm-broadband-modem-altair-lte.c
> >> +++ b/plugins/altair/mm-broadband-modem-altair-lte.c
> >> @@ -59,6 +59,10 @@ struct _MMBroadbandModemAltairLtePrivate {
> >> * This indicates that there are no more SIM refreshes and we should
> >> * reregister the device.*/
> >> guint sim_refresh_timer_id;
> >> + /* Flag indicating that we are detaching from the network to process SIM
> >> + * refresh. This is used to prevent connect requests while we're in this
> >> + * state.*/
> >> + gboolean sim_refresh_detach_in_progress;
> >> /* Regex for bearer related notifications */
> >> GRegex *statcm_regex;
> >> /* Regex for PCO notifications */
> >> @@ -639,6 +643,7 @@ altair_reregister_ready (MMBaseModem *self,
> >> } else {
> >> mm_dbg ("Modem reregistered successfully");
> >> }
> >> + MM_BROADBAND_MODEM_ALTAIR_LTE (self)->priv->sim_refresh_detach_in_progress = FALSE;
> >> }
> >>
> >> static void
> >> @@ -648,6 +653,7 @@ altair_deregister_ready (MMBaseModem *self,
> >> {
> >> if (!mm_base_modem_at_command_finish (self, res, NULL)) {
> >> mm_dbg ("Deregister modem failed");
> >> + MM_BROADBAND_MODEM_ALTAIR_LTE (self)->priv->sim_refresh_detach_in_progress = FALSE;
> >> return;
> >> }
> >>
> >> @@ -681,6 +687,10 @@ altair_load_own_numbers_ready (MMIfaceModem *iface_modem,
> >> g_strfreev (str_list);
> >> }
> >>
> >> + /* Set this flag to prevent connect requests from being processed while we
> >> + * detach from the network.*/
> >> + self->priv->sim_refresh_detach_in_progress = TRUE;
> >> +
> >> /* Deregister */
> >> mm_dbg ("Reregistering modem");
> >> mm_base_modem_at_command (
> >> @@ -703,8 +713,8 @@ altair_sim_refresh_timer_expired (MMBroadbandModemAltairLte *self)
> >> MM_IFACE_MODEM (self),
> >> (GAsyncReadyCallback)altair_load_own_numbers_ready,
> >> self);
> >> -
> >> self->priv->sim_refresh_timer_id = 0;
> >> +
> >> return FALSE;
> >> }
> >>
> >> @@ -1408,6 +1418,12 @@ mm_broadband_modem_altair_lte_new (const gchar *device,
> >> NULL);
> >> }
> >>
> >> +gboolean
> >> +mm_broadband_modem_altair_lte_is_sim_refresh_detach_in_progress (MMBroadbandModem *self)
> >> +{
> >> + return MM_BROADBAND_MODEM_ALTAIR_LTE (self)->priv->sim_refresh_detach_in_progress;
> >> +}
> >> +
> >> static void
> >> mm_broadband_modem_altair_lte_init (MMBroadbandModemAltairLte *self)
> >> {
> >> @@ -1419,6 +1435,7 @@ mm_broadband_modem_altair_lte_init (MMBroadbandModemAltairLte *self)
> >>
> >> self->priv->sim_refresh_regex = g_regex_new ("\\r\\n\\%NOTIFYEV:\\s*SIMREFRESH,?(\\d*)\\r+\\n",
> >> G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
> >> + self->priv->sim_refresh_detach_in_progress = FALSE;
> >> self->priv->sim_refresh_timer_id = 0;
> >> self->priv->statcm_regex = g_regex_new ("\\r\\n\\%STATCM:\\s*(\\d*),?(\\d*)\\r+\\n",
> >> G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
> >> diff --git a/plugins/altair/mm-broadband-modem-altair-lte.h b/plugins/altair/mm-broadband-modem-altair-lte.h
> >> index 50c9f3e..fc8d362 100644
> >> --- a/plugins/altair/mm-broadband-modem-altair-lte.h
> >> +++ b/plugins/altair/mm-broadband-modem-altair-lte.h
> >> @@ -48,4 +48,6 @@ MMBroadbandModemAltairLte *mm_broadband_modem_altair_lte_new (const gchar *devic
> >> guint16 vendor_id,
> >> guint16 product_id);
> >>
> >> +gboolean mm_broadband_modem_altair_lte_is_sim_refresh_detach_in_progress (MMBroadbandModem *self);
> >> +
> >> #endif /* MM_BROADBAND_MODEM_ALTAIR_LTE_H */
> >
> >
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
More information about the ModemManager-devel
mailing list