[PATCH] telit: unsupported CSIM lock should not skip loading unlock retries

[Carlo Lobrano]

> How about we store the "not supported" information somewhere, like e.g.
> in a "FeatureSupport" enum value as we do in Huawei for other things:
>
https://cgit.freedesktop.org/ModemManager/ModemManager/tree/plugins/huawei/mm-broadband-modem-huawei.c#n79

Yep, I think it's a good idea, I can update the patch with this implemented.


Il giovedì 6 aprile 2017, Aleksander Morgado <aleksander at aleksander.es> ha
scritto:

> On 06/04/17 14:37, Carlo Lobrano wrote:
> > Some modems do not support CSIM lock/unlock, but they do support
> > querying SIM unlock retries through +CSIM command.
> >
> > If CSIM lock returns with "unsupported command" do not propagate
> > the error and continue with the other CSIM queries instead.
> > ---
> >
> > Sorry for not having caught this problem earlier,
> > when reviewing the CSIM lock patch.
> >
>
> Yes, this is a good logic change that we should have done...
>
> How about we store the "not supported" information somewhere, like e.g.
> in a "FeatureSupport" enum value as we do in Huawei for other things:
> https://cgit.freedesktop.org/ModemManager/ModemManager/
> tree/plugins/huawei/mm-broadband-modem-huawei.c#n79
>
> i.e. when the object is created:
>   self->priv->csim_lock_supported = FEATURE_SUPPORT_UNKNOWN;
>
> Them, after the first time we try the CSIM lock operation, we store the
> proper result (e.g. FEATURE_NOT_SUPPORTED or FEATURE_SUPPORTED).
>
> Then, the next time we are going to do a CSIM lock (or unlock)
> operation, we check whether the feature is supported or not, and if it
> isn't, we just ignore the operation right away.
>
> What do you think? This would help to also avoid trying the CSIM unlock
> operation if we already know CSIM lock is unsupported.
>
>
> > ---
> >
> >  plugins/telit/mm-broadband-modem-telit.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/plugins/telit/mm-broadband-modem-telit.c
> b/plugins/telit/mm-broadband-modem-telit.c
> > index cce0229..3680a8a 100644
> > --- a/plugins/telit/mm-broadband-modem-telit.c
> > +++ b/plugins/telit/mm-broadband-modem-telit.c
> > @@ -591,10 +591,17 @@ csim_lock_ready (MMBaseModem              *self,
> >
> >      response = mm_base_modem_at_command_finish (self, res, &error);
> >      if (!response) {
> > -        g_prefix_error (&error, "Couldn't lock SIM card: ");
> > -        g_simple_async_result_take_error (ctx->result, error);
> > -        load_unlock_retries_context_complete_and_free (ctx);
> > -        return;
> > +        if (g_error_matches (error,
> > +                             MM_MOBILE_EQUIPMENT_ERROR,
> > +                             MM_MOBILE_EQUIPMENT_ERROR_NOT_SUPPORTED))
> {
> > +            mm_warn ("Couldn't lock SIM card: %s. Continuing as
> well...", error->message);
> > +            g_error_free (error);
> > +        } else {
> > +            g_prefix_error (&error, "Couldn't lock SIM card: ");
> > +            g_simple_async_result_take_error (ctx->result, error);
> > +            load_unlock_retries_context_complete_and_free (ctx);
> > +            return;
> > +        }
> >      }
> >
> >      ctx->step++;
> >
>
>
> --
> Aleksander
> https://aleksander.es
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170407/6d1820c6/attachment.html>