<div><div><div>> How about we store the "not supported" information somewhere, like e.g.</div><div>> in a "FeatureSupport" enum value as we do in Huawei for other things:</div><div>> <a href="https://cgit.freedesktop.org/ModemManager/ModemManager/tree/plugins/huawei/mm-broadband-modem-huawei.c#n79">https://cgit.freedesktop.org/ModemManager/ModemManager/tree/plugins/huawei/mm-broadband-modem-huawei.c#n79</a></div><div><br></div><div>Yep, I think it's a good idea, I can update the patch with this implemented.</div><div><br></div></div><div><br></div>Il giovedì 6 aprile 2017, Aleksander Morgado <<a href="mailto:aleksander@aleksander.es">aleksander@aleksander.es</a>> ha scritto:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 06/04/17 14:37, Carlo Lobrano wrote:<br>
> Some modems do not support CSIM lock/unlock, but they do support<br>
> querying SIM unlock retries through +CSIM command.<br>
><br>
> If CSIM lock returns with "unsupported command" do not propagate<br>
> the error and continue with the other CSIM queries instead.<br>
> ---<br>
><br>
> Sorry for not having caught this problem earlier,<br>
> when reviewing the CSIM lock patch.<br>
><br>
<br>
Yes, this is a good logic change that we should have done...<br>
<br>
How about we store the "not supported" information somewhere, like e.g.<br>
in a "FeatureSupport" enum value as we do in Huawei for other things:<br>
<a href="https://cgit.freedesktop.org/ModemManager/ModemManager/tree/plugins/huawei/mm-broadband-modem-huawei.c#n79" target="_blank">https://cgit.freedesktop.org/<wbr>ModemManager/ModemManager/<wbr>tree/plugins/huawei/mm-<wbr>broadband-modem-huawei.c#n79</a><br>
<br>
i.e. when the object is created:<br>
  self->priv->csim_lock_<wbr>supported = FEATURE_SUPPORT_UNKNOWN;<br>
<br>
Them, after the first time we try the CSIM lock operation, we store the<br>
proper result (e.g. FEATURE_NOT_SUPPORTED or FEATURE_SUPPORTED).<br>
<br>
Then, the next time we are going to do a CSIM lock (or unlock)<br>
operation, we check whether the feature is supported or not, and if it<br>
isn't, we just ignore the operation right away.<br>
<br>
What do you think? This would help to also avoid trying the CSIM unlock<br>
operation if we already know CSIM lock is unsupported.<br>
<br>
<br>
> ---<br>
><br>
>  plugins/telit/mm-broadband-<wbr>modem-telit.c | 15 +++++++++++----<br>
>  1 file changed, 11 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/plugins/telit/mm-broadband-<wbr>modem-telit.c b/plugins/telit/mm-broadband-<wbr>modem-telit.c<br>
> index cce0229..3680a8a 100644<br>
> --- a/plugins/telit/mm-broadband-<wbr>modem-telit.c<br>
> +++ b/plugins/telit/mm-broadband-<wbr>modem-telit.c<br>
> @@ -591,10 +591,17 @@ csim_lock_ready (MMBaseModem              *self,<br>
><br>
>      response = mm_base_modem_at_command_<wbr>finish (self, res, &error);<br>
>      if (!response) {<br>
> -        g_prefix_error (&error, "Couldn't lock SIM card: ");<br>
> -        g_simple_async_result_take_<wbr>error (ctx->result, error);<br>
> -        load_unlock_retries_context_<wbr>complete_and_free (ctx);<br>
> -        return;<br>
> +        if (g_error_matches (error,<br>
> +                             MM_MOBILE_EQUIPMENT_ERROR,<br>
> +                             MM_MOBILE_EQUIPMENT_ERROR_NOT_<wbr>SUPPORTED)) {<br>
> +            mm_warn ("Couldn't lock SIM card: %s. Continuing as well...", error->message);<br>
> +            g_error_free (error);<br>
> +        } else {<br>
> +            g_prefix_error (&error, "Couldn't lock SIM card: ");<br>
> +            g_simple_async_result_take_<wbr>error (ctx->result, error);<br>
> +            load_unlock_retries_context_<wbr>complete_and_free (ctx);<br>
> +            return;<br>
> +        }<br>
>      }<br>
><br>
>      ctx->step++;<br>
><br>
<br>
<br>
--<br>
Aleksander<br>
<a href="https://aleksander.es" target="_blank">https://aleksander.es</a><br>
</blockquote></div>