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

[Gabriel LUCAS]

Thanks Denis. My apologies for the unused variable.

On 2018-03-16 15:27, Denis Kenzior wrote:


Hi Gabriel, Mariem,

On 03/16/2018 07:59 AM, Gabriel Lucas wrote:


From: Mariem Cherif  ---
drivers/atmodem/network-registration.c | 45
++ 1 file changed, 45 insertions(+)


When applying this patch I got:

CC drivers/atmodem/network-registration.o
drivers/atmodem/network-registration.c: In function
'cinterion_parse_tech':
drivers/atmodem/network-registration.c:186:10: error: unused variable
'l' [-Werror=unused-variable]
GSList *l;
^
drivers/atmodem/network-registration.c: At top level:
cc1: error: unrecognized command line option '-Wno-format-truncation'
[-Werror]
cc1: all warnings being treated as errors

Please make sure you test with --disable-debug --enable-optimization
prior to submission so the compiler catches any unused variables.


It seems that my sdk's environment isn't configured to show all 
warnings.

I'm gonna fix that.

I went ahead and removed the offending GSList *l declaration and 
applied

this patch. Thanks!

Regards,
-Denis


Gabriel
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

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

[Denis Kenzior]

Hi Gabriel, Mariem,

On 03/16/2018 07:59 AM, Gabriel Lucas wrote:

From: Mariem Cherif 

---
  drivers/atmodem/network-registration.c | 45 ++
  1 file changed, 45 insertions(+)


When applying this patch I got:

  CC   drivers/atmodem/network-registration.o
drivers/atmodem/network-registration.c: In function ‘cinterion_parse_tech’:
drivers/atmodem/network-registration.c:186:10: error: unused variable 
‘l’ [-Werror=unused-variable]

  GSList *l;
  ^
drivers/atmodem/network-registration.c: At top level:
cc1: error: unrecognized command line option ‘-Wno-format-truncation’ 
[-Werror]

cc1: all warnings being treated as errors

Please make sure you test with --disable-debug --enable-optimization 
prior to submission so the compiler catches any unused variables.


I went ahead and removed the offending GSList *l declaration and applied 
this patch.  Thanks!


Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

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

[Gabriel Lucas]

From: Mariem Cherif 

---
 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..80f6291 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 };
 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;
+   const char *technology;
+
+   g_at_result_iter_init(, result);
+
+   if (!g_at_result_iter_next(, "^SMONI: "))
+   return tech;
+
+   if (!g_at_result_iter_next_unquoted_string(, ))
+   return tech;
+
+   if (strcmp(technology, "2G") == 0) {
+   tech = ACCESS_TECHNOLOGY_GSM_EGPRS;
+   } else if (strcmp(technology, "3G") == 0) {
+   tech = ACCESS_TECHNOLOGY_UTRAN;
+   } else if (strcmp(technology, "4G") == 0) {
+   tech = ACCESS_TECHNOLOGY_EUTRAN;
+   }
+
+   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(, 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);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

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

[Gabriel Lucas]

Hi Denis,

Really sorry, this patch is from Mariem and I haven't reviewed it.

the cinterion_parse_tech function really needs to be rework. I'll resend you
a cleaner version today.

On 15/03/2018 18:19, Denis Kenzior wrote:

Hi Gabriel,

On 03/15/2018 07:49 AM, Gabriel Lucas wrote:

From: Mariem Cherif 

---
  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(, 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.


Regards,
-Denis


___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

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

[Gabriel Lucas]

From: Mariem Cherif 

---
 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 };
 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;
+   g_at_result_iter_init(, result);
+   l = result->lines;
+   if (strstr(l->data, "^SMONI: ") != NULL) {
+   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;
+   }
+   }
+   g_strfreev(body);
+   }
+   }
+   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(, 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);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

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

[Gabriel Lucas]

From: Mariem Cherif 

---
 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 };
 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;
+   g_at_result_iter_init(, result);
+   l = result->lines;
+   if (strstr(l->data, "^SMONI: ") != NULL) {
+   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;
+   }
+   }
+   g_strfreev(body);
+   }
+   }
+   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(, 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);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

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

[Gabriel Lucas]

From: Mariem Cherif 

---
 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 };
 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;
+   g_at_result_iter_init(, result);
+   l = result->lines;
+   if (strstr(l->data, "^SMONI: ") != NULL) {
+   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;
+   }
+   }
+   g_strfreev(body);
+   }
+   }
+   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(, 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);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

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

[Gabriel Lucas]

From: Mariem Cherif 

---
 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 };
 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;
+   g_at_result_iter_init(, result);
+   l = result->lines;
+   if (strstr(l->data, "^SMONI: ") != NULL) {
+   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;
+   }
+   }
+   g_strfreev(body);
+   }
+   }
+   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(, 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);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono