Re: [PATCH v2 2/3] network-registration.c: implement CIND forTelit UC864-G

[Marcel Holtmann <marcel@holtmann.org>]

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

Hi Bernhard,

> *UC864 has an incompatible CIND
> *add a Telit specific check for "not measurable" strength
> 
> Co-authored-by: Christopher Vogl <Christopher.Vogl@hale.at>
> ---
>  drivers/atmodem/network-registration.c |   40 +++++++++++++++++++++++++++++--
>  1 files changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
> index b3aa511..984e85e 100644
> --- a/drivers/atmodem/network-registration.c
> +++ b/drivers/atmodem/network-registration.c
> @@ -666,7 +666,20 @@ static void ciev_notify(GAtResult *result, gpointer user_data)
>  	if (!g_at_result_iter_next_number(&iter, &strength))
>  		return;
>  
> -	strength = (strength * 100) / (nd->signal_max - nd->signal_min);
> +	switch (nd->vendor) {
> +	case OFONO_VENDOR_TELIT:
> +		/* Check for "signal not measurable" */
> +		if (strength == 99)
> +			strength = 0;
> +		else
> +			strength = (strength * 100) / (nd->signal_max -
> +						nd->signal_min);
> +		break;
> +	default:
> +		strength = (strength * 100) / (nd->signal_max - nd->signal_min);
> +		break;
> +	}
> +

this is something that does not look like vendor quirk. The 99 is always
a special value and we should just do this for all modems.

Denis, any thoughts?

>  	ofono_netreg_strength_notify(netreg, strength);
>  }
>  
> @@ -798,8 +811,19 @@ static void cind_cb(gboolean ok, GAtResult *result, gpointer user_data)
>  
>  	g_at_result_iter_next_number(&iter, &strength);
>  
> -	strength = (strength * 100) / (nd->signal_max - nd->signal_min);
> -
> +	switch (nd->vendor) {
> +	case OFONO_VENDOR_TELIT:
> +		/* Check for "signal not measurable" */
> +		if (strength == 99)
> +			strength = 0;
> +		else
> +			strength = (strength * 100) / (nd->signal_max -
> +						nd->signal_min);
> +		break;
> +	default:
> +		strength = (strength * 100) / (nd->signal_max - nd->signal_min);
> +		break;
> +	}
>  	cb(&error, strength, cbd->data);
>  }

If we really need quirks, then this code should be extracted into its
own function. However right now I would say to just treat 99 special for
all modems.

> @@ -1302,6 +1326,16 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
>  	case OFONO_VENDOR_NOKIA:
>  		/* Signal strength reporting via CIND is not supported */
>  		break;
> +	case OFONO_VENDOR_TELIT:
> +		/* Use RSSI instead of signal.*/
> +		nd->signal_index = 9;
> +		nd->signal_min = 0;
> +		nd->signal_max = 5;
> +		g_at_chat_send(nd->chat, "AT+CIND=1", cind_prefix,
> +						NULL, NULL, NULL);
> +		g_at_chat_register(nd->chat, "+CIEV:", ciev_notify,
> +					FALSE, netreg, NULL);
> +		break;
>  	default:
>  		g_at_chat_send(nd->chat, "AT+CIND=?", cind_prefix,
>  				cind_support_cb, netreg, NULL);

Regards

Marcel