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

[Aleksander Morgado]

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