RE: [PATCH 1/3] ifx: integration of kernel mux (n_gsm) and its line discipline

2011-03-14 Thread Lucas, GuillaumeX
Hi Marcel,

 Hi Guillaume,
 
diff --git a/plugins/ifx.c b/plugins/ifx.c
index 527a8c4..9c2a5c1 100644
--- a/plugins/ifx.c
+++ b/plugins/ifx.c
@@ -26,8 +26,12 @@
 #include stdio.h
 #include errno.h
 #include stdlib.h
+#include string.h
 #include sys/stat.h
 #include sys/ioctl.h
+#include linux/types.h
+#include linux/gsmmux.h
+#include unistd.h
  
   please move unistd.h up since as a rule of thumb, the system headers
   should be included before the unix ones.
  
   Also gsmmux.h is not acceptable. We have to carry a local copy for a
   while since otherwise compilation would fail on older kernels.
 
  Ok. It's what Robertino did in his first patch for the kernel mux 
  integration.
  I've changed that thinking it was better to used the gsmmux.h of the kernel 
  tree. 
  I'll redo that.
 
  
   Why do you need linux/types.h?
  
 
  Do not really remember :(
  I think this is for the gsmmux.h file but if it's the case it to this file 
  to 
  include this one. I'll review that.
 
 in theory if gsmmux.h requires linux/types.h, then it should include
 it.
 

I've asked the team who handle this file to add this missing include.

   
+   options = g_hash_table_new(g_str_hash, g_str_equal);
+   if (options == NULL)
+   goto error;
+
+   g_hash_table_insert(options, Local, on);
+
  
   Why is this needed?
 
  This is needed for PDP context deactivation.
  If you don't set the local to on a tty close occurs when you do the PDP 
  context deactivation.
 
 That sounds like a kernel bug to me as well. Why is the kernel closing
 the TTY if you switch back to AT command mode. We are still holding an
 open file descriptor to it.
 

It's the kernel mux driver that makes a TTY hang-up when the Local option is 
set to OFF.
The TTY hang-up is done because the modem line TIOCM_CD moves from 0 to 1 (DV 
bit of the modem status command set to 1).
It's what I've seen in the code, I'm not enough expert of 27.010 and TTY to be 
able to say if this correct or not.
Regarding the SPEC the mapping done in the kernel mux between 27.010 control 
signals and TTY signals seems correct for me. 

   
/* Enable multiplexer */
-   data-frame_size = 1509;
+   data-frame_size = 1500;
   
-   g_at_chat_send(chat, AT+CMUX=0,0,,1509,10,3,30,,, NULL,
+   g_at_chat_send(chat, AT+CMUX=0,0,,1500,10,3,30,,, NULL,
mux_setup_cb, modem, NULL);
  
   So why did the frame_size changed here? Does the modem firmware changed
   or is this actual a bug in oFono already today?
  
 
  It's a bug in oFono.
 
 Then lets get that fixed separately with a proper commit message
 explaining why using frame size of 1500 is correct.
 
 We need a proper track record of changes and don't intermix them.
 

I'll do that.

Regards,
Guillaume
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH 3/3] ifx: fix issue with closing of DLC_AUX tty channel

2011-03-10 Thread Lucas, GuillaumeX
Hi Marcel,

 Hi Guillaume,
 
  Using kernel mux, a recursive locking is detected. This issue is
  due to the fact that the DLC_AUX is not closed by ofono. This is
  not done because the shutdown_device() function in charge of it
  is called in the context of an AT callback function and for an
  AT sent in this same dlc. Calling the real shutdown_device()
  function using a timer solved the issue.
 
 I am not following here. Are you trying to fix a kernel locking with a
 userspace workaround? That is not really acceptable. It is the kernel's
 responsibility to cleanup after userspace. Not the other way around.
 

No it's not what I've try to fix. I realize that my explanation is not very 
clear :(
The real issue fixed here is the non-closing of the dlc when this one is did in 
the callback context of an AT command.

The lock issue that we have seen with the kernel mux is directly linked to this 
non-closing and I agree with you that this kind of issue is not acceptable. 
Kernel mux team is currently working on it.

About the non-closing issue itself. What I've seen is that if you do a 
g_at_chat_unref() for a dlc in the callback function of a g_at_chat_send() done 
on the same dlc, the tty linked to the dlc is not closed. This is typically 
what we do in the ifx.c code for the modem release.
If you change the g_at_chat_send() call to remove the callback and do the 
g_at_chat_unref() just after it, the tty is correctly closed. Same if instead 
to do the g_at_chat_unref() directly in the callback you do it in a timer 
callback.
This last one method is what I've did in my patch.

Regards,
Guillaume
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH 1/3] ifx: integration of kernel mux (n_gsm) and its line discipline

2011-03-10 Thread Lucas, GuillaumeX
Hi Marcel,

 Hi Lucas,
 
  diff --git a/plugins/ifx.c b/plugins/ifx.c
  index 527a8c4..9c2a5c1 100644
  --- a/plugins/ifx.c
  +++ b/plugins/ifx.c
  @@ -26,8 +26,12 @@
   #include stdio.h
   #include errno.h
   #include stdlib.h
  +#include string.h
   #include sys/stat.h
   #include sys/ioctl.h
  +#include linux/types.h
  +#include linux/gsmmux.h
  +#include unistd.h
 
 please move unistd.h up since as a rule of thumb, the system headers
 should be included before the unix ones.
 
 Also gsmmux.h is not acceptable. We have to carry a local copy for a
 while since otherwise compilation would fail on older kernels.

Ok. It's what Robertino did in his first patch for the kernel mux integration.
I've changed that thinking it was better to used the gsmmux.h of the kernel 
tree. I'll redo that.

 
 Why do you need linux/types.h?
 

Do not really remember :(
I think this is for the gsmmux.h file but if it's the case it to this file to 
include this one. I'll review that.

   #include glib.h
   #include gatchat.h
  @@ -75,9 +79,9 @@
   static char *dlc_prefixes[NUM_DLC] = { Voice: , Net: , GPRS1:
 ,
  GPRS2: , GPRS3: , Aux:  };
 
  -static const char *dlc_nodes[NUM_DLC] = { /dev/ttyGSM1,
 /dev/ttyGSM2,
  -   /dev/ttyGSM3, /dev/ttyGSM4,
  -   /dev/ttyGSM5, /dev/ttyGSM6 };
  +static const char *dlc_nodes[NUM_DLC] = { /dev/gsmtty1,
 /dev/gsmtty2,
  +   /dev/gsmtty3, /dev/gsmtty4,
  +   /dev/gsmtty5, /dev/gsmtty6 };
 
 I am not a big fan of these hardcoded names. I was not a big fan for
 the original MUX code and that has not changed.
 
 So what happens if you have to multiplexer running at the same time?
 

No idea. I'll try to analyze this point.

   static const char *none_prefix[] = { NULL };
   static const char *xdrv_prefix[] = { +XDRV:, NULL };
  @@ -392,6 +396,7 @@ static gboolean dlc_ready_check(gpointer
 user_data)
  struct ifx_data *data = ofono_modem_get_data(modem);
  struct stat st;
  int i;
  +   GHashTable *options;
 
  DBG();
 
  @@ -405,8 +410,14 @@ static gboolean dlc_ready_check(gpointer
 user_data)
  return TRUE;
  }
 
  +   options = g_hash_table_new(g_str_hash, g_str_equal);
  +   if (options == NULL)
  +   goto error;
  +
  +   g_hash_table_insert(options, Local, on);
  +
 
 Why is this needed?

This is needed for PDP context deactivation.
If you don't set the local to on a tty close occurs when you do the PDP context 
deactivation.

  +
  +   ofono_info(Got default configuration...);
  +   ofono_info(adaption = %d, cfg.adaption);
  +   ofono_info(encapsulation = %d, cfg.encapsulation);
  +   ofono_info(initiator = %d, cfg.initiator);
  +   ofono_info(t1 = %d, cfg.t1);
  +   ofono_info(t2 = %d, cfg.t2);
  +   ofono_info(t3 = %d, cfg.t3);
  +   ofono_info(n2 = %d, cfg.n2);
  +   ofono_info(mru = %d, cfg.mru);
  +   ofono_info(mtu = %d, cfg.mtu);
  +   ofono_info(k = %d, cfg.k);
 
 This pretty heavy debug information and not ofono_info material.
 

Agree. I kept them from the original patch of Robertino but I'll change that.

  +   cfg.encapsulation = 0; /* encoding - set to basic */
  +   cfg.initiator = 1; /* we are starting side */
  +   cfg.mru = 1500;
  +   cfg.mtu = 1500;
  +
  +   err = ioctl(fd, GSMIOC_SETCONF, cfg);
  +   if (err  0) {
  +   ofono_error(Failed to configure n_gsm multiplexer: %d,
 err);
  +   goto error;
  +   }
  +
  data-dlc_poll_count = 0;
  data-dlc_poll_source = g_timeout_add_seconds(1, dlc_ready_check,
  modem);
  @@ -511,6 +557,9 @@ static void mux_setup_cb(gboolean ok, GAtResult
 *result, gpointer user_data)
  return;
 
   error:
  +   if (ioctl(fd, TIOCSETD, data-saved_ldisc)  0)
  +   ofono_warn(Failed to restore line discipline);
  +
 
 This is not right. Even if you fail GSMIOC_GETCONF you try to restore
 the line discipline. That makes no sense.
 

Agree that is not right but not for the GSMIOC_GETCONF failing.
It's not right because I try to restore the line discipline even if the ioctl 
who gives it failed. I'll review that.

  data-saved_ldisc = -1;
 
  g_io_channel_unref(data-device);
  @@ -593,9 +642,9 @@ static int ifx_enable(struct ofono_modem *modem)
  NULL, NULL, NULL);
 
  /* Enable multiplexer */
  -   data-frame_size = 1509;
  +   data-frame_size = 1500;
 
  -   g_at_chat_send(chat, AT+CMUX=0,0,,1509,10,3,30,,, NULL,
  +   g_at_chat_send(chat, AT+CMUX=0,0,,1500,10,3,30,,, NULL,
  mux_setup_cb, modem, NULL);
 
 So why did the frame_size changed here? Does the modem firmware changed
 or is this actual a bug in oFono already today?
 

It's a bug in oFono.

  data-mux_init_timeout = g_timeout_add_seconds(5, mux_timeout_cb,
  @@ -732,22 +781,20 @@ static void 

[PATCH] simfs: fix GLib-CRITICAL issue occuring when there is no SIM card.

2011-03-09 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

During modem release, when there is no SIM card, a GLib-CRITICAL
issue occurs during the free of the sim fs context.
---
 src/simfs.c |   26 ++
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/simfs.c b/src/simfs.c
index 3a97a10..49f79dd 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -166,21 +166,23 @@ void sim_fs_context_free(struct ofono_sim_context 
*context)
int n = 0;
struct sim_fs_op *op;
 
-   while ((op = g_queue_peek_nth(fs-op_q, n)) != NULL) {
-   if (op-context != context) {
-   n += 1;
-   continue;
-   }
+   if (fs-op_q) {
+   while ((op = g_queue_peek_nth(fs-op_q, n)) != NULL) {
+   if (op-context != context) {
+   n += 1;
+   continue;
+   }
 
-   if (n == 0) {
-   op-cb = NULL;
+   if (n == 0) {
+   op-cb = NULL;
 
-   n += 1;
-   continue;
-   }
+   n += 1;
+   continue;
+   }
 
-   sim_fs_op_free(op);
-   g_queue_remove(fs-op_q, op);
+   sim_fs_op_free(op);
+   g_queue_remove(fs-op_q, op);
+   }
}
 
if (context-file_watches)
-- 
1.6.3.3

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH 0/3] integration of kernel mux (n_gsm) and its line discipline

2011-03-09 Thread Lucas, GuillaumeX
Hi,

Following set of patch adds support of the kernel mux (n_gsm) for the ifx modem.
They were tested with the latest version of the kernel mux who is currently 
under submission.

Following tests were performed (and pass):

- PDP context activation/deactivation back-to-back (x15)
- Enable/disable modem back-to-back (x10)
- PDP context activation with wrong APN
- Lost of network connection during PDP context activation
- Lost of network connection with a PDP context activated

Regards,
Guillaume

---
 plugins/ifx.c |   83 
 drivers/ifxmodem/gprs-context.c |   95 +--
 2 files changed, 169 insertions(+), 23 deletions(-)
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH 3/3] ifx: fix issue with closing of DLC_AUX tty channel

2011-03-09 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

Using kernel mux, a recursive locking is detected. This issue is
due to the fact that the DLC_AUX is not closed by ofono. This is
not done because the shutdown_device() function in charge of it
is called in the context of an AT callback function and for an
AT sent in this same dlc. Calling the real shutdown_device()
function using a timer solved the issue.
---
 plugins/ifx.c |   14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/plugins/ifx.c b/plugins/ifx.c
index 9c2a5c1..3581f80 100644
--- a/plugins/ifx.c
+++ b/plugins/ifx.c
@@ -93,6 +93,7 @@ struct ifx_data {
guint dlc_poll_count;
guint dlc_poll_source;
guint dlc_init_source;
+   guint shutdown_source;
guint mux_init_timeout;
guint frame_size;
int mux_ldisc;
@@ -190,8 +191,9 @@ static void xsim_notify(GAtResult *result, gpointer 
user_data)
}
 }
 
-static void shutdown_device(struct ifx_data *data)
+static gboolean shutdown_device_cb(gpointer user_data)
 {
+   struct ifx_data *data = user_data;
int i, fd;
 
DBG();
@@ -224,8 +226,18 @@ static void shutdown_device(struct ifx_data *data)
 done:
g_io_channel_unref(data-device);
data-device = NULL;
+   data-shutdown_source = 0;
+   return FALSE;
+}
+
+static void shutdown_device(struct ifx_data *data)
+{
+   DBG();
+   data-shutdown_source = g_timeout_add_seconds(0,
+   shutdown_device_cb, data);
 }
 
+
 static void dlc_disconnect(gpointer user_data)
 {
struct ofono_modem *modem = user_data;
-- 
1.6.3.3

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH 1/3] ifx: integration of kernel mux (n_gsm) and its line discipline

2011-03-09 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

---
 plugins/ifx.c |   83 
 1 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/plugins/ifx.c b/plugins/ifx.c
index 527a8c4..9c2a5c1 100644
--- a/plugins/ifx.c
+++ b/plugins/ifx.c
@@ -26,8 +26,12 @@
 #include stdio.h
 #include errno.h
 #include stdlib.h
+#include string.h
 #include sys/stat.h
 #include sys/ioctl.h
+#include linux/types.h
+#include linux/gsmmux.h
+#include unistd.h
 
 #include glib.h
 #include gatchat.h
@@ -75,9 +79,9 @@
 static char *dlc_prefixes[NUM_DLC] = { Voice: , Net: , GPRS1: ,
GPRS2: , GPRS3: , Aux:  };
 
-static const char *dlc_nodes[NUM_DLC] = { /dev/ttyGSM1, /dev/ttyGSM2,
-   /dev/ttyGSM3, /dev/ttyGSM4,
-   /dev/ttyGSM5, /dev/ttyGSM6 };
+static const char *dlc_nodes[NUM_DLC] = { /dev/gsmtty1, /dev/gsmtty2,
+   /dev/gsmtty3, /dev/gsmtty4,
+   /dev/gsmtty5, /dev/gsmtty6 };
 
 static const char *none_prefix[] = { NULL };
 static const char *xdrv_prefix[] = { +XDRV:, NULL };
@@ -392,6 +396,7 @@ static gboolean dlc_ready_check(gpointer user_data)
struct ifx_data *data = ofono_modem_get_data(modem);
struct stat st;
int i;
+   GHashTable *options;
 
DBG();
 
@@ -405,8 +410,14 @@ static gboolean dlc_ready_check(gpointer user_data)
return TRUE;
}
 
+   options = g_hash_table_new(g_str_hash, g_str_equal);
+   if (options == NULL)
+   goto error;
+
+   g_hash_table_insert(options, Local, on);
+
for (i = 0; i  NUM_DLC; i++) {
-   GIOChannel *channel = g_at_tty_open(dlc_nodes[i], NULL);
+   GIOChannel *channel = g_at_tty_open(dlc_nodes[i], options);
 
data-dlcs[i] = create_chat(channel, modem, dlc_prefixes[i]);
if (data-dlcs[i] == NULL) {
@@ -415,6 +426,8 @@ static gboolean dlc_ready_check(gpointer user_data)
}
}
 
+   g_hash_table_destroy(options);
+
data-dlc_poll_source = 0;
 
/* iterate through mainloop */
@@ -455,7 +468,7 @@ static void setup_internal_mux(struct ofono_modem *modem)
ofono_error(Failed to create channel);
goto error;
}
-}
+   }
 
/* wait for DLC creation to settle */
data-dlc_init_source = g_timeout_add(10, dlc_setup, modem);
@@ -471,7 +484,9 @@ static void mux_setup_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
 {
struct ofono_modem *modem = user_data;
struct ifx_data *data = ofono_modem_get_data(modem);
-   int fd;
+   int err;
+   struct gsm_config cfg;
+   int fd = -1;
 
DBG();
 
@@ -504,6 +519,37 @@ static void mux_setup_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
goto error;
}
 
+   memset(cfg, 0, sizeof(struct gsm_config));
+
+   err = ioctl(fd, GSMIOC_GETCONF, cfg);
+   if (err  0) {
+   ofono_error(Failed to get multiplexer configuration: %d, err);
+   goto error;
+   }
+
+   ofono_info(Got default configuration...);
+   ofono_info(adaption = %d, cfg.adaption);
+   ofono_info(encapsulation = %d, cfg.encapsulation);
+   ofono_info(initiator = %d, cfg.initiator);
+   ofono_info(t1 = %d, cfg.t1);
+   ofono_info(t2 = %d, cfg.t2);
+   ofono_info(t3 = %d, cfg.t3);
+   ofono_info(n2 = %d, cfg.n2);
+   ofono_info(mru = %d, cfg.mru);
+   ofono_info(mtu = %d, cfg.mtu);
+   ofono_info(k = %d, cfg.k);
+
+   cfg.encapsulation = 0; /* encoding - set to basic */
+   cfg.initiator = 1; /* we are starting side */
+   cfg.mru = 1500;
+   cfg.mtu = 1500;
+
+   err = ioctl(fd, GSMIOC_SETCONF, cfg);
+   if (err  0) {
+   ofono_error(Failed to configure n_gsm multiplexer: %d, err);
+   goto error;
+   }
+
data-dlc_poll_count = 0;
data-dlc_poll_source = g_timeout_add_seconds(1, dlc_ready_check,
modem);
@@ -511,6 +557,9 @@ static void mux_setup_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
return;
 
 error:
+   if (ioctl(fd, TIOCSETD, data-saved_ldisc)  0)
+   ofono_warn(Failed to restore line discipline);
+
data-saved_ldisc = -1;
 
g_io_channel_unref(data-device);
@@ -593,9 +642,9 @@ static int ifx_enable(struct ofono_modem *modem)
NULL, NULL, NULL);
 
/* Enable multiplexer */
-   data-frame_size = 1509;
+   data-frame_size = 1500;
 
-   g_at_chat_send(chat, AT+CMUX=0,0,,1509,10,3,30,,, NULL,
+   g_at_chat_send(chat, AT+CMUX=0,0,,1500,10,3,30,,, NULL,

[PATCH 2/3] ifxmodem: integration of kernel mux (n_gsm)

2011-03-09 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

---
 drivers/ifxmodem/gprs-context.c |   95 +--
 1 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/drivers/ifxmodem/gprs-context.c b/drivers/ifxmodem/gprs-context.c
index 2c68b44..0d133f9 100644
--- a/drivers/ifxmodem/gprs-context.c
+++ b/drivers/ifxmodem/gprs-context.c
@@ -29,6 +29,11 @@
 #include stdio.h
 #include errno.h
 #include sys/stat.h
+#include sys/ioctl.h
+#include net/if.h
+#include arpa/inet.h
+#include linux/if_ether.h
+#include linux/gsmmux.h
 
 #include glib.h
 
@@ -63,6 +68,7 @@ struct gprs_context_data {
char username[OFONO_GPRS_MAX_USERNAME_LENGTH + 1];
char password[OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
GAtRawIP *rawip;
+   gboolean use_ofono_mux;
enum state state;
char address[32];
char dns1[32];
@@ -79,7 +85,46 @@ static void rawip_debug(const char *str, void *data)
ofono_info(%s: %s, (const char *) data, str);
 }
 
-static const char *setup_rawip(struct ofono_gprs_context *gc)
+static const char *setup_rawip_kmux(struct ofono_gprs_context *gc)
+{
+   struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+   struct gsm_netconfig netconfig;
+   int fd;
+   int n;
+   char *interface = NULL;
+   char ifname[IFNAMSIZ];
+
+   memset(netconfig, 0, sizeof(struct gsm_netconfig));
+   gcd-rawip = NULL;
+
+   fd = g_io_channel_unix_get_fd(g_at_chat_get_channel(gcd-chat));
+
+   g_at_chat_suspend(gcd-chat);
+
+   netconfig.adaption = 3;
+   netconfig.protocol = htons(ETH_P_IP);
+
+   n = ioctl(fd, GSMIOC_ENABLE_NET, netconfig);
+   if (n  0) {
+   ofono_error(Cannot enable network on the mux: %d, n);
+   return NULL;
+   }
+
+   ofono_info(Net index: %d, n);
+
+   interface = if_indextoname(n, ifname);
+   if (interface == NULL) {
+   ofono_error(Interface index %d error %d error msg %s, n,
+   errno, strerror(errno));
+   return NULL;
+   }
+
+   ofono_info(Interface name: %s, interface);
+
+   return interface;
+}
+
+static const char *setup_rawip_internal(struct ofono_gprs_context *gc)
 {
struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
GAtIO *io;
@@ -153,7 +198,14 @@ static void session_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
dns[1] = gcd-dns2;
dns[2] = 0;
 
-   interface = setup_rawip(gc);
+   if (gcd-use_ofono_mux == FALSE) {
+   ofono_info(Calling setup_rawip_kmux());
+   interface = setup_rawip_kmux(gc);
+   } else {
+   ofono_info(Calling setup_rawip_internal());
+   interface = setup_rawip_internal(gc);
+   }
+
if (interface == NULL)
interface = invalid;
 
@@ -352,7 +404,9 @@ static void deactivate_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
 
DBG(ok %d, ok);
 
-   g_at_rawip_unref(gcd-rawip);
+   if (gcd-rawip)
+   g_at_rawip_unref(gcd-rawip);
+
gcd-rawip = NULL;
 
gcd-active_context = 0;
@@ -370,6 +424,8 @@ static void ifx_gprs_deactivate_primary(struct 
ofono_gprs_context *gc,
struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
GAtChat *chat = g_at_chat_get_slave(gcd-chat);
char buf[64];
+   int fd;
+   int n;
 
DBG(cid %u, cid);
 
@@ -377,7 +433,14 @@ static void ifx_gprs_deactivate_primary(struct 
ofono_gprs_context *gc,
gcd-down_cb = cb;
gcd-cb_data = data;
 
-   g_at_rawip_shutdown(gcd-rawip);
+   if (gcd-rawip)
+   g_at_rawip_shutdown(gcd-rawip);
+
+   fd = g_io_channel_unix_get_fd(g_at_chat_get_channel(gcd-chat));
+
+   n = ioctl(fd, GSMIOC_DISABLE_NET, NULL);
+   if (n  0)
+   ofono_error(Cannot disable network on the mux: %d, n);
 
sprintf(buf, AT+CGACT=0,%u, gcd-active_context);
if (g_at_chat_send(chat, buf, none_prefix,
@@ -394,6 +457,8 @@ static void cgev_notify(GAtResult *result, gpointer 
user_data)
const char *event;
int cid;
GAtResultIter iter;
+   int fd;
+   int n;
 
g_at_result_iter_init(iter, result);
 
@@ -424,6 +489,14 @@ static void cgev_notify(GAtResult *result, gpointer 
user_data)
gcd-rawip = NULL;
}
 
+   if (gcd-use_ofono_mux == FALSE) {
+   fd = g_io_channel_unix_get_fd(g_at_chat_get_channel(gcd-chat));
+
+   n = ioctl(fd, GSMIOC_DISABLE_NET, NULL);
+   if (n  0)
+   ofono_error(Cannot disable network on the mux: %d, n);
+   }
+
ofono_gprs_context_deactivated(gc, gcd-active_context);
 
gcd-active_context = 0;
@@ -436,7 +509,9 @@ static int ifx_gprs_context_probe(struct ofono_gprs_context 
*gc,
unsigned 

[PATCH] test-stk: update STK test script for conformance

2010-12-21 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

Update the STK test script to be able to use it for
conformance test. Using 'agent' as parameter the
script will now register a STK agent to be able to
handle unsolicited proactive command. Without any
parameter or with 'menu' the script will try to
display the STK main menu of the (U)SIM.
---
 test/test-stk-menu |  101 +++-
 1 files changed, 76 insertions(+), 25 deletions(-)

diff --git a/test/test-stk-menu b/test/test-stk-menu
index 94d2d6b..b396452 100755
--- a/test/test-stk-menu
+++ b/test/test-stk-menu
@@ -13,6 +13,9 @@ class GoBack(dbus.DBusException):
 class EndSession(dbus.DBusException):
_dbus_error_name = org.ofono.Error.EndSession
 
+class Busy(dbus.DBusException):
+   _dbus_error_name = org.ofono.Error.Busy
+
 class StkAgent(dbus.service.Object):
exit_on_release = True
 
@@ -30,9 +33,10 @@ class StkAgent(dbus.service.Object):
in_signature=sya(sy)n, out_signature=y)
def RequestSelection(self, title, icon, items, default):
print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
index = 0;
for item in items:
-   print %d. %s % (index, item[0])
+   print %d. %s (icon: %d) % (index, item[0], item[1])
index += 1
 
print \nDefault: %d % (default)
@@ -48,14 +52,27 @@ class StkAgent(dbus.service.Object):
@dbus.service.method(org.ofono.SimToolkitAgent,
in_signature=syb, out_signature=)
def DisplayText(self, title, icon, urgent):
-   print DisplayText (%s, %s) % (title, urgent)
+   print DisplayText (%s) % (title)
+   print Icon: (%d) % (icon)
+   print Urgent: (%d) % (urgent)
+   key = raw_input(Press return to clear ('t' terminates, 
+   'b' goest 
back, 'n' busy):)
+
+   if key == 'b':
+   raise GoBack(User wishes to go back)
+   elif key == 't':
+   raise EndSession(User wishes to terminate session)
+   elif key == 'n':
+   raise Busy(User wishes to simulate busy screen)
 
@dbus.service.method(org.ofono.SimToolkitAgent,
in_signature=sysyyb, out_signature=s)
def RequestInput(self, title, icon, default, min_chars, max_chars,
hide_typing):
print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
print Default: (%s) % (default)
+   print Hide typing: (%s) % (hide_typing)
print Enter characters, min: %d, max: %d: % (min_chars,
max_chars)
userin = raw_input();
@@ -67,7 +84,9 @@ class StkAgent(dbus.service.Object):
def RequestDigits(self, title, icon, default, min_chars, max_chars,
hide_typing):
print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
print Default: (%s) % (default)
+   print Hide typing: (%s) % (hide_typing)
print Enter digits, min: %d, max: %d: % (min_chars,
max_chars)
userin = raw_input('t' terminates, 'b' goes back:);
@@ -83,6 +102,7 @@ class StkAgent(dbus.service.Object):
in_signature=sy, out_signature=s)
def RequestKey(self, title, icon):
print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
key = raw_input(Enter Key (t, b):)
 
if key == 'b':
@@ -96,6 +116,7 @@ class StkAgent(dbus.service.Object):
in_signature=sy, out_signature=s)
def RequestDigit(self, title, icon):
print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
key = raw_input(Enter Digit (t, b):)
 
if key == 'b':
@@ -109,6 +130,7 @@ class StkAgent(dbus.service.Object):
in_signature=sy, out_signature=b)
def RequestConfirmation(self, title, icon):
print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
key = raw_input(Enter Confirmation (t, b, y, n):)
 
if key == 'b':
@@ -124,6 +146,7 @@ class StkAgent(dbus.service.Object):
in_signature=sy, out_signature=b)
def ConfirmCallSetup(self, info, icon):
print Information: (%s) % (info)
+   print Icon: (%d) % (icon)
key = raw_input(Enter Confirmation (t, y, n):)
 
if key == 't':
@@ -141,14 +164,30 @@ 

RE: [PATCH] test: add a script for STK conformance tests

2010-12-20 Thread Lucas, GuillaumeX
Hi Denis,

 -Original Message-
 From: Denis Kenzior [mailto:denk...@gmail.com]
 Sent: Friday, December 17, 2010 4:32 PM
 To: ofono@ofono.org
 Cc: Lucas, GuillaumeX
 Subject: Re: [PATCH] test: add a script for STK conformance tests
 
 Hi Guillaume,
 
 On 12/16/2010 10:04 AM, Lucas, GuillaumeX wrote:
  From: Guillaume Lucas guillaumex.lu...@intel.com
 
  To be able to run STK conformance tests at ofono
  level, we need to have a STK agent registered. The
  test-stk-conformance script provided here do that.
  This script is for conformance so all the data input
  of the agent are displayed to be able to check them.
  The existing test-stk-menu is usable only when there
  is a STK menu provided by the (U)SIM card.
  ---
   test/test-stk-conformance |  226
 +
   1 files changed, 226 insertions(+), 0 deletions(-)
   create mode 100755 test/test-stk-conformance
 
 
 I really don't like the amount of code copying going on here.  Have you
 considered extending the current test-stk-menu with the ability to run
 as the 'default' agent instead?  Maybe via a command line option?
 

You're right, it will be probably better to extend the current stk test script.
I'll review my changes in this way.

Regards,
Guillaume
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH] test: add a script for STK conformance tests

2010-12-16 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

To be able to run STK conformance tests at ofono
level, we need to have a STK agent registered. The
test-stk-conformance script provided here do that.
This script is for conformance so all the data input
of the agent are displayed to be able to check them.
The existing test-stk-menu is usable only when there
is a STK menu provided by the (U)SIM card.
---
 test/test-stk-conformance |  226 +
 1 files changed, 226 insertions(+), 0 deletions(-)
 create mode 100755 test/test-stk-conformance

diff --git a/test/test-stk-conformance b/test/test-stk-conformance
new file mode 100755
index 000..bb28e40
--- /dev/null
+++ b/test/test-stk-conformance
@@ -0,0 +1,226 @@
+#!/usr/bin/python
+
+import gobject
+
+import sys
+import dbus
+import dbus.service
+import dbus.mainloop.glib
+import os
+
+class GoBack(dbus.DBusException):
+   _dbus_error_name = org.ofono.Error.GoBack
+
+class EndSession(dbus.DBusException):
+   _dbus_error_name = org.ofono.Error.EndSession
+
+class Busy(dbus.DBusException):
+   _dbus_error_name = org.ofono.Error.Busy
+
+class StkAgent(dbus.service.Object):
+   exit_on_release = True
+
+   def set_exit_on_release(self, exit_on_release):
+   self.exit_on_release = exit_on_release
+
+   @dbus.service.method(org.ofono.SimToolkitAgent,
+   in_signature=, out_signature=)
+   def Release(self):
+   print Release
+   if self.exit_on_release:
+   mainloop.quit()
+
+   @dbus.service.method(org.ofono.SimToolkitAgent,
+   in_signature=sya(sy)n, out_signature=y)
+   def RequestSelection(self, title, icon, items, default):
+   print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
+   index = 0;
+   for item in items:
+   print %d. %s (icon: %d) % (index, item[0], item[1])
+   index += 1
+
+   print \nDefault: %d % (default)
+   select = raw_input(Enter Selection (t, b):)
+
+   if select == 'b':
+   raise GoBack(User wishes to go back)
+   elif select == 't':
+   raise EndSession(User wishes to terminate session)
+   else:
+   return int(select);
+
+   @dbus.service.method(org.ofono.SimToolkitAgent,
+   in_signature=syb, out_signature=)
+   def DisplayText(self, title, icon, urgent):
+   print DisplayText (%s) % (title)
+   print Icon: (%d) % (icon)
+   print Urgent: (%d) % (urgent)
+   userin = raw_input(Press return to clear ('t' terminates, 
+   'b' goest 
back, 'n' busy):)
+
+   if userin == 'b':
+   raise GoBack(User wishes to go back)
+   elif userin == 't':
+   raise EndSession(User wishes to terminate session)
+   elif userin == 'n':
+   raise Busy(User wishes to simulate busy screen)
+
+   @dbus.service.method(org.ofono.SimToolkitAgent,
+   in_signature=sysyyb, out_signature=s)
+   def RequestInput(self, title, icon, default, min_chars, max_chars,
+   hide_typing):
+   print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
+   print Default: (%s) % (default)
+   print Hide typing: (%s) % (hide_typing)
+   print Enter characters, min: %d, max: %d: % (min_chars,
+   max_chars)
+   userin = raw_input()
+
+   return userin
+
+   @dbus.service.method(org.ofono.SimToolkitAgent,
+   in_signature=sysyyb, out_signature=s)
+   def RequestDigits(self, title, icon, default, min_chars, max_chars,
+   hide_typing):
+   print Title: (%s) % (title)
+   print Icon: (%d) % (icon)
+   print Default: (%s) % (default)
+   print Hide typing: (%s) % (hide_typing)
+   print Enter digits, min: %d, max: %d: % (min_chars,
+   max_chars)
+   userin = raw_input('t' terminates, 'b' goes back:);
+
+   if userin == 'b':
+   raise GoBack(User wishes to go back)
+   elif userin == 't':
+   raise EndSession(User wishes to terminate session)
+   else:
+   return userin
+
+   @dbus.service.method(org.ofono.SimToolkitAgent,
+   in_signature=sy, out_signature=s)
+   def RequestKey(self, title, icon):
+  

RE: [RFC PATCH] stk: add support of next action for menu

2010-12-13 Thread Lucas, GuillaumeX
Hi Denis,

 -Original Message-
 From: Denis Kenzior [mailto:denk...@gmail.com]
 Sent: Friday, December 10, 2010 6:00 PM
 To: ofono@ofono.org
 Cc: Lucas, GuillaumeX
 Subject: Re: [RFC PATCH] stk: add support of next action for menu
 
 Hi Guillaume,
 
 On 12/09/2010 07:00 AM, Lucas, GuillaumeX wrote:
  From: Guillaume Lucas guillaumex.lu...@intel.com
 
  According to the 3GPP 31.124 the support of next action
  in SIM toolkit menus is mandatory.
  ---
   doc/stk-api.txt|5 +++--
   src/stk.c  |6 ++
   src/stkagent.c |6 --
   src/stkagent.h |1 +
   test/test-stk-menu |6 +++---
   5 files changed, 17 insertions(+), 7 deletions(-)
 
  diff --git a/doc/stk-api.txt b/doc/stk-api.txt
  index 79daee6..f5bcae4 100644
  --- a/doc/stk-api.txt
  +++ b/doc/stk-api.txt
  @@ -66,13 +66,14 @@ Properties  string IdleModeText [readonly]
  Contains the identifier of the icon accompanying
  the idle mode text.
 
  -   array{struct{string, byte}} MainMenu [readonly]
  +   array{struct{string, byte, byte}} MainMenu [readonly]
 
 Can you tell me exactly what this can be used for?  What is the usecase
 for the application to know this information?
 

The next action is an indicator added for each item of a STK menu. It is to 
allow the terminal to indicate to the user the consequences of performing the 
selection of an item. I confess that I've never seen use of this option in a 
real STK application yet.

According to the applicability table (chapter 3.4) of the SATK conformance 
specification (3GPP TS 31.124) the support of the next action indicator is 
mandatory. Chapters 27.22.4.8.3 and 27.22.4.9.2 are the ones relative to the 
test of this feature.

Regards,
Guillaume
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH] stk: fix issue to avoid null pointer for alpha_id

2010-12-08 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

If a SET UP CALL proactive command with no alpha identifier
occurs, the alpha_id pointer will be set to NULL. This will
generate a crash in the stkagent with dbus function. To avoid
this we use an empty string if the alpha identifier is not
present in the command.
---
 src/stk.c |   13 ++---
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 7b39f7e..8c7cb8a 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1755,13 +1755,12 @@ static gboolean handle_command_set_up_call(const struct 
stk_command *cmd,
return TRUE;
}
 
-   if (sc-alpha_id_usr_cfm) {
-   alpha_id = dbus_apply_text_attributes(sc-alpha_id_usr_cfm,
-   sc-text_attr_usr_cfm);
-   if (alpha_id == NULL) {
-   rsp-result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
-   return TRUE;
-   }
+   alpha_id = dbus_apply_text_attributes(sc-alpha_id_usr_cfm ?
+   sc-alpha_id_usr_cfm : ,
+   sc-text_attr_usr_cfm);
+   if (alpha_id == NULL) {
+   rsp-result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+   return TRUE;
}
 
err = stk_agent_confirm_call(stk-current_agent, alpha_id,
-- 
1.7.0.4

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH] stkutil: fix crash issue cause by null length of text string

2010-12-07 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

According to 3GPP TS 31.124 a null length for the text string
should be allowed. An empty string must be returned to the
user in this case.
---
 src/stkutil.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index a211462..cab22f4 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -562,7 +562,7 @@ static gboolean parse_dataobj_text(struct 
comprehension_tlv_iter *iter,
 {
char **text = user;
unsigned int len = comprehension_tlv_iter_get_length(iter);
-   const unsigned char *data = comprehension_tlv_iter_get_data(iter);
+   const unsigned char *data;
char *utf8;
 
/* DCS followed by some text, cannot be 1 */
@@ -570,10 +570,12 @@ static gboolean parse_dataobj_text(struct 
comprehension_tlv_iter *iter,
return FALSE;
 
if (len == 0) {
-   *text = NULL;
+   *text = g_try_malloc0(1);
return TRUE;
}
 
+   data = comprehension_tlv_iter_get_data(iter);
+
utf8 = decode_text(data[0], len - 1, data + 1);
 
if (utf8 == NULL)
-- 
1.7.0.4

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH] stk: Add busy error for the display text command

2010-11-30 Thread Lucas, GuillaumeX
Hi Andrzej

 
 Hi Guillaume,
 
 On 25 November 2010 09:44, Lucas, GuillaumeX
 guillaumex.lu...@intel.com wrote:
  From: Guillaume Lucas guillaumex.lu...@intel.com
 
  According to the sequence 1.2 of the ETSI TS 102 384 a busy
  screen error type must be returned for the display text
  proactive command when the message is not urgent and if the
  ME is not able to display the text.
  ---
   doc/stk-api.txt |    1 +
   src/stk.c       |    7 +++
   src/stkagent.c  |   12 ++--
   src/stkagent.h  |    1 +
   4 files changed, 19 insertions(+), 2 deletions(-)
 
  diff --git a/doc/stk-api.txt b/doc/stk-api.txt
  index b9ca592..79daee6 100644
  --- a/doc/stk-api.txt
  +++ b/doc/stk-api.txt
  @@ -116,6 +116,7 @@ Methods             byte RequestSelection(string
 title, byte icon_id,
                         cleared prior to the display of this text.
 
                         Possible Errors:
 [service].Error.SimToolkit.GoBack
  +
  [service].Error.SimToolkit.Busy
 
                         Implementation notes:
 
  diff --git a/src/stk.c b/src/stk.c
  index ac2e646..e6b78a4 100644
  --- a/src/stk.c
  +++ b/src/stk.c
  @@ -1250,6 +1250,10 @@ static void display_text_cb(enum
 stk_agent_result result, void *user_data)
                         STK_RESULT_TYPE_NO_RESPONSE :
 STK_RESULT_TYPE_SUCCESS);
                 break;
 
  +       case STK_AGENT_RESULT_BUSY:
  +               send_simple_response(stk,
 STK_RESULT_TYPE_TERMINAL_BUSY);
  +               break;
  +
 
 According to TS 102.223 on Screen Busy we also need to append a
 additional info field in the Terminal Busy response with a 1-byte
 value 01 (screen is busy).

Yes you're right. I've seen this but after sending my patch.
I've another patch in my pipe regarding this but in top of the current busy 
patch.
I'm waiting the integration of the busy patch to submit it.

 
 Denis on IRC also wondered if this response should be allowed when the
 message is flagged urgent.
 

According to the spec I think no, this type of response is not allowed when the 
message is fluffed urgent.

         case STK_AGENT_RESULT_TERMINATE:
         default:
                 send_simple_response(stk,
 STK_RESULT_TYPE_USER_TERMINATED);
  @@ -1363,6 +1367,7 @@ static void request_confirmation_cb(enum
 stk_agent_result result,
                 break;
 
         case STK_AGENT_RESULT_TERMINATE:
  +       default:
                 send_simple_response(stk,
 STK_RESULT_TYPE_USER_TERMINATED);
                 break;
 
 Most of the other commands also allow a Terminal busy response in
 table 6.1 in section 6.11, without giving a specific additional info
 value.  I think it would be ok to treat them the same way as
 DisplayText and supply the screen is busy value, what do you think?
 

You're right for the other commands but those cases are already handled by 
ofono.
In fact there is only for the display text proactive command that we can have 
this error for a busy screen reason.

Regards,
Guillaume
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH] stk: Add busy error for the display text command

2010-11-30 Thread Lucas, GuillaumeX
Hi Denis,

 -Original Message-
 From: Denis Kenzior [mailto:denk...@gmail.com]
 Sent: Tuesday, November 30, 2010 1:25 PM
 To: ofono@ofono.org
 Cc: Lucas, GuillaumeX
 Subject: Re: [PATCH] stk: Add busy error for the display text command
 
 Hi Guillaume,
 
  According to TS 102.223 on Screen Busy we also need to append a
  additional info field in the Terminal Busy response with a 1-byte
  value 01 (screen is busy).
 
  Yes you're right. I've seen this but after sending my patch.
  I've another patch in my pipe regarding this but in top of the
 current busy patch.
  I'm waiting the integration of the busy patch to submit it.
 
 This is not how it works ;)  Please resubmit this patch with the code
 to
 send 'Screen is Busy' additional info included.  You can even submit
 your other pending patches in a series along with this one...
 
 So e.g.
 
 patch 1/n - reworked add busy error...
 patch 2/n - pending patch 1,
 etc
 

Ok, I'll resubmmit my patch with all the necessary changes.

Regards,
Guillaume
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH] stk: Add busy error for the display text command

2010-11-30 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

According to the sequence 1.2 of the ETSI TS 102 384 a busy
screen error should be returns for the display text proactive
command when the ME is not able to display the text.
---
 doc/stk-api.txt |1 +
 src/stk.c   |   15 +++
 src/stkagent.c  |   12 ++--
 src/stkagent.h  |1 +
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/doc/stk-api.txt b/doc/stk-api.txt
index b9ca592..79daee6 100644
--- a/doc/stk-api.txt
+++ b/doc/stk-api.txt
@@ -116,6 +116,7 @@ Methods byte RequestSelection(string title, 
byte icon_id,
cleared prior to the display of this text.
 
Possible Errors: [service].Error.SimToolkit.GoBack
+[service].Error.SimToolkit.Busy
 
Implementation notes:
 
diff --git a/src/stk.c b/src/stk.c
index a4abb7d..7b39f7e 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1203,6 +1203,9 @@ static void display_text_cb(enum stk_agent_result result, 
void *user_data)
 {
struct ofono_stk *stk = user_data;
gboolean confirm;
+   struct stk_response rsp;
+   static unsigned char screen_busy_result[] = { 0x01 };
+   static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
 
stk-respond_on_exit = FALSE;
 
@@ -1250,6 +1253,15 @@ static void display_text_cb(enum stk_agent_result 
result, void *user_data)
STK_RESULT_TYPE_NO_RESPONSE : STK_RESULT_TYPE_SUCCESS);
break;
 
+   case STK_AGENT_RESULT_BUSY:
+   memset(rsp, 0, sizeof(rsp));
+   rsp.result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+   rsp.result.additional_len = sizeof(screen_busy_result);
+   rsp.result.additional = screen_busy_result;
+   if (stk_respond(stk, rsp, stk_command_cb))
+   stk_command_cb(error, stk);
+   break;
+
case STK_AGENT_RESULT_TERMINATE:
default:
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
@@ -1366,6 +1378,7 @@ static void request_confirmation_cb(enum stk_agent_result 
result,
break;
 
case STK_AGENT_RESULT_TERMINATE:
+   default:
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
break;
}
@@ -1408,6 +1421,7 @@ static void request_key_cb(enum stk_agent_result result, 
char *string,
break;
 
case STK_AGENT_RESULT_TERMINATE:
+   default:
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
break;
}
@@ -1505,6 +1519,7 @@ static void request_string_cb(enum stk_agent_result 
result, char *string,
break;
 
case STK_AGENT_RESULT_TERMINATE:
+   default:
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
break;
}
diff --git a/src/stkagent.c b/src/stkagent.c
index 8315040..874b6dd 100644
--- a/src/stkagent.c
+++ b/src/stkagent.c
@@ -41,6 +41,7 @@
 enum allowed_error {
ALLOWED_ERROR_GO_BACK   = 0x1,
ALLOWED_ERROR_TERMINATE = 0x2,
+   ALLOWED_ERROR_BUSY  = 0x4,
 };
 
 struct stk_agent {
@@ -62,6 +63,7 @@ struct stk_agent {
 #define ERROR_PREFIX OFONO_SERVICE .Error
 #define GOBACK_ERROR ERROR_PREFIX .GoBack
 #define TERMINATE_ERROR ERROR_PREFIX .EndSession
+#define BUSY_ERROR ERROR_PREFIX .Busy
 
 static void stk_agent_send_noreply(struct stk_agent *agent, const char *method)
 {
@@ -194,6 +196,12 @@ static int check_error(struct stk_agent *agent, 
DBusMessage *reply,
goto out;
}
 
+   if ((allowed_errors  ALLOWED_ERROR_BUSY) 
+   g_str_equal(err.name, BUSY_ERROR)) {
+   *out_result = STK_AGENT_RESULT_BUSY;
+   goto out;
+   }
+
result = -EINVAL;
 
 out:
@@ -376,8 +384,8 @@ static void display_text_cb(DBusPendingCall *call, void 
*data)
gboolean remove_agent;
 
if (check_error(agent, reply,
-   ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE,
-   result) == -EINVAL) {
+   ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE | 
+   ALLOWED_ERROR_BUSY, result) == -EINVAL) {
remove_agent = TRUE;
goto error;
}
diff --git a/src/stkagent.h b/src/stkagent.h
index c8e1886..517bcfe 100644
--- a/src/stkagent.h
+++ b/src/stkagent.h
@@ -26,6 +26,7 @@ enum stk_agent_result {
STK_AGENT_RESULT_BACK,
STK_AGENT_RESULT_TERMINATE,
STK_AGENT_RESULT_TIMEOUT,
+   STK_AGENT_RESULT_BUSY,
 };
 
 struct stk_menu_item {
-- 
1.7.0.4

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France

[PATCH] stk: Add busy error for the display text command

2010-11-25 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

According to the sequence 1.2 of the ETSI TS 102 384 a busy
screen error type must be returned for the display text 
proactive command when the message is not urgent and if the
ME is not able to display the text.
---
 doc/stk-api.txt |1 +
 src/stk.c   |7 +++
 src/stkagent.c  |   12 ++--
 src/stkagent.h  |1 +
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/doc/stk-api.txt b/doc/stk-api.txt
index b9ca592..79daee6 100644
--- a/doc/stk-api.txt
+++ b/doc/stk-api.txt
@@ -116,6 +116,7 @@ Methods byte RequestSelection(string title, 
byte icon_id,
cleared prior to the display of this text.
 
Possible Errors: [service].Error.SimToolkit.GoBack
+[service].Error.SimToolkit.Busy
 
Implementation notes:
 
diff --git a/src/stk.c b/src/stk.c
index ac2e646..e6b78a4 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1250,6 +1250,10 @@ static void display_text_cb(enum stk_agent_result 
result, void *user_data)
STK_RESULT_TYPE_NO_RESPONSE : STK_RESULT_TYPE_SUCCESS);
break;
 
+   case STK_AGENT_RESULT_BUSY:
+   send_simple_response(stk, STK_RESULT_TYPE_TERMINAL_BUSY);
+   break;
+
case STK_AGENT_RESULT_TERMINATE:
default:
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
@@ -1363,6 +1367,7 @@ static void request_confirmation_cb(enum stk_agent_result 
result,
break;
 
case STK_AGENT_RESULT_TERMINATE:
+   default:
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
break;
}
@@ -1405,6 +1410,7 @@ static void request_key_cb(enum stk_agent_result result, 
char *string,
break;
 
case STK_AGENT_RESULT_TERMINATE:
+   default:
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
break;
}
@@ -1502,6 +1508,7 @@ static void request_string_cb(enum stk_agent_result 
result, char *string,
break;
 
case STK_AGENT_RESULT_TERMINATE:
+   default:
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
break;
}
diff --git a/src/stkagent.c b/src/stkagent.c
index 5cf83e4..6e8aff6 100644
--- a/src/stkagent.c
+++ b/src/stkagent.c
@@ -41,6 +41,7 @@
 enum allowed_error {
ALLOWED_ERROR_GO_BACK   = 0x1,
ALLOWED_ERROR_TERMINATE = 0x2,
+   ALLOWED_ERROR_BUSY  = 0x4,
 };
 
 struct stk_agent {
@@ -62,6 +63,7 @@ struct stk_agent {
 #define ERROR_PREFIX OFONO_SERVICE .Error
 #define GOBACK_ERROR ERROR_PREFIX .GoBack
 #define TERMINATE_ERROR ERROR_PREFIX .EndSession
+#define BUSY_ERROR ERROR_PREFIX .Busy
 
 static void stk_agent_send_noreply(struct stk_agent *agent, const char *method)
 {
@@ -194,6 +196,12 @@ static int check_error(struct stk_agent *agent, 
DBusMessage *reply,
goto out;
}
 
+   if ((allowed_errors  ALLOWED_ERROR_BUSY) 
+   g_str_equal(err.name, BUSY_ERROR)) {
+   *out_result = STK_AGENT_RESULT_BUSY;
+   goto out;
+   }
+
result = -EINVAL;
 
 out:
@@ -376,8 +384,8 @@ static void display_text_cb(DBusPendingCall *call, void 
*data)
gboolean remove_agent;
 
if (check_error(agent, reply,
-   ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE,
-   result) == -EINVAL) {
+   ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE | 
+   ALLOWED_ERROR_BUSY, result) == -EINVAL) {
remove_agent = TRUE;
goto error;
}
diff --git a/src/stkagent.h b/src/stkagent.h
index c8e1886..517bcfe 100644
--- a/src/stkagent.h
+++ b/src/stkagent.h
@@ -26,6 +26,7 @@ enum stk_agent_result {
STK_AGENT_RESULT_BACK,
STK_AGENT_RESULT_TERMINATE,
STK_AGENT_RESULT_TIMEOUT,
+   STK_AGENT_RESULT_BUSY,
 };
 
 struct stk_menu_item {
-- 
1.7.0.4
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH] stkutil: Use sms_dcs_decode to retrieve the STK character set.

2010-11-25 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

---
 src/stkutil.c |   12 
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 377ceff..78efbf0 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -77,9 +77,13 @@ struct gsm_sms_tpdu {
 static char *decode_text(unsigned char dcs, int len, const unsigned char *data)
 {
char *utf8;
+   enum sms_charset charset;
 
-   switch (dcs) {
-   case 0x00:
+   if (!sms_dcs_decode(dcs, NULL, charset, NULL, NULL))
+   return NULL;
+
+   switch (charset) {
+   case SMS_CHARSET_7BIT:
{
long written;
unsigned long max_to_unpack = len * 8 / 7;
@@ -94,10 +98,10 @@ static char *decode_text(unsigned char dcs, int len, const 
unsigned char *data)
g_free(unpacked);
break;
}
-   case 0x04:
+   case SMS_CHARSET_8BIT:
utf8 = convert_gsm_to_utf8(data, len, NULL, NULL, 0);
break;
-   case 0x08:
+   case SMS_CHARSET_UCS2:
utf8 = g_convert((const gchar *) data, len,
UTF-8//TRANSLIT, UCS-2BE,
NULL, NULL, NULL);
-- 
1.7.0.4

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: STK DisplayText changes proposal

2010-11-24 Thread Lucas, GuillaumeX
Hi Andrew,

 
 On 23 November 2010 18:08, Denis Kenzior denk...@gmail.com wrote:
  On 11/23/2010 10:55 AM, Andrzej Zaborowski wrote:
  As for the clear after delay flag, there's a check for it in
  display_text_cb (line 1248).  As far as I can make out the only
  difference is the response sent to the SIM.
 
  So it has been a long time since those discussions, but it does seem
 to
  me that using a higher timeout in the case of 'wait_for_user' flag
 seems
  like a good idea.  Do you remember why we left this out?
 
 No, I don't, I'll send a patch to use the normal timeout instead of
 short timeout when clearing after user action is requested.  As for
 the proposal to add a parameter clear_after_delay and the ScreenBusy
 response I'm fine with both of them (as I don't think the saving from
 hiding these details is worth it) but I leave it up to you and
 Guillaume.  It seems that a user action always clears the message but
 it may be good to display a confirm button anyway.  The ScreenBusy
 response is also possible for other proactive commands so for
 consistency would need to be added there too.
 

For the 'clear_after_delay' flag your patch to used the normal timeout and not 
the short when a user request is waiting will be probably sufficient for the 
moment. For the ScreenBusy response I'm agree with you that this response is 
also possible for other proactive commands. I'll do the necessary patch for 
this.

Regards,
Guillaume
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


STK DisplayText changes proposal

2010-11-23 Thread Lucas, GuillaumeX
Hi,

According to the 3GPP specifications there is some missing points in the SATK 
DisplayText implementation and I want to propose some API changes here.

1. clear message after delay / wait for user to clear flag

This flag corresponds to the bit 8 of the command qualifier (see ETSI TS 102 
223) and indicates how the message should be cleared by the UI.
In the STK agent documentation it's indicated that this flag is handled using 
different timeout value for the Agent DisplayText method call. But that doesn't 
seem the case in the code: there is no check of the flag.

For me it's to the UI to handle the way how the message must be cleared and not 
to ofono. With the current Agent DisplayText method API this is not possible as 
there is no indication if the message must be cleared after a delay or after a 
user action. So I want to propose the following change for the STK Agent:

Changevoid DisplayText(string text, byte icon_id, boolean urgent)
Byvoid DisplayText(string text, byte icon_id, boolean urgent, boolean 
clear_after_delay)

Where clear_after_delay boolean must be set to '1' for the clear message after 
delay case and to '0' for the wait for user to clear case.

2. Busy screen

The other missing point in the DisplayText implementation is the case of the 
busy screen.
According to the sequence 1.2 of the ETSI TS 102 384 we must return a terminal 
response with the error screen busy when the ME is not in the idle screen and 
if the text message is not urgent. In the current implementation there is no 
way to return this type of error in the terminal response.

I propose to add a new error code ScreenBusy in the same way of what it's done 
for GoBack and EndSession event.

Please let me know if you agree with it.

Regards,
Guillaume
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH] stkutil: mask the DCS value to keep only the intersting bits

2010-11-23 Thread Lucas, GuillaumeX
Hi Denis,

 
 Hi Guillaume,
 
  - switch (dcs) {
  + switch (dcs  0x06) {
 
  Do you mean to bitwise and with 0xc here?  Otherwise you break the
 UCS2
  case.  Also do any SIMs use the '' Coding Bits Entry entry from
  23.038 Section 4?  If so, then we need to use sms_dcs_decode to
  retrieve
  the character set.
 
  Yes you are right. I did a bit-shifting error in my mask and don't
 see it during my test.
  It's 0xc and not 0x6 to use for the mask.
 
  The issue that I've is effectively due to the fact that some SIMs use
 the '' Coding Bits Entry (it's the case for French Orange SIM card
 for example). For me having the upper bits set to '1' do not really
 changes the character set decoding (only diff is that UCS2 will not be
 possible in that case). It's the reason why I've simply add a mask in
 the decode_text function. Using the sms_dcs_decode function is probably
 better as the DCS of the SIM ToolKit is the same as the one used for
 SMS.
 
  Do you want that I re-publish a patch using the sms_dcs_decode
 function?
 
 Bitwise shifting with 0x0c would work.  However, to be safe we might
 just be extra pedantic and check that the DCS the SIM sends to us is
 actually valid.
 

I agree.
I will rewrite my patch using the sms_dcs_decode function to secure this.

Regards,
Guillaume

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH] sim: use mask for check of sim file status

2010-11-05 Thread Lucas, GuillaumeX
From: Guillaume Lucas guillaumex.lu...@intel.com

Is' the first bit of the sim file status who indicates if a file
is valid or not. So a mask must be used to check this.
---
 src/sim.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index 02ab329..2ee2b29 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -1290,7 +1290,7 @@ static void sim_efbdn_info_read_cb(int ok, unsigned char 
file_status,
if (!ok)
goto out;
 
-   if (file_status == SIM_FILE_STATUS_VALID)
+   if (file_status  SIM_FILE_STATUS_VALID)
sim_bdn_enabled(sim);
 
 out:
@@ -1327,7 +1327,7 @@ static void sim_efadn_info_read_cb(int ok, unsigned char 
file_status,
if (!ok)
goto out;
 
-   if (file_status != SIM_FILE_STATUS_VALID)
+   if (!(file_status  SIM_FILE_STATUS_VALID))
sim_fdn_enabled(sim);
 
 out:
-- 
1.7.0.4
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH] sim: use mask for check of sim file status

2010-11-05 Thread Lucas, GuillaumeX
Hi Denis,

I've tested your changes and all is ok :)

Regards,
Guillaume

-Original Message-
From: Denis Kenzior [mailto:denk...@gmail.com] 
Sent: Friday, November 05, 2010 3:17 PM
To: ofono@ofono.org
Cc: Lucas, GuillaumeX
Subject: Re: [PATCH] sim: use mask for check of sim file status

Hi Guillaume,

On 11/05/2010 08:05 AM, Lucas, GuillaumeX wrote:
 From: Guillaume Lucas guillaumex.lu...@intel.com
 
 Is' the first bit of the sim file status who indicates if a file
 is valid or not. So a mask must be used to check this.
 ---
  src/sim.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

Good catch on these.  Patch has been applied, thanks.  I also applied a
couple of extra fixes that add more sanity checking to BDN  FDN.
Please check that my fixes didn't break anything else :)

Regards,
-Denis
-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono