Re: [PATCH] atmodem: Do some polling on at_pin_query

2012-04-17 Thread Guillaume Zajac

Hi Denis,

On 16/04/2012 16:15, Denis Kenzior wrote:

Hi Guillaume,

On 04/16/2012 03:06 AM, Guillaume Zajac wrote:

Hi Denis,

On 12/04/2012 21:36, Denis Kenzior wrote:

Hi Guillaume,

On 04/12/2012 09:28 AM, Guillaume Zajac wrote:

For some modem like ZTE MF180/190, we need to do some
polling to check SIM state when it returns +CME ERROR: 14 busy.
---
   drivers/atmodem/sim.c |   59
+++-
   1 files changed, 48 insertions(+), 11 deletions(-)


snip


@@ -874,9 +878,22 @@ static void at_cpin_cb(gboolean ok, GAtResult
*result, gpointer user_data)
   else
   decode_at_error(error, final);

-if (!ok) {
+switch (error.type) {
+case OFONO_ERROR_TYPE_NO_ERROR:
+break;
+case OFONO_ERROR_TYPE_CME:
+/* Check for SIM busy - try again later */
+if (error.error == 14) {
+if (sd-poll_count++   12) {
+sd-poll_source = g_timeout_add_seconds(2,
+sim_state_check, cbd);
+return;
+}
+}
+/* fall through */
+default:
   cb(error, -1, cbd-data);
-return;
+goto done;
   }

   if (sd-vendor == OFONO_VENDOR_WAVECOM) {

Is there a reason we are not using at_util_sim_state_query_new?

This function is only giving sim present information through its
callback, however I also need to parse the answer from AT+CPIN?
So it means we would send once the AT+CPIN? using atutil helper and once
to parse the answer.
I thought about implementing a new atutil helper function or a new
g_at_chat_send_recursive() function but I saw in
drivers/atmodem/phonebook.c on AT+CPBS=? command that polling mechanism
is integrated into the driver itself.
That's why I have chosen this solution.


Then I'm a bit lost what you're trying to solve.  The ZTE modem driver
is already running CPIN query repeatedly to figure out the SIM state.
So the only reason why we might be receiving a CME ERROR 14 is if we
(successfully) entered the PIN but the modem is busy initializing the
SIM.  In this case polling until CPIN becomes ready from within
at_pin_send_cb seems good enough.  Or is there something I am missing


According to our discussion on IRC, I will send a v2 for which the 
polling is done after PIN is entered.


Kind regards,
Guillaume
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH_v2] atmodem: Poll SIM state after having entered PIN

2012-04-17 Thread Guillaume Zajac
Some modems use SIM notification to check SIM state.
If not do some poll using atutil helper.
---
 drivers/atmodem/sim.c |   28 
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index c51b1d4..269079a 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -51,6 +51,7 @@ struct sim_data {
GAtChat *chat;
unsigned int vendor;
guint ready_id;
+   struct at_util_sim_state_query *sim_state_query;
 };
 
 static const char *crsm_prefix[] = { +CRSM:, NULL };
@@ -972,6 +973,23 @@ static void at_epev_notify(GAtResult *result, gpointer 
user_data)
sd-ready_id = 0;
 }
 
+static void sim_state_cb(gboolean present, gpointer user_data)
+{
+   struct cb_data *cbd = user_data;
+   struct sim_data *sd = cbd-user;
+   ofono_sim_lock_unlock_cb_t cb = cbd-cb;
+
+   at_util_sim_state_query_free(sd-sim_state_query);
+   sd-sim_state_query = NULL;
+
+   if (present == 1)
+   CALLBACK_WITH_SUCCESS(cb, cbd-data);
+   else
+   CALLBACK_WITH_FAILURE(cb, cbd-data);
+
+   g_free(cbd);
+}
+
 static void at_pin_send_cb(gboolean ok, GAtResult *result,
gpointer user_data)
 {
@@ -1005,6 +1023,13 @@ static void at_pin_send_cb(gboolean ok, GAtResult 
*result,
sd-ready_id = g_at_chat_register(sd-chat, *EPEV,
at_epev_notify,
FALSE, cbd, g_free);
+   /*
+* After pin is entered, SIM state is check by doing
+* some polling if modem doesn't use notification.
+*/
+   default:
+   sd-sim_state_query = at_util_sim_state_query_new(sd-chat,
+   2, 20, sim_state_cb, cbd);
return;
}
 
@@ -1246,6 +1271,9 @@ static void at_sim_remove(struct ofono_sim *sim)
 {
struct sim_data *sd = ofono_sim_get_data(sim);
 
+   /* Cleanup potential SIM state polling */
+   at_util_sim_state_query_free(sd-sim_state_query);
+
ofono_sim_set_data(sim, NULL);
 
g_at_chat_unref(sd-chat);
-- 
1.7.4.1

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


Re: [PATCH_v2] atmodem: Poll SIM state after having entered PIN

2012-04-17 Thread Marcel Holtmann
Hi Guillaume,

 Some modems use SIM notification to check SIM state.
 If not do some poll using atutil helper.
 ---
  drivers/atmodem/sim.c |   28 
  1 files changed, 28 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
 index c51b1d4..269079a 100644
 --- a/drivers/atmodem/sim.c
 +++ b/drivers/atmodem/sim.c
 @@ -51,6 +51,7 @@ struct sim_data {
   GAtChat *chat;
   unsigned int vendor;
   guint ready_id;
 + struct at_util_sim_state_query *sim_state_query;
  };
  
  static const char *crsm_prefix[] = { +CRSM:, NULL };
 @@ -972,6 +973,23 @@ static void at_epev_notify(GAtResult *result, gpointer 
 user_data)
   sd-ready_id = 0;
  }
  
 +static void sim_state_cb(gboolean present, gpointer user_data)
 +{
 + struct cb_data *cbd = user_data;
 + struct sim_data *sd = cbd-user;
 + ofono_sim_lock_unlock_cb_t cb = cbd-cb;
 +
 + at_util_sim_state_query_free(sd-sim_state_query);
 + sd-sim_state_query = NULL;
 +
 + if (present == 1)
 + CALLBACK_WITH_SUCCESS(cb, cbd-data);
 + else
 + CALLBACK_WITH_FAILURE(cb, cbd-data);
 +
 + g_free(cbd);
 +}
 +
  static void at_pin_send_cb(gboolean ok, GAtResult *result,
   gpointer user_data)
  {
 @@ -1005,6 +1023,13 @@ static void at_pin_send_cb(gboolean ok, GAtResult 
 *result,
   sd-ready_id = g_at_chat_register(sd-chat, *EPEV,
   at_epev_notify,
   FALSE, cbd, g_free);
 + /*
 +  * After pin is entered, SIM state is check by doing
 +  * some polling if modem doesn't use notification.
 +  */
 + default:
 + sd-sim_state_query = at_util_sim_state_query_new(sd-chat,
 + 2, 20, sim_state_cb, cbd);
   return;
   }

this part just looks wrong. You are now also doing CPIN polling for the
Ericsson and ST-Ericsson cards.

  
 @@ -1246,6 +1271,9 @@ static void at_sim_remove(struct ofono_sim *sim)
  {
   struct sim_data *sd = ofono_sim_get_data(sim);
  
 + /* Cleanup potential SIM state polling */
 + at_util_sim_state_query_free(sd-sim_state_query);
 +

Who owns the cbd data in this case? Are we not leaking that here?

   ofono_sim_set_data(sim, NULL);
  
   g_at_chat_unref(sd-chat);

Regards

Marcel


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


[PATCH v2] Speedup: Use speedup specific driver for ussd

2012-04-17 Thread Nicolas Bertrand
---
 plugins/speedup.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/plugins/speedup.c b/plugins/speedup.c
index ca6ed13..0261f49 100644
--- a/plugins/speedup.c
+++ b/plugins/speedup.c
@@ -2,7 +2,7 @@
  *
  *  oFono - Open Source Telephony
  *
- *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2008-2012  Intel Corporation. All rights reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -282,8 +282,7 @@ static void speedup_post_online(struct ofono_modem *modem)
 
ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
atmodem, data-aux);
-   ofono_ussd_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
-   atmodem, data-aux);
+   ofono_ussd_create(modem, 0, speedupmodem, data-aux);
 }
 
 static struct ofono_modem_driver speedup_driver = {
-- 
1.7.5.4

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


Re: [PATCH_v2] atmodem: Poll SIM state after having entered PIN

2012-04-17 Thread Denis Kenzior
Hi Guillaume,

 Who owns the cbd data in this case? Are we not leaking that here?

 
 Yes, we are. I see 2 options here:
 a- Tweaking current at_util_sim_state_query_new(GAtChat *chat,
 guint interval, guint num_times,
 at_util_sim_inserted_cb_t cb,
 void *userdata, GDestroyNotify data_destroy); in
 adding a destroyer for user data
 
 b- Implementing new gpointer at_util_sim_state_query_get_data(struct
 at_util_sim_state_query *req); and free user_data during remove
 

Option a would be fine.

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


Re: [PATCH v2] Speedup: Use speedup specific driver for ussd

2012-04-17 Thread Denis Kenzior
Hi Nicolas,

On 04/17/2012 07:17 AM, Nicolas Bertrand wrote:
 ---
  plugins/speedup.c |5 ++---
  1 files changed, 2 insertions(+), 3 deletions(-)
 

Patch has been applied, thanks.

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


[PATCH 1/2] atutil: Add destroyer in at_util_sim_state_query_new prototype

2012-04-17 Thread Guillaume Zajac
---
 drivers/atmodem/atutil.c |8 +++-
 drivers/atmodem/atutil.h |3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
index 0c6aab4..7c46dc5 100644
--- a/drivers/atmodem/atutil.c
+++ b/drivers/atmodem/atutil.c
@@ -45,6 +45,7 @@ struct at_util_sim_state_query {
guint num_times;
at_util_sim_inserted_cb_t cb;
void *userdata;
+   GDestroyNotify data_destroy;
 };
 
 static gboolean cpin_check(gpointer userdata);
@@ -574,7 +575,8 @@ static gboolean cpin_check(gpointer userdata)
 struct at_util_sim_state_query *at_util_sim_state_query_new(GAtChat *chat,
guint interval, guint num_times,
at_util_sim_inserted_cb_t cb,
-   void *userdata)
+   void *userdata,
+   GDestroyNotify data_destroy)
 {
struct at_util_sim_state_query *req;
 
@@ -585,6 +587,7 @@ struct at_util_sim_state_query 
*at_util_sim_state_query_new(GAtChat *chat,
req-num_times = num_times;
req-cb = cb;
req-userdata = userdata;
+   req-data_destroy = data_destroy;
 
cpin_check(req);
 
@@ -599,5 +602,8 @@ void at_util_sim_state_query_free(struct 
at_util_sim_state_query *req)
if (req-cpin_poll_source  0)
g_source_remove(req-cpin_poll_source);
 
+   if (req-data_destroy)
+   req-data_destroy(req-userdata);
+
g_free(req);
 }
diff --git a/drivers/atmodem/atutil.h b/drivers/atmodem/atutil.h
index 5046547..5ab9901 100644
--- a/drivers/atmodem/atutil.h
+++ b/drivers/atmodem/atutil.h
@@ -79,7 +79,8 @@ gboolean at_util_parse_attr(GAtResult *result, const char 
*prefix,
 struct at_util_sim_state_query *at_util_sim_state_query_new(GAtChat *chat,
guint interval, guint num_times,
at_util_sim_inserted_cb_t cb,
-   void *userdata);
+   void *userdata,
+   GDestroyNotify data_destroy);
 void at_util_sim_state_query_free(struct at_util_sim_state_query *req);
 
 struct cb_data {
-- 
1.7.4.1

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


[PATCH 0/2] Tweak at_util_sim_state_query_new() function

2012-04-17 Thread Guillaume Zajac
Hi all,

First patch improves at_util_sim_state_query_new() function.
A user data destroyer can be now specifed.
Second is just updating the plugins that are using this function.

Kind regards,
Guillaume

Guillaume Zajac (2):
  atutil: Add destroyer in at_util_sim_state_query_new prototype
  plugins: Update new at_util_sim_state_query_new() prototype use

 drivers/atmodem/atutil.c |8 +++-
 drivers/atmodem/atutil.h |3 ++-
 plugins/alcatel.c|3 ++-
 plugins/mbm.c|2 +-
 plugins/samsung.c|3 ++-
 plugins/speedup.c|3 ++-
 plugins/zte.c|3 ++-
 7 files changed, 18 insertions(+), 7 deletions(-)

-- 
1.7.4.1

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


[PATCH 2/2] plugins: Update new at_util_sim_state_query_new() prototype use

2012-04-17 Thread Guillaume Zajac
---
 plugins/alcatel.c |3 ++-
 plugins/mbm.c |2 +-
 plugins/samsung.c |3 ++-
 plugins/speedup.c |3 ++-
 plugins/zte.c |3 ++-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/plugins/alcatel.c b/plugins/alcatel.c
index fd0e4ec..8005170 100644
--- a/plugins/alcatel.c
+++ b/plugins/alcatel.c
@@ -157,7 +157,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, 
gpointer user_data)
}
 
data-sim_state_query = at_util_sim_state_query_new(data-aux,
-   2, 20, sim_state_cb, modem);
+   2, 20, sim_state_cb, modem,
+   NULL);
 }
 
 static int alcatel_enable(struct ofono_modem *modem)
diff --git a/plugins/mbm.c b/plugins/mbm.c
index 32c7665..035f87e 100644
--- a/plugins/mbm.c
+++ b/plugins/mbm.c
@@ -164,7 +164,7 @@ done:
data-sim_state_query = at_util_sim_state_query_new(data-modem_port,
1, 5,
sim_state_cb,
-   modem);
+   modem, NULL);
 }
 
 static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
diff --git a/plugins/samsung.c b/plugins/samsung.c
index 8e0d360..68a9b0a 100644
--- a/plugins/samsung.c
+++ b/plugins/samsung.c
@@ -142,7 +142,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, 
gpointer user_data)
}
 
data-sim_state_query = at_util_sim_state_query_new(data-chat,
-   1, 5, sim_state_cb, modem);
+   1, 5, sim_state_cb, modem,
+   NULL);
 }
 
 static int samsung_enable(struct ofono_modem *modem)
diff --git a/plugins/speedup.c b/plugins/speedup.c
index ca6ed13..ca33f48 100644
--- a/plugins/speedup.c
+++ b/plugins/speedup.c
@@ -174,7 +174,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, 
gpointer user_data)
}
 
data-sim_state_query = at_util_sim_state_query_new(data-aux,
-   2, 20, sim_state_cb, modem);
+   2, 20, sim_state_cb, modem,
+   NULL);
 }
 
 static int speedup_enable(struct ofono_modem *modem)
diff --git a/plugins/zte.c b/plugins/zte.c
index 3a83c8b..53beefe 100644
--- a/plugins/zte.c
+++ b/plugins/zte.c
@@ -218,7 +218,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, 
gpointer user_data)
}
 
data-sim_state_query = at_util_sim_state_query_new(data-aux,
-   2, 20, sim_state_cb, modem);
+   2, 20, sim_state_cb, modem,
+   NULL);
 }
 
 static int zte_enable(struct ofono_modem *modem)
-- 
1.7.4.1

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


[PATCH_v3] plugins: Update new at_util_sim_state_query_new() prototype use

2012-04-17 Thread Guillaume Zajac
---
 plugins/alcatel.c |3 ++-
 plugins/mbm.c |2 +-
 plugins/samsung.c |3 ++-
 plugins/speedup.c |3 ++-
 plugins/zte.c |3 ++-
 5 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/plugins/alcatel.c b/plugins/alcatel.c
index fd0e4ec..8005170 100644
--- a/plugins/alcatel.c
+++ b/plugins/alcatel.c
@@ -157,7 +157,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, 
gpointer user_data)
}
 
data-sim_state_query = at_util_sim_state_query_new(data-aux,
-   2, 20, sim_state_cb, modem);
+   2, 20, sim_state_cb, modem,
+   NULL);
 }
 
 static int alcatel_enable(struct ofono_modem *modem)
diff --git a/plugins/mbm.c b/plugins/mbm.c
index 32c7665..035f87e 100644
--- a/plugins/mbm.c
+++ b/plugins/mbm.c
@@ -164,7 +164,7 @@ done:
data-sim_state_query = at_util_sim_state_query_new(data-modem_port,
1, 5,
sim_state_cb,
-   modem);
+   modem, NULL);
 }
 
 static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
diff --git a/plugins/samsung.c b/plugins/samsung.c
index 8e0d360..68a9b0a 100644
--- a/plugins/samsung.c
+++ b/plugins/samsung.c
@@ -142,7 +142,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, 
gpointer user_data)
}
 
data-sim_state_query = at_util_sim_state_query_new(data-chat,
-   1, 5, sim_state_cb, modem);
+   1, 5, sim_state_cb, modem,
+   NULL);
 }
 
 static int samsung_enable(struct ofono_modem *modem)
diff --git a/plugins/speedup.c b/plugins/speedup.c
index ca6ed13..ca33f48 100644
--- a/plugins/speedup.c
+++ b/plugins/speedup.c
@@ -174,7 +174,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, 
gpointer user_data)
}
 
data-sim_state_query = at_util_sim_state_query_new(data-aux,
-   2, 20, sim_state_cb, modem);
+   2, 20, sim_state_cb, modem,
+   NULL);
 }
 
 static int speedup_enable(struct ofono_modem *modem)
diff --git a/plugins/zte.c b/plugins/zte.c
index 3a83c8b..53beefe 100644
--- a/plugins/zte.c
+++ b/plugins/zte.c
@@ -218,7 +218,8 @@ static void cfun_enable(gboolean ok, GAtResult *result, 
gpointer user_data)
}
 
data-sim_state_query = at_util_sim_state_query_new(data-aux,
-   2, 20, sim_state_cb, modem);
+   2, 20, sim_state_cb, modem,
+   NULL);
 }
 
 static int zte_enable(struct ofono_modem *modem)
-- 
1.7.4.1

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


Re: [PATCH_v3] plugins: Update new at_util_sim_state_query_new() prototype use

2012-04-17 Thread Guillaume Zajac

Hi All,


On 17/04/2012 16:49, Guillaume Zajac wrote:

---
  plugins/alcatel.c |3 ++-
  plugins/mbm.c |2 +-
  plugins/samsung.c |3 ++-
  plugins/speedup.c |3 ++-
  plugins/zte.c |3 ++-
  5 files changed, 9 insertions(+), 5 deletions(-)


snip

Please ignore this one.

Kind regards,
Guillaume
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono