[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