date:20180316

Re: [PATCH] gemalto: Add more details in setup_gemalto comment

2018-03-16 Thread Denis Kenzior


Hi Bassem,

On 03/16/2018 12:21 PM, Bassem Boubaker wrote:

ALS3, PLS8-E and PLS8-X have same vid/pid with same enumeration process
---
  plugins/udevng.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)



Applied, thanks.

Regards,
-Denis

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

[PATCH] gemalto: Add more details in setup_gemalto comment

2018-03-16 Thread Bassem Boubaker

ALS3, PLS8-E and PLS8-X have same vid/pid with same enumeration process
---
 plugins/udevng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 6b5b015..ff5d41a 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1146,7 +1146,7 @@ static gboolean setup_gemalto(struct modem_info* modem)
qmi = info->devnode;
}
 
-   /* ALS3 */
+   /* Cinterion ALS3, PLS8-E, PLS8-X */
if (g_strcmp0(info->interface, "2/2/1") == 0) {
if (g_strcmp0(info->number, "00") == 0)
mdm = info->devnode;
-- 
2.7.4

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

Re: [PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

2018-03-16 Thread Denis Kenzior


Hi Gabriel,



@@ -300,29 +322,24 @@ static int gemalto_hardware_monitor_enable(struct 
ofono_modem *modem)
return 0;
  }
  
-static int gemalto_enable(struct ofono_modem *modem)

+static void gemalto_initialize(struct ofono_modem *modem)
  {
struct gemalto_data *data = ofono_modem_get_data(modem);
-   const char *app, *mdm;
+   const char *mdm;
  
-	DBG("%p", modem);

+   DBG("");
  
-	app = ofono_modem_get_string(modem, "Application");

mdm = ofono_modem_get_string(modem, "Modem");
  
-	if (app == NULL || mdm == NULL)

-   return -EINVAL;
+   if (mdm == NULL)
+   return;
  
  	/* Open devices */

-   data->app = open_device(app);
-   if (data->app == NULL)
-   return -EINVAL;
-
data->mdm = open_device(mdm);
if (data->mdm == NULL) {
g_at_chat_unref(data->app);
data->app = NULL;
-   return -EINVAL;
+   return;
}
  
  	if (getenv("OFONO_AT_DEBUG")) {

@@ -340,6 +357,67 @@ static int gemalto_enable(struct ofono_modem *modem)
cfun_enable, modem, NULL);
  
  	gemalto_hardware_monitor_enable(modem);

+}
+
+static void gemalto_modem_ready(GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+   const char *app = ofono_modem_get_string(modem, "Application");
+
+   DBG("");
+
+   g_at_chat_unregister(data->app, data->modem_ready_id);
+   g_at_chat_cancel(data->app, data->trial_cmd_id);


Since you're unrefing data->app below, this isn't really necessary.  You 
might want to reset the ids to zero here though.



+
+   /*!
+* As the modem wasn't ready to handle AT commands when we opened
+* it, we have to close and reopen the device app.
+	 */ 
+	g_at_chat_unref(data->app);

+
+   if(app == NULL)
+   return;


Not our coding style.  Also, the "Application" property should not 
disappear under you, so I would leave this check out.



+
+   data->app = open_device(app);
+   if (data->app == NULL)
+   return;


You might want to tell the core that the enable operation failed.  So 
ofono_modem_set_powered(modem, FALSE);



+
+   gemalto_initialize(modem);
+}
+
+static void gemalto_at_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+
+   g_at_chat_unregister(data->app, data->modem_ready_id);
+   gemalto_initialize(modem);


Do you also want to reset modem_ready_id to 0 as well?  And trial_cmd_id


+}
+
+static int gemalto_enable(struct ofono_modem *modem)
+{
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+   const char *app, *mdm;
+
+   DBG("%p", modem);
+
+   app = ofono_modem_get_string(modem, "Application");
+   mdm = ofono_modem_get_string(modem, "Modem");
+
+   if (app == NULL || mdm == NULL)
+   return -EINVAL;
+
+   /* Open devices */
+   data->app = open_device(app);
+   if (data->app == NULL)
+   return -EINVAL;
+
+   /* Try the AT command. If it doesn't work, wait for ^SYSSTART */
+   data->modem_ready_id = g_at_chat_register(data->app, "^SYSSTART",
+   gemalto_modem_ready, FALSE, modem, NULL);
+   data->trial_cmd_id = g_at_chat_send(data->app, "ATE0 AT",
+   none_prefix, gemalto_at_cb, modem, NULL);
  
  	return -EINPROGRESS;

  }
@@ -432,6 +510,7 @@ static void gemalto_post_sim(struct ofono_modem *modem)
struct gemalto_data *data = ofono_modem_get_data(modem);
struct ofono_gprs *gprs;
struct ofono_gprs_context *gc;
+   const char *model = ofono_modem_get_string(modem, "Model");
  
  	DBG("%p", modem);
  
@@ -444,6 +523,9 @@ static void gemalto_post_sim(struct ofono_modem *modem)
  
  	if (gprs && gc)

ofono_gprs_add_context(gprs, gc);
+
+   if (!g_strcmp0(model, GEMALTO_MODEL_ALS3))
+   ofono_lte_create(modem, OFONO_VENDOR_CINTERION, "atmodem", 
data->app);


Over 80 characters here


  }
  
  static void gemalto_post_online(struct ofono_modem *modem)




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

[PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

2018-03-16 Thread Gabriel Lucas

Force serial port opening options
Wait for modem to be ready to start
initializing it
Handle LTE
---
 plugins/gemalto.c | 107 --
 1 file changed, 95 insertions(+), 12 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index 3739d7b..3fb2904 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -53,6 +53,10 @@
 
 #define HARDWARE_MONITOR_INTERFACE OFONO_SERVICE ".cinterion.HardwareMonitor"
 
+/* Supported gemalto's modem */
+#define GEMALTO_MODEL_PHS8P"0053"
+#define GEMALTO_MODEL_ALS3 "0061"
+
 static const char *none_prefix[] = { NULL };
 static const char *sctm_prefix[] = { "^SCTM:", NULL };
 static const char *sbv_prefix[] = { "^SBV:", NULL };
@@ -70,6 +74,8 @@ struct gemalto_data {
gboolean have_sim;
struct at_util_sim_state_query *sim_state_query;
struct gemalto_hardware_monitor *hm;
+   guint modem_ready_id;
+   guint trial_cmd_id;
 };
 
 static int gemalto_probe(struct ofono_modem *modem)
@@ -107,10 +113,26 @@ static GAtChat *open_device(const char *device)
GAtSyntax *syntax;
GIOChannel *channel;
GAtChat *chat;
+   GHashTable *options;
+
+   options = g_hash_table_new(g_str_hash, g_str_equal);
+   if (options == NULL)
+   return NULL;
+
+   g_hash_table_insert(options, "Baud", "115200");
+   g_hash_table_insert(options, "StopBits", "1");
+   g_hash_table_insert(options, "DataBits", "8");
+   g_hash_table_insert(options, "Parity", "none");
+   g_hash_table_insert(options, "XonXoff", "off");
+   g_hash_table_insert(options, "RtsCts", "on");
+   g_hash_table_insert(options, "Local", "on");
+   g_hash_table_insert(options, "Read", "on");
 
DBG("Opening device %s", device);
 
-   channel = g_at_tty_open(device, NULL);
+   channel = g_at_tty_open(device, options);
+   g_hash_table_destroy(options);
+
if (channel == NULL)
return NULL;
 
@@ -300,29 +322,24 @@ static int gemalto_hardware_monitor_enable(struct 
ofono_modem *modem)
return 0;
 }
 
-static int gemalto_enable(struct ofono_modem *modem)
+static void gemalto_initialize(struct ofono_modem *modem)
 {
struct gemalto_data *data = ofono_modem_get_data(modem);
-   const char *app, *mdm;
+   const char *mdm;
 
-   DBG("%p", modem);
+   DBG("");
 
-   app = ofono_modem_get_string(modem, "Application");
mdm = ofono_modem_get_string(modem, "Modem");
 
-   if (app == NULL || mdm == NULL)
-   return -EINVAL;
+   if (mdm == NULL)
+   return;
 
/* Open devices */
-   data->app = open_device(app);
-   if (data->app == NULL)
-   return -EINVAL;
-
data->mdm = open_device(mdm);
if (data->mdm == NULL) {
g_at_chat_unref(data->app);
data->app = NULL;
-   return -EINVAL;
+   return;
}
 
if (getenv("OFONO_AT_DEBUG")) {
@@ -340,6 +357,67 @@ static int gemalto_enable(struct ofono_modem *modem)
cfun_enable, modem, NULL);
 
gemalto_hardware_monitor_enable(modem);
+}
+
+static void gemalto_modem_ready(GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+   const char *app = ofono_modem_get_string(modem, "Application");
+
+   DBG("");
+
+   g_at_chat_unregister(data->app, data->modem_ready_id);
+   g_at_chat_cancel(data->app, data->trial_cmd_id);
+
+   /*!
+* As the modem wasn't ready to handle AT commands when we opened
+* it, we have to close and reopen the device app.
+*/
+   g_at_chat_unref(data->app);
+
+   if(app == NULL)
+   return;
+
+   data->app = open_device(app);
+   if (data->app == NULL)
+   return;
+
+   gemalto_initialize(modem);
+}
+
+static void gemalto_at_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+
+   g_at_chat_unregister(data->app, data->modem_ready_id);
+   gemalto_initialize(modem);
+}
+
+static int gemalto_enable(struct ofono_modem *modem)
+{
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+   const char *app, *mdm;
+
+   DBG("%p", modem);
+
+   app = ofono_modem_get_string(modem, "Application");
+   mdm = ofono_modem_get_string(modem, "Modem");
+
+   if (app == NULL || mdm == NULL)
+   return -EINVAL;
+
+   /* Open devices */
+   data->app = open_device(app);
+   if (data->app == NULL)
+   return -EINVAL;
+
+   /* Try the AT command. If it doesn't work, wait for ^SYSSTART */
+   data->modem_ready_id = g_at_chat_register(data->app, "^SYSSTART",
+   gemalto_modem_ready,

[PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

2018-03-16 Thread Gabriel Lucas

Force serial port opening options
Wait for modem to be ready to start
initializing it
Handle LTE
---
 plugins/gemalto.c | 106 +++---
 1 file changed, 94 insertions(+), 12 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index 3739d7b..af50a9b 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -53,6 +53,10 @@
 
 #define HARDWARE_MONITOR_INTERFACE OFONO_SERVICE ".cinterion.HardwareMonitor"
 
+/* Supported gemalto's modem */
+#define GEMALTO_MODEL_PHS8P"0053"
+#define GEMALTO_MODEL_ALS3 "0061"
+
 static const char *none_prefix[] = { NULL };
 static const char *sctm_prefix[] = { "^SCTM:", NULL };
 static const char *sbv_prefix[] = { "^SBV:", NULL };
@@ -70,6 +74,8 @@ struct gemalto_data {
gboolean have_sim;
struct at_util_sim_state_query *sim_state_query;
struct gemalto_hardware_monitor *hm;
+   guint modem_ready_id;
+   guint trial_cmd_id;
 };
 
 static int gemalto_probe(struct ofono_modem *modem)
@@ -107,10 +113,26 @@ static GAtChat *open_device(const char *device)
GAtSyntax *syntax;
GIOChannel *channel;
GAtChat *chat;
+   GHashTable *options;
+
+   options = g_hash_table_new(g_str_hash, g_str_equal);
+   if (options == NULL)
+   return NULL;
+
+   g_hash_table_insert(options, "Baud", "115200");
+   g_hash_table_insert(options, "StopBits", "1");
+   g_hash_table_insert(options, "DataBits", "8");
+   g_hash_table_insert(options, "Parity", "none");
+   g_hash_table_insert(options, "XonXoff", "off");
+   g_hash_table_insert(options, "RtsCts", "on");
+   g_hash_table_insert(options, "Local", "on");
+   g_hash_table_insert(options, "Read", "on");
 
DBG("Opening device %s", device);
 
-   channel = g_at_tty_open(device, NULL);
+   channel = g_at_tty_open(device, options);
+   g_hash_table_destroy(options);
+
if (channel == NULL)
return NULL;
 
@@ -300,29 +322,24 @@ static int gemalto_hardware_monitor_enable(struct 
ofono_modem *modem)
return 0;
 }
 
-static int gemalto_enable(struct ofono_modem *modem)
+static void gemalto_initialize(struct ofono_modem *modem)
 {
struct gemalto_data *data = ofono_modem_get_data(modem);
-   const char *app, *mdm;
+   const char *mdm;
 
-   DBG("%p", modem);
+   DBG("");
 
-   app = ofono_modem_get_string(modem, "Application");
mdm = ofono_modem_get_string(modem, "Modem");
 
-   if (app == NULL || mdm == NULL)
-   return -EINVAL;
+   if (mdm == NULL)
+   return;
 
/* Open devices */
-   data->app = open_device(app);
-   if (data->app == NULL)
-   return -EINVAL;
-
data->mdm = open_device(mdm);
if (data->mdm == NULL) {
g_at_chat_unref(data->app);
data->app = NULL;
-   return -EINVAL;
+   return;
}
 
if (getenv("OFONO_AT_DEBUG")) {
@@ -340,6 +357,67 @@ static int gemalto_enable(struct ofono_modem *modem)
cfun_enable, modem, NULL);
 
gemalto_hardware_monitor_enable(modem);
+}
+
+static void gemalto_modem_ready(GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+   const char *app = ofono_modem_get_string(modem, "Application");
+
+   DBG("");
+
+   g_at_chat_unregister(data->app, data->modem_ready_id);
+   g_at_chat_cancel(data->app, data->trial_cmd_id);
+
+   /*!
+* As the modem wasn't ready to handle AT commands when we opened
+* it, we have to close and reopen the device app.
+*/
+   g_at_chat_unref(data->app);
+
+   if(app == NULL)
+   return;
+
+   data->app = open_device(app);
+   if (data->app == NULL)
+   return;
+
+   gemalto_initialize(modem);
+}
+
+static void gemalto_at_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+
+   g_at_chat_unregister(data->app, data->modem_ready_id);
+   gemalto_initialize(modem);
+}
+
+static int gemalto_enable(struct ofono_modem *modem)
+{
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+   const char *app, *mdm;
+
+   DBG("%p", modem);
+
+   app = ofono_modem_get_string(modem, "Application");
+   mdm = ofono_modem_get_string(modem, "Modem");
+
+   if (app == NULL || mdm == NULL)
+   return -EINVAL;
+
+   /* Open devices */
+   data->app = open_device(app);
+   if (data->app == NULL)
+   return -EINVAL;
+
+   /* Try the AT command. If it doesn't work, wait for ^SYSSTART */
+   data->modem_ready_id = g_at_chat_register(data->app, "^SYSSTART",
+   gemalto_modem_ready,

[PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

2018-03-16 Thread Gabriel Lucas

Force serial port opening options
Wait for modem to be ready to start
initializing it
Handle LTE
---
 plugins/gemalto.c | 106 +++---
 1 file changed, 94 insertions(+), 12 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index 3739d7b..af50a9b 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -53,6 +53,10 @@
 
 #define HARDWARE_MONITOR_INTERFACE OFONO_SERVICE ".cinterion.HardwareMonitor"
 
+/* Supported gemalto's modem */
+#define GEMALTO_MODEL_PHS8P"0053"
+#define GEMALTO_MODEL_ALS3 "0061"
+
 static const char *none_prefix[] = { NULL };
 static const char *sctm_prefix[] = { "^SCTM:", NULL };
 static const char *sbv_prefix[] = { "^SBV:", NULL };
@@ -70,6 +74,8 @@ struct gemalto_data {
gboolean have_sim;
struct at_util_sim_state_query *sim_state_query;
struct gemalto_hardware_monitor *hm;
+   guint modem_ready_id;
+   guint trial_cmd_id;
 };
 
 static int gemalto_probe(struct ofono_modem *modem)
@@ -107,10 +113,26 @@ static GAtChat *open_device(const char *device)
GAtSyntax *syntax;
GIOChannel *channel;
GAtChat *chat;
+   GHashTable *options;
+
+   options = g_hash_table_new(g_str_hash, g_str_equal);
+   if (options == NULL)
+   return NULL;
+
+   g_hash_table_insert(options, "Baud", "115200");
+   g_hash_table_insert(options, "StopBits", "1");
+   g_hash_table_insert(options, "DataBits", "8");
+   g_hash_table_insert(options, "Parity", "none");
+   g_hash_table_insert(options, "XonXoff", "off");
+   g_hash_table_insert(options, "RtsCts", "on");
+   g_hash_table_insert(options, "Local", "on");
+   g_hash_table_insert(options, "Read", "on");
 
DBG("Opening device %s", device);
 
-   channel = g_at_tty_open(device, NULL);
+   channel = g_at_tty_open(device, options);
+   g_hash_table_destroy(options);
+
if (channel == NULL)
return NULL;
 
@@ -300,29 +322,24 @@ static int gemalto_hardware_monitor_enable(struct 
ofono_modem *modem)
return 0;
 }
 
-static int gemalto_enable(struct ofono_modem *modem)
+static void gemalto_initialize(struct ofono_modem *modem)
 {
struct gemalto_data *data = ofono_modem_get_data(modem);
-   const char *app, *mdm;
+   const char *mdm;
 
-   DBG("%p", modem);
+   DBG("");
 
-   app = ofono_modem_get_string(modem, "Application");
mdm = ofono_modem_get_string(modem, "Modem");
 
-   if (app == NULL || mdm == NULL)
-   return -EINVAL;
+   if (mdm == NULL)
+   return;
 
/* Open devices */
-   data->app = open_device(app);
-   if (data->app == NULL)
-   return -EINVAL;
-
data->mdm = open_device(mdm);
if (data->mdm == NULL) {
g_at_chat_unref(data->app);
data->app = NULL;
-   return -EINVAL;
+   return;
}
 
if (getenv("OFONO_AT_DEBUG")) {
@@ -340,6 +357,67 @@ static int gemalto_enable(struct ofono_modem *modem)
cfun_enable, modem, NULL);
 
gemalto_hardware_monitor_enable(modem);
+}
+
+static void gemalto_modem_ready(GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+   const char *app = ofono_modem_get_string(modem, "Application");
+
+   DBG("");
+
+   g_at_chat_unregister(data->app, data->modem_ready_id);
+   g_at_chat_cancel(data->app, data->trial_cmd_id);
+
+   /*!
+* As the modem wasn't ready to handle AT commands when we opened
+* it, we have to close and reopen the device app.
+*/
+   g_at_chat_unref(data->app);
+
+   if(app == NULL)
+   return;
+
+   data->app = open_device(app);
+   if (data->app == NULL)
+   return;
+
+   gemalto_initialize(modem);
+}
+
+static void gemalto_at_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+
+   g_at_chat_unregister(data->app, data->modem_ready_id);
+   gemalto_initialize(modem);
+}
+
+static int gemalto_enable(struct ofono_modem *modem)
+{
+   struct gemalto_data *data = ofono_modem_get_data(modem);
+   const char *app, *mdm;
+
+   DBG("%p", modem);
+
+   app = ofono_modem_get_string(modem, "Application");
+   mdm = ofono_modem_get_string(modem, "Modem");
+
+   if (app == NULL || mdm == NULL)
+   return -EINVAL;
+
+   /* Open devices */
+   data->app = open_device(app);
+   if (data->app == NULL)
+   return -EINVAL;
+
+   /* Try the AT command. If it doesn't work, wait for ^SYSSTART */
+   data->modem_ready_id = g_at_chat_register(data->app, "^SYSSTART",
+

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

2018-03-16 Thread 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] Fix bug with "PropertyChanged" signals not being emmitted by ofono for org.ofono.cdma.NetworkRegistration

2018-03-16 Thread Denis Kenzior


Hi Bassem,

On 03/16/2018 10:24 AM, Bassem Boubaker wrote:

Signed-off-by: Bassem Boubaker 


We don't use Signed-off-by, so please update your configuration not to 
include this.  I've removed this manually and ...



---
  src/cdma-netreg.c | 2 ++
  1 file changed, 2 insertions(+)


patch applied, thanks!

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

[PATCH] Fix bug with "PropertyChanged" signals not being emmitted by ofono for org.ofono.cdma.NetworkRegistration

2018-03-16 Thread Bassem Boubaker

Signed-off-by: Bassem Boubaker 
---
 src/cdma-netreg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/cdma-netreg.c b/src/cdma-netreg.c
index ba9ee23..23616d9 100644
--- a/src/cdma-netreg.c
+++ b/src/cdma-netreg.c
@@ -115,6 +115,8 @@ static const GDBusMethodTable cdma_netreg_manager_methods[] 
= {
 };
 
 static const GDBusSignalTable cdma_netreg_manager_signals[] = {
+   { GDBUS_SIGNAL("PropertyChanged",
+   GDBUS_ARGS({ "name", "s" }, { "value", "v" })) },
{ }
 };
 
-- 
2.7.4

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

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

2018-03-16 Thread 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

Re: [PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

2018-03-16 Thread Denis Kenzior


Hi Gabriel,


  { >>>   struct gemalto_data *data = ofono_modem_get_data(modem);
  const char *app, *mdm;
  -    DBG("%p", modem);
+    /* Close devices */
+    g_at_chat_unref(data->mdm);
+    g_at_chat_unref(data->app);
+


Why would you close both devices?  It is unnecessary, especially in 
the case that the modem is already up and responding to commands.
Because it seems that opening the device while it is not ready leads the 
driver to a weird state.
When the modem becomes ready, I am not able to send any AT command to 
the APP device.

Here is the output:

ofonod[12826]: src/modem.c:get_modem_property() modem 0x72b750 property 
Application
ofonod[12826]: src/modem.c:get_modem_property() modem 0x72b750 property 
Modem

ofonod[12826]: plugins/gemalto.c:open_device() Opening device /dev/ttyACM1
ofonod[12826]: plugins/gemalto.c:open_device() Opening device /dev/ttyACM0
ofonod[12826]: plugins/gemalto.c:gemalto_modem_ready()
ofonod[12826]: plugins/gemalto.c:gemalto_initialize()
ofonod[12826]: src/modem.c:get_modem_property() modem 0x72b750 property 
Application
ofonod[12826]: src/modem.c:get_modem_property() modem 0x72b750 property 
Modem

ofonod[12826]: plugins/gemalto.c:gemalto_hardware_monitor_enable()
ofonod[12826]: Mdm> ATE0\r
ofonod[12826]: Mdm< ATE0\r\r\nOK\r\n
ofonod[12826]: Mdm> AT\r
ofonod[12826]: Mdm< \r\n+CME ERROR: operation not supported\r\n
ofonod[12826]: src/modem.c:set_powered_timeout() modem: 0x72b750

When it is re-opened, it works fine.


So my question was two fold:

1. Why would you open both devices?  Wouldn't opening just one to figure 
out whether the modem is 'ready' or not be enough?
2. If the modem is ready, why would you close the opened device?  That 
just seems to be a waste of time.


If the device isn't ready then I could see how re-opening the port might 
be required.



+
+    if(!strncmp(model, GEMALTO_MODEL_ALS3, 4)) {
+    ofono_lte_create(modem, "atmodem", data->app);
+    }


Why strncmp?  No need for {} and there should be a space between if 
and '('
Because strncmp is more secured than strcmp. But if you prefer strcmp or 
anything else, I'll change it.


In this case they're the same and its not like you have user input to 
worry about here...


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

Re: [PATCH 4/6] gemalto: handle sim is inserted or removed URCs

2018-03-16 Thread Denis Kenzior


Hi Gabriel,

On 03/16/2018 08:28 AM, Gabriel Lucas wrote:

Hi Denis,

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

Hi Gabriel,

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

From: Mariem Cherif 

+    g_at_result_iter_init(, result);
+
+    if (!g_at_result_iter_next(, "+CIEV:"))
+    return;
+


So generally +CIEV indication is an , syntax.

We receive this kind of signal: +CIEV: simstatus,.
In this code, Mariem is ensuring the index is simstatus.
Do you prefer us to use g_at_result_iter_next to handle the index ?
Or even to use "+CIEV: simstatus" as prefix in the register function ?


Okay, that makes sense now.  Gemalto should really fix their firmware as 
this syntax is not really correct, but nothing we can do about it now. 
I would stick with the current approach.



+    if (!g_at_result_iter_next_unquoted_string(, _str))
+    return;


Does Gemalto use some other syntax here?  If so, you might want to 
document a simple example above.

I've added a comment

+    ofono_sim_inserted_notify(sim, FALSE);
+    } else if(status == 1) {
+    ofono_sim_inserted_notify(sim, TRUE);
+    }


Okay, but simpler written as ofono_sim_inserted_notify(sim, status);
The thing is we don't only receive the status 0 and 1. We also have 4 
(ALS3) and

5 (ALS3 + PHS8P). Only the first two should be used.


In that case I would use a switch/case statement, add a case label 
/comment for the other possibilities and what they mean and simply use a 
'break' statement for those.


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

Re: [PATCH 6/6] gemalto: fix sim reinsertion issue

2018-03-16 Thread Gabriel Lucas


Hi Denis,

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

Hi Gabriel,

Why don't you just use at_util_sim_state_query_new here?


Well, because I didn't know it existed :).
Thanks, I going to use it.

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

Re: [PATCH 4/6] gemalto: handle sim is inserted or removed URCs

2018-03-16 Thread Gabriel Lucas


Hi Denis,

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

Hi Gabriel,

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

From: Mariem Cherif 

+    g_at_result_iter_init(, result);
+
+    if (!g_at_result_iter_next(, "+CIEV:"))
+    return;
+


So generally +CIEV indication is an , syntax.

We receive this kind of signal: +CIEV: simstatus,.
In this code, Mariem is ensuring the index is simstatus.
Do you prefer us to use g_at_result_iter_next to handle the index ?
Or even to use "+CIEV: simstatus" as prefix in the register function ?

+    if (!g_at_result_iter_next_unquoted_string(, _str))
+    return;


Does Gemalto use some other syntax here?  If so, you might want to 
document a simple example above.

I've added a comment

+    ofono_sim_inserted_notify(sim, FALSE);
+    } else if(status == 1) {
+    ofono_sim_inserted_notify(sim, TRUE);
+    }


Okay, but simpler written as ofono_sim_inserted_notify(sim, status);
The thing is we don't only receive the status 0 and 1. We also have 4 
(ALS3) and

5 (ALS3 + PHS8P). Only the first two should be used.

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

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

2018-03-16 Thread 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

2018-03-16 Thread 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

Re: [PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

2018-03-16 Thread Gabriel Lucas


Hi Denis,

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

No // please, use /* */.  This is C code ;)

Fixed. I've just learned it was not ANSI C :).


You leak options here in case channel == NULL.  The typical pattern is 
to call g_hash_table_destroy(options) right after g_at_tty_open.  See 
other plugins for details.

Fixed



  @@ -118,6 +138,7 @@ static GAtChat *open_device(const char *device)
  chat = g_at_chat_new(channel, syntax);
  g_at_syntax_unref(syntax);
  g_io_channel_unref(channel);
+    g_hash_table_destroy(options);
    if (chat == NULL)
  return NULL;
@@ -300,29 +321,33 @@ static int 
gemalto_hardware_monitor_enable(struct ofono_modem *modem)

  return 0;
  }
  -static int gemalto_enable(struct ofono_modem *modem)
+static void gemalto_initialize(struct ofono_modem *modem)
  {
  struct gemalto_data *data = ofono_modem_get_data(modem);
  const char *app, *mdm;
  -    DBG("%p", modem);
+    /* Close devices */
+    g_at_chat_unref(data->mdm);
+    g_at_chat_unref(data->app);
+


Why would you close both devices?  It is unnecessary, especially in 
the case that the modem is already up and responding to commands.
Because it seems that opening the device while it is not ready leads the 
driver to a weird state.
When the modem becomes ready, I am not able to send any AT command to 
the APP device.

Here is the output:

ofonod[12826]: src/modem.c:get_modem_property() modem 0x72b750 property 
Application
ofonod[12826]: src/modem.c:get_modem_property() modem 0x72b750 property 
Modem

ofonod[12826]: plugins/gemalto.c:open_device() Opening device /dev/ttyACM1
ofonod[12826]: plugins/gemalto.c:open_device() Opening device /dev/ttyACM0
ofonod[12826]: plugins/gemalto.c:gemalto_modem_ready()
ofonod[12826]: plugins/gemalto.c:gemalto_initialize()
ofonod[12826]: src/modem.c:get_modem_property() modem 0x72b750 property 
Application
ofonod[12826]: src/modem.c:get_modem_property() modem 0x72b750 property 
Modem

ofonod[12826]: plugins/gemalto.c:gemalto_hardware_monitor_enable()
ofonod[12826]: Mdm> ATE0\r
ofonod[12826]: Mdm< ATE0\r\r\nOK\r\n
ofonod[12826]: Mdm> AT\r
ofonod[12826]: Mdm< \r\n+CME ERROR: operation not supported\r\n
ofonod[12826]: src/modem.c:set_powered_timeout() modem: 0x72b750

When it is re-opened, it works fine.



+    DBG("");
    app = ofono_modem_get_string(modem, "Application");
  mdm = ofono_modem_get_string(modem, "Modem");
    if (app == NULL || mdm == NULL)
-    return -EINVAL;
+    return;
    /* Open devices */
  data->app = open_device(app);
  if (data->app == NULL)
-    return -EINVAL;
+    return;
    data->mdm = open_device(mdm);
  if (data->mdm == NULL) {
  g_at_chat_unref(data->app);
  data->app = NULL;
-    return -EINVAL;
+    return;
  }
    if (getenv("OFONO_AT_DEBUG")) {
@@ -340,6 +365,67 @@ static int gemalto_enable(struct ofono_modem 
*modem)

  cfun_enable, modem, NULL);
    gemalto_hardware_monitor_enable(modem);
+}
+
+static void gemalto_modem_ready(GAtResult *result, gpointer user_data)
+{
+    struct ofono_modem *modem = user_data;
+    struct gemalto_data *data = ofono_modem_get_data(modem);
+
+    DBG("");
+
+    g_at_chat_unregister(data->app, data->modem_ready_id);
+    g_at_chat_cancel(data->app, data->trial_cmd_id);
+
+    gemalto_initialize(modem);
+}
+
+static void gemalto_at_cb(gboolean ok, GAtResult *result, gpointer 
user_data)

+{
+    struct ofono_modem *modem = user_data;
+    struct gemalto_data *data = ofono_modem_get_data(modem);
+
+    g_at_chat_unregister(data->app, data->modem_ready_id);
+    gemalto_initialize(modem);
+}
+
+static gboolean gemalto_at_timeout(gpointer user_data)
+{
+    struct ofono_modem *modem = user_data;
+    struct gemalto_data *data = ofono_modem_get_data(modem);
+
+    data->trial_cmd_id = g_at_chat_send(data->app, "AT+CFUN?", 
none_prefix, gemalto_at_cb, modem, NULL);


What does this do?
Some gemalto's modems have their USB devices mounted before they are 
ready to handle AT command.
At modem's or oFono's startup, the modem is either ready or not. In 
gemalto_enable, we register to the "^SYSSTART" URC and send the "AT" 
command.
- If the AT command returns, then the modem is ready so we unregister 
the "^SYSSTART" URC and start initializing the modem.
- If the modem isn't ready, the AT command will never return so we have 
to wait for the "^SYSSTART" URC. When it is received, we cancel the AT
command because it will never get any answer and start initializing the 
modem.


I've no cleaner way to do that.



+
+    return FALSE;
+}


Where is the timeout started?

Seems like I forgot to clean some parts of the code ;). There is no timeout.


We use the Linux Kernel coding style with house rules documented in 
doc/coding-style.txt.  No lines over 80 characters please.

Fixed


Why do you open both ports only to close them in gemalto_initialize? 
One port here would be sufficient.
You are right,

Re: [PATCH] gemalto: Add more details in setup_gemalto comment

[PATCH] gemalto: Add more details in setup_gemalto comment

Re: [PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

[PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

[PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

[PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

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

Re: [PATCH] Fix bug with "PropertyChanged" signals not being emmitted by ofono for org.ofono.cdma.NetworkRegistration

[PATCH] Fix bug with "PropertyChanged" signals not being emmitted by ofono for org.ofono.cdma.NetworkRegistration

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

Re: [PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

Re: [PATCH 4/6] gemalto: handle sim is inserted or removed URCs

Re: [PATCH 6/6] gemalto: fix sim reinsertion issue

Re: [PATCH 4/6] gemalto: handle sim is inserted or removed URCs

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

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

Re: [PATCH 2/6] gemalto: support ALS3 in gemalto's plugin

17 matches

Site Navigation

Mail list logo

Footer information