RE: [PATCH 1/3] ifx: integration of kernel mux (n_gsm) and its line discipline
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
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
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.
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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