Re: [PATCH] ublox: network-registration: Handle UREG unsolicited during poll

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2326 bytes --]

Hi Richard,

On 2/9/20 3:59 PM, richard.rojfors(a)gmail.com wrote:
> From: Richard Röjfors <richard@puffinpack.se>
> 
> In the case a unsolicited indication for UREG was received
> while the status was polled. The poll response failed to parse.
> This since the unsolicited indication only carries one
> parameter, while the poll response is expected to carry two.
> 
> Update the code to loop until the response is found.
> 
> The log below shows a case where this happened.
> 
> 10:07:55 ofonod[520]: Aux: > AT+UREG?\r
> 10:07:55 ofonod[520]: Aux: < \r\n+CGREG: 4\r\n\r\n+UREG: 0\r\n\r\n+CIEV: 9,1\r\n
> 10:07:55 ofonod[520]: src/gprs.c:ofono_gprs_status_notify() /ublox_0 status unknown (4)
> 10:07:55 ofonod[520]: src/gprs.c:ofono_gprs_detached_notify() /ublox_0
> 10:07:55 ofonod[520]: Aux: < \r\n+UREG: 1,0\r\n
> 10:07:55 ofonod[520]: Aux: < \r\nOK\r\n
> ---
>   drivers/ubloxmodem/network-registration.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/ubloxmodem/network-registration.c b/drivers/ubloxmodem/network-registration.c
> index 6a524f47..2c6358e3 100644
> --- a/drivers/ubloxmodem/network-registration.c
> +++ b/drivers/ubloxmodem/network-registration.c
> @@ -288,15 +288,21 @@ static void ublox_ureg_cb(gboolean ok, GAtResult *result,
>   
>   	g_at_result_iter_init(&iter, result);
>   
> -	if (!g_at_result_iter_next(&iter, "+UREG:"))
> -		return;
> +	while (g_at_result_iter_next(&iter, "+UREG:")) {
> +		if (!g_at_result_iter_next_number(&iter, &enabled))
> +			return;
>   
> -	if (!g_at_result_iter_next_number(&iter, &enabled))
> -		return;
> +		/* Sometimes we get an unsolicited UREG here, skip it */
> +		if (!g_at_result_iter_next_number(&iter, &state))
> +			continue;
>   
> -	if (!g_at_result_iter_next_number(&iter, &state))
> -		return;
> +		goto parsed_state;

Wouldn't it look nicer to just perform the ublox_ureg_state_to_tech 
steps here (i.e within the loop) without the goto?

> +	}
> +
> +	/* In case we get here, the response was not parsed */
> +	goto error;

Then this also becomes unnecessary.

>   
> +parsed_state:
>   	tech = ublox_ureg_state_to_tech(state);
>   	if (tech < 0)
>   		/* No valid UREG status, we have to trust CREG... */
> 

Regards,
-Denis