Re: [PATCH 3/6] gemalto: acquire the network technology

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Gabriel,

On 03/15/2018 07:49 AM, Gabriel Lucas wrote:
> From: Mariem Cherif <mariem.cherif@ardia.com.tn>
> 
> ---
>   drivers/atmodem/network-registration.c | 45 ++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
> 
> diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
> index a5e2af3..aec9c2d 100644
> --- a/drivers/atmodem/network-registration.c
> +++ b/drivers/atmodem/network-registration.c
> @@ -48,6 +48,7 @@ static const char *cops_prefix[] = { "+COPS:", NULL };
>   static const char *csq_prefix[] = { "+CSQ:", NULL };
>   static const char *cind_prefix[] = { "+CIND:", NULL };
>   static const char *cmer_prefix[] = { "+CMER:", NULL };
> +static const char *smoni_prefix[] = { "^SMONI:", "", NULL };

Is there a reason why "" is added?  Is the modem sending lines not 
prefixed by '^SMONI:' in the response?

>   static const char *zpas_prefix[] = { "+ZPAS:", NULL };
>   static const char *option_tech_prefix[] = { "_OCTI:", "_OUWCTI:", NULL };
>   
> @@ -178,6 +179,32 @@ static int option_parse_tech(GAtResult *result)
>   	return tech;
>   }
>   
> +static int cinterion_parse_tech(GAtResult *result)
> +{
> +	int tech = -1;
> +	GAtResultIter iter;
> +	GSList *l;

new line here please

> +	g_at_result_iter_init(&iter, result);
> +	l = result->lines;

What does the output look like?

> +	if (strstr(l->data, "^SMONI: ") != NULL) {

Generally we use g_at_result_iter_next("^SMONI:");

> +		gchar **body = g_strsplit(l->data, "^SMONI: ", 2);
> +		if (*body != NULL) {
> +			gchar **data = g_strsplit(body[1], ",", 20);
> +			if (*data != NULL) {
> +				if (g_strcmp0(data[0], "2G") == 0) {
> +					tech = ACCESS_TECHNOLOGY_GSM;
> +				} else if (g_strcmp0 (data[0], "3G") == 0) {
> +					tech = ACCESS_TECHNOLOGY_UTRAN;
> +				} else if (g_strcmp0 (data[0], "4G") == 0) {
> +					tech = ACCESS_TECHNOLOGY_EUTRAN;
> +				}
> +			}

Are you leaking data?  Have you run this through valgrind?

> +			g_strfreev(body);

It seems all of this can be accomplished with 
g_at_result_iter_next_unquoted_string.

> +		}
> +	}
> +	return tech;
> +}
> +
>   static void at_creg_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   {
>   	struct cb_data *cbd = user_data;
> @@ -205,6 +232,18 @@ static void at_creg_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   	cb(&error, status, lac, ci, tech, cbd->data);
>   }
>   
> +static void cinterion_query_tech_cb(gboolean ok, GAtResult *result,
> +                                              gpointer user_data)
> +{
> +	struct tech_query *tq = user_data;
> +	int tech;
> +
> +	tech = cinterion_parse_tech(result);
> +
> +	ofono_netreg_status_notify(tq->netreg,
> +			tq->status, tq->lac, tq->ci, tech);
> +}
> +
>   static void zte_tech_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   {
>   	struct cb_data *cbd = user_data;
> @@ -1518,6 +1557,12 @@ static void creg_notify(GAtResult *result, gpointer user_data)
>   					option_query_tech_cb, tq, g_free) > 0)
>   			return;
>   		break;
> +    case OFONO_VENDOR_CINTERION:
> +              if (g_at_chat_send(nd->chat, "AT^SMONI",
> +                                      smoni_prefix,
> +                                      cinterion_query_tech_cb, tq, g_free) > 0)
> +                      return;
> +              break;
>   	}
>   
>   	g_free(tq);
> 

Regards,
-Denis