[PATCH] altair-lte: prevent connect while processing sim refresh

Thieu Le thieule at chromium.org
Wed May 28 13:36:34 PDT 2014


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.


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 */
>
>


More information about the ModemManager-devel mailing list