New 'u-blox' plugin (v2)

[Aleksander Morgado]

Hey hey,

Thanks again for the review :)

>>
>>  * Added support for the new CSFB related registration state values.
>
> The DENIED handling looks like it changed:
>
> +    /* Denied? */
> +    if (ctx->cs  == MM_MODEM_3GPP_REGISTRATION_STATE_DENIED ||
> +        ctx->ps  == MM_MODEM_3GPP_REGISTRATION_STATE_DENIED ||
> +        ctx->eps == MM_MODEM_3GPP_REGISTRATION_STATE_DENIED)
> +         return MM_MODEM_3GPP_REGISTRATION_STATE_DENIED;
>
> -    /* If one state is DENIED and the others are UNKNOWN, use DENIED
> */
> -    if (ctx->cs == MM_MODEM_3GPP_REGISTRATION_STATE_DENIED &&
> -        ctx->ps == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN &&
> -        ctx->eps == MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN)
>
> This effectively reverts part of
> 9186e2aa39668c0d071f52d47d6a1aea087bc8a7 which was discussed on the
> list on 2015-02-23.  The background for that commit was that I had a
> Verizon SIM in my EM7345, the device doesn't have Verizon LTE bands
> (B13), and other visible LTE providers (AT&T and T-Mobile) denied LTE
> registration to the modem.  So the modem should report DENIED in that
> case.
>
> But is there any scenario where a device could be CS/PS
> registered/searching/etc and EPS be DENIED, or the other way around?
>  In that case if the modem can still be used on some domain (CS/PS/EPS)
> I would expect the modem state to not be DENIED.
>

Ah! You're totally right, the logic of DENIED ended up changing, I'll
recover the old one as you're right, we should only show DENIED if the
others aren't in any other known state.

>
>>  * I didn't do the +CFUN parsing generic, because the +CFUN state
>> numbers reported are mostly (except for the first 5) vendor-specific.
>
> Though couldn't we  move the actual CFUN parsing to the core, and leave
> the <state>/<aux> interpretation up to the plugin itself?  Linktop,
> MBM, generic, and now Ublox all send "CFUN?" and could all have
> consolidated code.  Note that some devices may return only one number
> there, so the regex would have to consider <aux> optional.
>

I understand what you wanted now, yes, we can definitely do that.

>> You can find the updated ublox plugin in the 'aleksander/ublox'
>> branch
>> in the upstream git repository:
>> https://cgit.freedesktop.org/ModemManager/ModemManager/log/?h=aleksan
>> der/ublox
>>
>> Reviews of the implementation are more than welcome.
>
>> ublox: implement PIN retry count loading
>> ublox: new +UPINCNT response parser
>
> The last hunk of "implement PI retry count loading" shouljd be in the
> +UPINCNT patch.
>

Ok.

>> modem-helpers: new helper to parse +CESQ response into MMSignal
> objects
>
> There's some small differences from the way I implemented it in the
> Huawei signal interface, at least for get_rsrq_db() (-19 instead of
> -20).  Not sure which one is right.
>

I'll give it a look.

>> ublox: new +UBANDSEL? response parser
>
> +    dupstr = g_strdup (mm_strip_tag (response, "+UBANDSEL:"));
> +    i = strlen (dupstr);
> +    while (i > 0 && (dupstr[i - 1] == '\r' || dupstr[i - 1] == '\n' || dupstr[i - 1] == ' ')) {
> +        dupstr[i - 1] = '\0';
> +        i--;
> +    }
>
> dupstr = g_strchomp (g_strdup (mm_strip_tag (response, "+UBANDSEL:")));
>
> Maybe?
>

But g_strchomp() doesn't remove LFs and CRs, right?

-- 
Aleksander
https://aleksander.es