[PATCH] sim: return from cpin when sim ready for telit

[PATCH] sim: return from cpin when sim ready for telit

[christopher.vogl]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

From: Christopher Vogl <christopher.vogl@hale.at>

Especially for Telit HE910 it is not enough to wait for
entering a PIN code.
If we do not wait for #QSS: 3, subsequent commands,
like +CMER will report SIM BUSY and the network registration
atom will be removed as a consequence.
---
 drivers/atmodem/sim.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 4095337..4448e3d 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -1178,7 +1178,7 @@ static void at_qss_notify(GAtResult *result, gpointer user_data)
 		return;
 
 	switch (state) {
-	case 2:	/* PIN unlocked */
+	case 3:	/* SIM inserted and READY. */
 		break;
 	default:
 		return;
-- 
1.7.7.6

[PATCH] telit: signal sim inserted when sim ready.

[christopher.vogl]

[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]

From: Christopher Vogl <christopher.vogl@hale.at>

We need to wait for #QSS: 3 until SIM inserted may be signaled or
subsequent commands like +CMER will report SIM BUSY which in turn
leads to a removal of the network registration atom. This has been
observed with Telit HE910.

In case of #QSS: 1 we need to check if the SIM is locked.
If the SIM is unlocked we just wait for #QSS: 3,
if the SIM is not unlocked we signal SIM inserted to be able to
enter a PIN code.
---
 plugins/telit.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/plugins/telit.c b/plugins/telit.c
index fe2ccd6..cd67ef4 100644
--- a/plugins/telit.c
+++ b/plugins/telit.c
@@ -61,6 +61,7 @@
 #include "bluetooth.h"
 
 static const char *none_prefix[] = { NULL };
+static const char *cpin_prefix[] = { "+CPIN:", NULL };
 static const char *rsen_prefix[]= { "#RSEN:", NULL };
 
 struct telit_data {
@@ -211,6 +212,33 @@ static GAtChat *open_device(struct ofono_modem *modem,
 	return chat;
 }
 
+static void at_cpin_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct telit_data *data = ofono_modem_get_data(modem);
+	GAtResultIter iter;
+	const char *pin_required;
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CPIN:"))
+		return;
+
+	g_at_result_iter_next_unquoted_string(&iter, &pin_required);
+
+	// No PIN required, we can wait for #QSS: 3.
+	if (strcmp(pin_required, "READY")==0)
+		return;
+
+	if (data->have_sim == FALSE) {
+		ofono_sim_inserted_notify(data->sim, TRUE);
+		data->have_sim = TRUE;
+	}
+}
+
 static void switch_sim_state_status(struct ofono_modem *modem, int status)
 {
 	struct telit_data *data = ofono_modem_get_data(modem);
@@ -226,13 +254,24 @@ static void switch_sim_state_status(struct ofono_modem *modem, int status)
 		}
 		break;
 	case 1:	/* SIM inserted */
+		/*
+		 * We want to call ofono_sim_inserted_notify() only if #QSS: 3
+		 * (SIM READY), as otherwise subsequent commands like +CMER may
+		 * report SIM BUSY.
+		 * In case a PIN is required, we have to signal
+		 * sim inserted on #QSS: 1 and wait in the SIM atom for #QSS: 3
+		 * after entering the PIN.
+		 */
+		g_at_chat_send(data->chat, "AT+CPIN?", cpin_prefix,
+				at_cpin_cb, modem, NULL);
+		break;
 	case 2:	/* SIM inserted and PIN unlocked */
+		break;
+	case 3:	/* SIM inserted and READY (SMS & Phonebook access possible) */
 		if (data->have_sim == FALSE) {
 			ofono_sim_inserted_notify(data->sim, TRUE);
 			data->have_sim = TRUE;
 		}
-		break;
-	case 3:	/* SIM inserted, SMS and phonebook ready */
 		if (data->sms_phonebook_added == FALSE) {
 			ofono_phonebook_create(modem, 0, "atmodem", data->chat);
 			ofono_sms_create(modem, 0, "atmodem", data->chat);
-- 
1.7.7.6

Re: [PATCH] telit: signal sim inserted when sim ready.

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 3380 bytes --]

Hi Christopher,

On 12/14/2012 08:19 AM, christopher.vogl(a)hale.at wrote:
> From: Christopher Vogl<christopher.vogl@hale.at>
>
> We need to wait for #QSS: 3 until SIM inserted may be signaled or
> subsequent commands like +CMER will report SIM BUSY which in turn
> leads to a removal of the network registration atom. This has been
> observed with Telit HE910.
>
> In case of #QSS: 1 we need to check if the SIM is locked.
> If the SIM is unlocked we just wait for #QSS: 3,
> if the SIM is not unlocked we signal SIM inserted to be able to
> enter a PIN code.
> ---
>   plugins/telit.c |   43 +++++++++++++++++++++++++++++++++++++++++--
>   1 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/plugins/telit.c b/plugins/telit.c
> index fe2ccd6..cd67ef4 100644
> --- a/plugins/telit.c
> +++ b/plugins/telit.c
> @@ -61,6 +61,7 @@
>   #include "bluetooth.h"
>
>   static const char *none_prefix[] = { NULL };
> +static const char *cpin_prefix[] = { "+CPIN:", NULL };
>   static const char *rsen_prefix[]= { "#RSEN:", NULL };
>
>   struct telit_data {
> @@ -211,6 +212,33 @@ static GAtChat *open_device(struct ofono_modem *modem,
>   	return chat;
>   }
>
> +static void at_cpin_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +	struct telit_data *data = ofono_modem_get_data(modem);
> +	GAtResultIter iter;
> +	const char *pin_required;
> +
> +	if (!ok)
> +		return;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "+CPIN:"))
> +		return;
> +
> +	g_at_result_iter_next_unquoted_string(&iter,&pin_required);
> +
> +	// No PIN required, we can wait for #QSS: 3.

Nitpicking, but we don't use // style comments.

> +	if (strcmp(pin_required, "READY")==0)
> +		return;
> +
> +	if (data->have_sim == FALSE) {
> +		ofono_sim_inserted_notify(data->sim, TRUE);
> +		data->have_sim = TRUE;
> +	}
> +}
> +
>   static void switch_sim_state_status(struct ofono_modem *modem, int status)
>   {
>   	struct telit_data *data = ofono_modem_get_data(modem);
> @@ -226,13 +254,24 @@ static void switch_sim_state_status(struct ofono_modem *modem, int status)
>   		}
>   		break;
>   	case 1:	/* SIM inserted */
> +		/*
> +		 * We want to call ofono_sim_inserted_notify() only if #QSS: 3
> +		 * (SIM READY), as otherwise subsequent commands like +CMER may
> +		 * report SIM BUSY.
> +		 * In case a PIN is required, we have to signal
> +		 * sim inserted on #QSS: 1 and wait in the SIM atom for #QSS: 3
> +		 * after entering the PIN.
> +		 */
> +		g_at_chat_send(data->chat, "AT+CPIN?", cpin_prefix,
> +				at_cpin_cb, modem, NULL);
> +		break;
>   	case 2:	/* SIM inserted and PIN unlocked */
> +		break;
> +	case 3:	/* SIM inserted and READY (SMS&  Phonebook access possible) */
>   		if (data->have_sim == FALSE) {
>   			ofono_sim_inserted_notify(data->sim, TRUE);
>   			data->have_sim = TRUE;
>   		}
> -		break;
> -	case 3:	/* SIM inserted, SMS and phonebook ready */
>   		if (data->sms_phonebook_added == FALSE) {
>   			ofono_phonebook_create(modem, 0, "atmodem", data->chat);
>   			ofono_sms_create(modem, 0, "atmodem", data->chat);

Since we are waiting for QSS:3, should phonebook and sms atom creation 
be moved out of here and simply put into post_sim()?

Regards,
-Denis

Re: [PATCH] sim: return from cpin when sim ready for telit

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

Hi Christopher,

On 12/14/2012 08:19 AM, christopher.vogl(a)hale.at wrote:
> From: Christopher Vogl<christopher.vogl@hale.at>
>
> Especially for Telit HE910 it is not enough to wait for
> entering a PIN code.
> If we do not wait for #QSS: 3, subsequent commands,
> like +CMER will report SIM BUSY and the network registration
> atom will be removed as a consequence.
> ---
>   drivers/atmodem/sim.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH] telit: signal sim inserted when sim ready.

[Christopher Vogl]

[-- Attachment #1: Type: text/plain, Size: 4229 bytes --]

On 17/12/12 16:50, Denis Kenzior wrote:
> Hi Christopher,
>
> On 12/14/2012 08:19 AM, christopher.vogl(a)hale.at wrote:
>> From: Christopher Vogl<christopher.vogl@hale.at>
>>
>> We need to wait for #QSS: 3 until SIM inserted may be signaled or
>> subsequent commands like +CMER will report SIM BUSY which in turn
>> leads to a removal of the network registration atom. This has been
>> observed with Telit HE910.
>>
>> In case of #QSS: 1 we need to check if the SIM is locked.
>> If the SIM is unlocked we just wait for #QSS: 3,
>> if the SIM is not unlocked we signal SIM inserted to be able to
>> enter a PIN code.
>> ---
>>   plugins/telit.c |   43 +++++++++++++++++++++++++++++++++++++++++--
>>   1 files changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/plugins/telit.c b/plugins/telit.c
>> index fe2ccd6..cd67ef4 100644
>> --- a/plugins/telit.c
>> +++ b/plugins/telit.c
>> @@ -61,6 +61,7 @@
>>   #include "bluetooth.h"
>>
>>   static const char *none_prefix[] = { NULL };
>> +static const char *cpin_prefix[] = { "+CPIN:", NULL };
>>   static const char *rsen_prefix[]= { "#RSEN:", NULL };
>>
>>   struct telit_data {
>> @@ -211,6 +212,33 @@ static GAtChat *open_device(struct ofono_modem 
>> *modem,
>>       return chat;
>>   }
>>
>> +static void at_cpin_cb(gboolean ok, GAtResult *result, gpointer 
>> user_data)
>> +{
>> +    struct ofono_modem *modem = user_data;
>> +    struct telit_data *data = ofono_modem_get_data(modem);
>> +    GAtResultIter iter;
>> +    const char *pin_required;
>> +
>> +    if (!ok)
>> +        return;
>> +
>> +    g_at_result_iter_init(&iter, result);
>> +
>> +    if (!g_at_result_iter_next(&iter, "+CPIN:"))
>> +        return;
>> +
>> + g_at_result_iter_next_unquoted_string(&iter,&pin_required);
>> +
>> +    // No PIN required, we can wait for #QSS: 3.
>
> Nitpicking, but we don't use // style comments.
>
>> +    if (strcmp(pin_required, "READY")==0)
>> +        return;
>> +
>> +    if (data->have_sim == FALSE) {
>> +        ofono_sim_inserted_notify(data->sim, TRUE);
>> +        data->have_sim = TRUE;
>> +    }
>> +}
>> +
>>   static void switch_sim_state_status(struct ofono_modem *modem, int 
>> status)
>>   {
>>       struct telit_data *data = ofono_modem_get_data(modem);
>> @@ -226,13 +254,24 @@ static void switch_sim_state_status(struct 
>> ofono_modem *modem, int status)
>>           }
>>           break;
>>       case 1:    /* SIM inserted */
>> +        /*
>> +         * We want to call ofono_sim_inserted_notify() only if #QSS: 3
>> +         * (SIM READY), as otherwise subsequent commands like +CMER may
>> +         * report SIM BUSY.
>> +         * In case a PIN is required, we have to signal
>> +         * sim inserted on #QSS: 1 and wait in the SIM atom for #QSS: 3
>> +         * after entering the PIN.
>> +         */
>> +        g_at_chat_send(data->chat, "AT+CPIN?", cpin_prefix,
>> +                at_cpin_cb, modem, NULL);
>> +        break;
>>       case 2:    /* SIM inserted and PIN unlocked */
>> +        break;
>> +    case 3:    /* SIM inserted and READY (SMS&  Phonebook access 
>> possible) */
>>           if (data->have_sim == FALSE) {
>>               ofono_sim_inserted_notify(data->sim, TRUE);
>>               data->have_sim = TRUE;
>>           }
>> -        break;
>> -    case 3:    /* SIM inserted, SMS and phonebook ready */
>>           if (data->sms_phonebook_added == FALSE) {
>>               ofono_phonebook_create(modem, 0, "atmodem", data->chat);
>>               ofono_sms_create(modem, 0, "atmodem", data->chat);
>
> Since we are waiting for QSS:3, should phonebook and sms atom creation 
> be moved out of here and simply put into post_sim()?
>

Ok, I will put the creation of those atoms back into post_sim() so it is 
consistent with other plugins again.

I also realised that we only get #QSS: 1 iff a PIN is required, i.e. the 
modem firmware is doing the +CPIN query implicitly for us.
If there is no PIN required we always get #QSS: 2 (SIM UNLOCKED) instead 
of #QSS: 1.
Better leave the PIN querying or would you remove it?

Regards,
Christopher


--
Scanned by MailScanner.

Re: [PATCH] telit: signal sim inserted when sim ready.

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 450 bytes --]

Hi Christopher,

> I also realised that we only get #QSS: 1 iff a PIN is required, i.e. the
> modem firmware is doing the +CPIN query implicitly for us.
> If there is no PIN required we always get #QSS: 2 (SIM UNLOCKED) instead
> of #QSS: 1.
> Better leave the PIN querying or would you remove it?

If this behavior is consistent, then just get rid of CPIN query.  We can 
always add it back later if we really need it.

Regards,
-Denis

[PATCH] telit: signal sim inserted when sim ready.

[Christopher Vogl]

[-- Attachment #1: Type: text/plain, Size: 3337 bytes --]

We need to wait for #QSS: 3 until SIM inserted may be signaled or
subsequent commands like +CMER will report SIM BUSY which in turn
leads to a removal of the network registration atom. This has been
observed with Telit HE910.

In case of #QSS: 1 we need to check if the SIM is locked.
If the SIM is unlocked we just wait for #QSS: 3,
if the SIM is locked we signal SIM inserted to be able to
enter a PIN code.

Since the modem firmware (at least for Telit UC864 and HE910) does only
send #QSS: 1 iff a PIN is required, we can go without an explicit PIN
check.

Move the creation of the phonebook and sms atom back to post_sim()
as we are only getting there after #QSS: 3 (SMS and phonebook ready).
---
 plugins/telit.c |   27 ++++++++++++++++++---------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/plugins/telit.c b/plugins/telit.c
index 79bc421..9907824 100644
--- a/plugins/telit.c
+++ b/plugins/telit.c
@@ -68,7 +68,6 @@ struct telit_data {
 	GAtChat *modem;		/* Data port */
 	struct ofono_sim *sim;
 	ofono_bool_t have_sim;
-	ofono_bool_t sms_phonebook_added;
 	struct ofono_modem *sap_modem;
 	GIOChannel *bt_io;
 	GIOChannel *hw_io;
@@ -232,21 +231,30 @@ static void switch_sim_state_status(struct ofono_modem *modem, int status)
 		if (data->have_sim == TRUE) {
 			ofono_sim_inserted_notify(data->sim, FALSE);
 			data->have_sim = FALSE;
-			data->sms_phonebook_added = FALSE;
 		}
 		break;
 	case 1:	/* SIM inserted */
-	case 2:	/* SIM inserted and PIN unlocked */
+		/*
+		 * We want to call ofono_sim_inserted_notify() only if #QSS: 3
+		 * (SIM READY), as otherwise subsequent commands like +CMER may
+		 * report SIM BUSY.
+		 * In case a PIN is required, we have to signal
+		 * sim inserted on #QSS: 1 and wait in the SIM atom for #QSS: 3
+		 * after entering the PIN.
+		 * The modem will only emit #QSS: 1 if a PIN is required,
+		 * if the SIM is unlocked we get #QSS: 2 right away.
+		 */
 		if (data->have_sim == FALSE) {
 			ofono_sim_inserted_notify(data->sim, TRUE);
 			data->have_sim = TRUE;
 		}
 		break;
-	case 3:	/* SIM inserted, SMS and phonebook ready */
-		if (data->sms_phonebook_added == FALSE) {
-			ofono_phonebook_create(modem, 0, "atmodem", data->chat);
-			ofono_sms_create(modem, 0, "atmodem", data->chat);
-			data->sms_phonebook_added = TRUE;
+	case 2:	/* SIM inserted and PIN unlocked */
+		break;
+	case 3:	/* SIM inserted and READY (SMS & Phonebook access possible) */
+		if (data->have_sim == FALSE) {
+			ofono_sim_inserted_notify(data->sim, TRUE);
+			data->have_sim = TRUE;
 		}
 		break;
 	default:
@@ -297,7 +305,6 @@ static void cfun_enable_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	g_at_chat_send(data->chat, "AT&C0", NULL, NULL, NULL, NULL);
 
 	data->have_sim = FALSE;
-	data->sms_phonebook_added = FALSE;
 
 	ofono_modem_set_powered(m, TRUE);
 
@@ -569,6 +576,8 @@ static void telit_post_sim(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
+	ofono_phonebook_create(modem, 0, "atmodem", data->chat);
+	ofono_sms_create(modem, 0, "atmodem", data->chat);
 	gprs = ofono_gprs_create(modem, OFONO_VENDOR_TELIT, "atmodem",
 					data->chat);
 	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->modem);
-- 
1.7.7.6


--
Scanned by MailScanner.