<div dir="ltr">I'm OK with either approach.<div><br></div><div>I like the one that does not raise the error better, since the interpretation of 99 really belongs in the implementation rather than the interface.</div></div>

<div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Oct 22, 2013 at 2:23 PM, Dan Williams <span dir="ltr"><<a href="mailto:dcbw@redhat.com" target="_blank">dcbw@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im">On Tue, 2013-10-22 at 12:17 -0700, Prathmesh Prabhu wrote:<br>
> When querying signal quality using AT+CSQ, a response of '+CSQ:99,99' indicates no<br>
> network. This should lead to the signal quality being set to 0. Before this<br>
> patch, the last most recent signal quality would be left untouched.<br>
<br>
</div>Instead, how about we just want to ditch the NO_NETWORK bits in<br>
mm-broadband-mode.c::signal_quality_csq_ready() and just report 0 signal<br>
when CSQ returns 99?  There was no good reason NO_NETWORK was used,<br>
except that in MM 0.6 and earlier it was returned directly to callers.<br>
But now in MM 1.0, the error is only consumed by MM itself, internally,<br>
so there's no point to it.  So maybe just:<br>
<br>
        if (sscanf (result_str, "%d, %d", &quality, &ber)) {<br>
            if (quality == 99) {<br>
                /* 99 means unknown */<br>
                quality = 0;<br>
            } else {<br>
                /* Normalize it */<br>
                quality = CLAMP (quality, 0, 31) * 100 / 31;<br>
            }<br>
            g_simple_async_result_set_op_res_gpointer (ctx->result,<br>
                                                       GUINT_TO_POINTER (quality),<br>
                                                       NULL);<br>
<br>
or something like that.  What do you think?<br>
<span class="HOEnZb"><font color="#888888"><br>
Dan<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> ---<br>
>  src/mm-iface-modem.c | 11 ++++++++++-<br>
>  1 file changed, 10 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/mm-iface-modem.c b/src/mm-iface-modem.c<br>
> index 433dbac..ea433ed 100644<br>
> --- a/src/mm-iface-modem.c<br>
> +++ b/src/mm-iface-modem.c<br>
> @@ -1169,7 +1169,16 @@ signal_quality_check_ready (MMIfaceModem *self,<br>
>                                                                                        res,<br>
>                                                                                        &error);<br>
>      if (error) {<br>
> -        mm_dbg ("Couldn't refresh signal quality: '%s'", error->message);<br>
> +        /* If we failed to update signal quality because there was no network,<br>
> +         * we should set the signal quality to zero to reflect no network<br>
> +         * coverage.<br>
> +         */<br>
> +        if (error->code == MM_MOBILE_EQUIPMENT_ERROR_NO_NETWORK) {<br>
> +            mm_dbg ("Out of coverage area. Setting signal quality to '0'");<br>
> +            update_signal_quality (self, 0, TRUE);<br>
> +        } else {<br>
> +            mm_dbg ("Couldn't refresh signal quality: '%s'", error->message);<br>
> +        }<br>
>          g_error_free (error);<br>
>      } else<br>
>          update_signal_quality (self, signal_quality, TRUE);<br>
<br>
<br>
</div></div></blockquote></div><br></div>