[PATCH 1/3] Make fetch command function as external

[PATCH 1/3] Make fetch command function as external

[Yang Gu]

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

---
 Makefile.am                |    1 +
 drivers/atmodem/sim-poll.c |   56 ++-----------------------------------------
 drivers/atmodem/stk.c      |   53 +++++++++++++++++++++++++++++++++++++++++
 drivers/atmodem/stk.h      |   22 +++++++++++++++++
 4 files changed, 79 insertions(+), 53 deletions(-)
 create mode 100644 drivers/atmodem/stk.h

diff --git a/Makefile.am b/Makefile.am
index 24aa886..e256841 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -143,6 +143,7 @@ builtin_sources += $(gatchat_sources) \
 				drivers/atmodem/network-registration.c \
 				drivers/atmodem/sim.c \
 				drivers/atmodem/stk.c \
+				drivers/atmodem/stk.h \
 				drivers/atmodem/sim-poll.c \
 				drivers/atmodem/sim-poll.h \
 				drivers/atmodem/ussd.c \
diff --git a/drivers/atmodem/sim-poll.c b/drivers/atmodem/sim-poll.c
index f1a83e3..f79d6df 100644
--- a/drivers/atmodem/sim-poll.c
+++ b/drivers/atmodem/sim-poll.c
@@ -39,6 +39,7 @@
 
 #include "atmodem.h"
 #include "sim-poll.h"
+#include "stk.h"
 
 struct sim_poll_data {
 	GAtChat *chat;
@@ -58,58 +59,6 @@ struct sim_poll_data {
 static const char *csim_prefix[] = { "+CSIM:", NULL };
 
 static gboolean sim_status_poll(gpointer user_data);
-static void sim_fetch_command(struct sim_poll_data *spd, int length);
-
-static void at_csim_fetch_cb(gboolean ok, GAtResult *result,
-		gpointer user_data)
-{
-	struct sim_poll_data *spd = user_data;
-	GAtResultIter iter;
-	const guint8 *response;
-	gint rlen, len;
-
-	if (!ok)
-		return;
-
-	g_at_result_iter_init(&iter, result);
-
-	if (!g_at_result_iter_next(&iter, "+CSIM:"))
-		return;
-
-	if (!g_at_result_iter_next_number(&iter, &rlen))
-		return;
-
-	if (!g_at_result_iter_next_hexstring(&iter, &response, &len))
-		return;
-
-	if (rlen != len * 2 || len < 2)
-		return;
-
-	/* Check that SW1 indicates success */
-	if (response[len - 2] != 0x90 && response[len - 2] != 0x91)
-		return;
-
-	if (response[len - 2] == 0x90 && response[len - 1] != 0)
-		return;
-
-	DBG("csim_fetch_cb: %i", len);
-
-	ofono_stk_proactive_command_notify(spd->stk, len - 2, response);
-
-	/* Can this happen? */
-	if (response[len - 2] == 0x91)
-		sim_fetch_command(spd, response[len - 1]);
-}
-
-static void sim_fetch_command(struct sim_poll_data *spd, int length)
-{
-	char buf[64];
-
-	snprintf(buf, sizeof(buf), "AT+CSIM=10,A0120000%02hhX", length);
-
-	g_at_chat_send(spd->chat, buf, csim_prefix,
-			at_csim_fetch_cb, spd, NULL);
-}
 
 static void sim_status_poll_schedule(struct sim_poll_data *spd)
 {
@@ -196,7 +145,7 @@ static void at_csim_status_cb(gboolean ok, GAtResult *result,
 		return;
 
 	/* We have a proactive command pending, FETCH it */
-	sim_fetch_command(spd, response[len - 1]);
+	at_sim_fetch_command(spd->chat, spd->stk, response[len - 1]);
 }
 
 static gboolean sim_status_poll(gpointer user_data)
@@ -317,3 +266,4 @@ void atmodem_poll_enable(struct ofono_modem *modem, GAtChat *chat)
 		sim_watch(sim_atom,
 				OFONO_ATOM_WATCH_CONDITION_REGISTERED, spd);
 }
+
diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
index aede668..6058d69 100644
--- a/drivers/atmodem/stk.c
+++ b/drivers/atmodem/stk.c
@@ -38,6 +38,7 @@
 #include "gatresult.h"
 
 #include "atmodem.h"
+#include "stk.h"
 
 struct stk_data {
 	GAtChat *chat;
@@ -45,6 +46,58 @@ struct stk_data {
 
 static const char *csim_prefix[] = { "+CSIM:", NULL };
 
+static void csim_fetch_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	GAtChat *chat = cbd->cb;
+	struct ofono_stk *stk = cbd->data;
+	GAtResultIter iter;
+	const guint8 *response;
+	gint rlen, len;
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CSIM:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &rlen))
+		return;
+
+	if (!g_at_result_iter_next_hexstring(&iter, &response, &len))
+		return;
+
+	if (rlen != len * 2 || len < 2)
+		return;
+
+	/* Check that SW1 indicates success */
+	if (response[len - 2] != 0x90 && response[len - 2] != 0x91)
+		return;
+
+	if (response[len - 2] == 0x90 && response[len - 1] != 0)
+		return;
+
+	DBG("csim_fetch_cb: %i", len);
+
+	ofono_stk_proactive_command_notify(stk, len - 2, response);
+
+	/* Can this happen? */
+	if (response[len - 2] == 0x91)
+		at_sim_fetch_command(chat, stk, response[len - 1]);
+}
+
+void at_sim_fetch_command(GAtChat *chat, struct ofono_stk *stk, int length)
+{
+	char buf[64];
+	struct cb_data *cbd = cb_data_new(chat, stk);
+
+	snprintf(buf, sizeof(buf), "AT+CSIM=10,A0120000%02hhX", length);
+	g_at_chat_send(chat, buf, csim_prefix, csim_fetch_cb, cbd, g_free);
+}
+
 static void at_csim_envelope_cb(gboolean ok, GAtResult *result,
 				gpointer user_data)
 {
diff --git a/drivers/atmodem/stk.h b/drivers/atmodem/stk.h
new file mode 100644
index 0000000..6e998ac
--- /dev/null
+++ b/drivers/atmodem/stk.h
@@ -0,0 +1,22 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  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
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+void at_sim_fetch_command(GAtChat *chat, struct ofono_stk *stk, int length);
-- 
1.7.0.4

[PATCH 2/3] Support phonesim proactive command notification

[Yang Gu]

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

---
 drivers/atmodem/stk.c |   25 +++++++++++++++++++++++++
 plugins/phonesim.c    |    2 +-
 2 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
index 6058d69..91b9195 100644
--- a/drivers/atmodem/stk.c
+++ b/drivers/atmodem/stk.c
@@ -39,9 +39,11 @@
 
 #include "atmodem.h"
 #include "stk.h"
+#include "vendor.h"
 
 struct stk_data {
 	GAtChat *chat;
+	unsigned int vendor;
 };
 
 static const char *csim_prefix[] = { "+CSIM:", NULL };
@@ -264,10 +266,32 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static void phonesim_tcmd_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_stk *stk = user_data;
+	struct stk_data *data = ofono_stk_get_data(stk);
+	GAtResultIter iter;
+	int length;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "*TCMD:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &length))
+		return;
+
+	at_sim_fetch_command(data->chat, stk, length);
+}
+
 static gboolean at_stk_register(gpointer user)
 {
 	struct ofono_stk *stk = user;
+	struct stk_data *sd = ofono_stk_get_data(stk);
 
+	if (sd->vendor == OFONO_VENDOR_PHONESIM)
+		g_at_chat_register(sd->chat, "*TCMD", phonesim_tcmd_notify,
+							FALSE, stk, NULL);
 	ofono_stk_register(stk);
 
 	return FALSE;
@@ -280,6 +304,7 @@ static int at_stk_probe(struct ofono_stk *stk, unsigned int vendor, void *data)
 
 	sd = g_new0(struct stk_data, 1);
 	sd->chat = chat;
+	sd->vendor = vendor;
 
 	ofono_stk_set_data(stk, sd);
 	g_idle_add(at_stk_register, stk);
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index d0cd7f3..9c5b2cb 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -289,7 +289,7 @@ static void phonesim_pre_sim(struct ofono_modem *modem)
 	else
 		ofono_voicecall_create(modem, 0, "atmodem", data->chat);
 
-	ofono_stk_create(modem, 0, "atmodem", data->chat);
+	ofono_stk_create(modem, OFONO_VENDOR_PHONESIM, "atmodem", data->chat);
 
 	if (sim)
 		ofono_sim_inserted_notify(sim, TRUE);
-- 
1.7.0.4

Re: [PATCH 2/3] Support phonesim proactive command notification

[Denis Kenzior]

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

Hi Yang,

On 07/09/2010 09:52 AM, Yang Gu wrote:
> ---
>  drivers/atmodem/stk.c |   25 +++++++++++++++++++++++++
>  plugins/phonesim.c    |    2 +-
>  2 files changed, 26 insertions(+), 1 deletions(-)

If possible, please break up your patches when crossing a directory
boundary.  E.g. a patch for drivers/atmodem and a patch for
plugins/phonesim.

Thanks,
-Denis

[PATCH 3/3] Interpret "" Alpha Id as empty data object

[Yang Gu]

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

There needs to be a way to distinguish between no alphaid and "empty
data object" because on some occasions they have different meanings.  In
the Call Control envelope, no Alpha Identifier means the terminal can
inform the user about the call being modified by SIM while empty data
object means no hint should be given.
---
 src/stkutil.c       |    4 +++-
 unit/test-stkutil.c |   18 ++++++++++++++----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 94ff182..c45f985 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -297,8 +297,10 @@ static gboolean parse_dataobj_alpha_id(struct comprehension_tlv_iter *iter,
 	char *utf8;
 
 	len = comprehension_tlv_iter_get_length(iter);
-	if (len == 0)
+	if (len == 0) {
+		*alpha_id = g_try_malloc0(1);
 		return TRUE;
+	}
 
 	data = comprehension_tlv_iter_get_data(iter);
 	utf8 = sim_string_to_utf8(data, len);
diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
index 0b95265..dbd5b5e 100644
--- a/unit/test-stkutil.c
+++ b/unit/test-stkutil.c
@@ -4974,7 +4974,8 @@ static struct setup_menu_test setup_menu_data_112 = {
 static struct setup_menu_test setup_menu_data_113 = {
 	.pdu = setup_menu_113,
 	.pdu_len = sizeof(setup_menu_113),
-	.qualifier = 0x00
+	.qualifier = 0x00,
+	.alpha_id = ""
 };
 
 static struct setup_menu_test setup_menu_data_121 = {
@@ -5442,7 +5443,8 @@ static struct setup_menu_test setup_menu_data_712 = {
 static struct setup_menu_test setup_menu_data_713 = {
 	.pdu = setup_menu_713,
 	.pdu_len = sizeof(setup_menu_713),
-	.qualifier = 0x00
+	.qualifier = 0x00,
+	.alpha_id = ""
 };
 
 static struct setup_menu_test setup_menu_data_811 = {
@@ -5472,7 +5474,8 @@ static struct setup_menu_test setup_menu_data_812 = {
 static struct setup_menu_test setup_menu_data_813 = {
 	.pdu = setup_menu_813,
 	.pdu_len = sizeof(setup_menu_813),
-	.qualifier = 0x00
+	.qualifier = 0x00,
+	.alpha_id = ""
 };
 
 static struct setup_menu_test setup_menu_data_911 = {
@@ -5502,7 +5505,8 @@ static struct setup_menu_test setup_menu_data_912 = {
 static struct setup_menu_test setup_menu_data_913 = {
 	.pdu = setup_menu_913,
 	.pdu_len = sizeof(setup_menu_913),
-	.qualifier = 0x00
+	.qualifier = 0x00,
+	.alpha_id = ""
 };
 
 static struct setup_menu_test setup_menu_data_neg_1 = {
@@ -7957,6 +7961,7 @@ static struct send_sms_test send_sms_data_171 = {
 	.pdu = send_sms_171,
 	.pdu_len = sizeof(send_sms_171),
 	.qualifier = 0x00,
+	.alpha_id = "",
 	.gsm_sms = {
 		{
 			.number_type = SMS_NUMBER_TYPE_INTERNATIONAL,
@@ -9474,6 +9479,7 @@ static struct send_ss_test send_ss_data_161 = {
 	.pdu = send_ss_161,
 	.pdu_len = sizeof(send_ss_161),
 	.qualifier = 0x00,
+	.alpha_id = "",
 	.ss = {
 		.ton_npi = 0x91,
 		.ss = "**21*01234567890123456789*10#"
@@ -10774,6 +10780,7 @@ static struct send_ussd_test send_ussd_data_181 = {
 	.pdu = send_ussd_181,
 	.pdu_len = sizeof(send_ussd_181),
 	.qualifier = 0x00,
+	.alpha_id = "",
 	.ussd = "ABCDEFGHIJKLMNOPQRSTUVWXYZ-abcdefghijklmnopqrstuvwxyz-"
 		"1234567890"
 };
@@ -14621,6 +14628,7 @@ static struct run_at_command_test run_at_command_data_121 = {
 	.pdu = run_at_command_121,
 	.pdu_len = sizeof(run_at_command_121),
 	.qualifier = 0x00,
+	.alpha_id = "",
 	.at_command = "AT+CGMI"
 };
 
@@ -15319,6 +15327,7 @@ static struct send_dtmf_test send_dtmf_data_131 = {
 	.pdu = send_dtmf_131,
 	.pdu_len = sizeof(send_dtmf_131),
 	.qualifier = 0x00,
+	.alpha_id = "",
 	.dtmf = "1cccccccccc2"
 };
 
@@ -16039,6 +16048,7 @@ static struct launch_browser_test launch_browser_data_121 = {
 	.pdu = launch_browser_121,
 	.pdu_len = sizeof(launch_browser_121),
 	.qualifier = 0x00,
+	.alpha_id = "",
 	.url = "http://xxx.yyy.zzz"
 };
 
-- 
1.7.0.4

RE: [PATCH 3/3] Interpret "" Alpha Id as empty data object

[Gu, Yang]

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

Hi, 
	I "stole" this patch from Andrew, and handled some more cases in send ss and send ussd. As he is off these days and I need this patch for my code, so I dig it out and resubmit here. 
	I declare Andrew still has the copyright of this patch :)

>-----Original Message-----
>From: Yang Gu [mailto:gyagp0(a)gmail.com]
>Sent: Friday, July 09, 2010 10:52 PM
>To: ofono(a)ofono.org
>Cc: Gu, Yang
>Subject: [PATCH 3/3] Interpret "" Alpha Id as empty data object
>
>There needs to be a way to distinguish between no alphaid and "empty
>data object" because on some occasions they have different meanings.  In
>the Call Control envelope, no Alpha Identifier means the terminal can
>inform the user about the call being modified by SIM while empty data
>object means no hint should be given.
>---
> src/stkutil.c       |    4 +++-
> unit/test-stkutil.c |   18 ++++++++++++++----
> 2 files changed, 17 insertions(+), 5 deletions(-)
>
>diff --git a/src/stkutil.c b/src/stkutil.c
>index 94ff182..c45f985 100644
>--- a/src/stkutil.c
>+++ b/src/stkutil.c
>@@ -297,8 +297,10 @@ static gboolean parse_dataobj_alpha_id(struct
>comprehension_tlv_iter *iter,
> 	char *utf8;
>
> 	len = comprehension_tlv_iter_get_length(iter);
>-	if (len == 0)
>+	if (len == 0) {
>+		*alpha_id = g_try_malloc0(1);
> 		return TRUE;
>+	}
>
> 	data = comprehension_tlv_iter_get_data(iter);
> 	utf8 = sim_string_to_utf8(data, len);
>diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
>index 0b95265..dbd5b5e 100644
>--- a/unit/test-stkutil.c
>+++ b/unit/test-stkutil.c
>@@ -4974,7 +4974,8 @@ static struct setup_menu_test setup_menu_data_112 = {
> static struct setup_menu_test setup_menu_data_113 = {
> 	.pdu = setup_menu_113,
> 	.pdu_len = sizeof(setup_menu_113),
>-	.qualifier = 0x00
>+	.qualifier = 0x00,
>+	.alpha_id = ""
> };
>
> static struct setup_menu_test setup_menu_data_121 = {
>@@ -5442,7 +5443,8 @@ static struct setup_menu_test setup_menu_data_712 = {
> static struct setup_menu_test setup_menu_data_713 = {
> 	.pdu = setup_menu_713,
> 	.pdu_len = sizeof(setup_menu_713),
>-	.qualifier = 0x00
>+	.qualifier = 0x00,
>+	.alpha_id = ""
> };
>
> static struct setup_menu_test setup_menu_data_811 = {
>@@ -5472,7 +5474,8 @@ static struct setup_menu_test setup_menu_data_812 = {
> static struct setup_menu_test setup_menu_data_813 = {
> 	.pdu = setup_menu_813,
> 	.pdu_len = sizeof(setup_menu_813),
>-	.qualifier = 0x00
>+	.qualifier = 0x00,
>+	.alpha_id = ""
> };
>
> static struct setup_menu_test setup_menu_data_911 = {
>@@ -5502,7 +5505,8 @@ static struct setup_menu_test setup_menu_data_912 = {
> static struct setup_menu_test setup_menu_data_913 = {
> 	.pdu = setup_menu_913,
> 	.pdu_len = sizeof(setup_menu_913),
>-	.qualifier = 0x00
>+	.qualifier = 0x00,
>+	.alpha_id = ""
> };
>
> static struct setup_menu_test setup_menu_data_neg_1 = {
>@@ -7957,6 +7961,7 @@ static struct send_sms_test send_sms_data_171 = {
> 	.pdu = send_sms_171,
> 	.pdu_len = sizeof(send_sms_171),
> 	.qualifier = 0x00,
>+	.alpha_id = "",
> 	.gsm_sms = {
> 		{
> 			.number_type = SMS_NUMBER_TYPE_INTERNATIONAL,
>@@ -9474,6 +9479,7 @@ static struct send_ss_test send_ss_data_161 = {
> 	.pdu = send_ss_161,
> 	.pdu_len = sizeof(send_ss_161),
> 	.qualifier = 0x00,
>+	.alpha_id = "",
> 	.ss = {
> 		.ton_npi = 0x91,
> 		.ss = "**21*01234567890123456789*10#"
>@@ -10774,6 +10780,7 @@ static struct send_ussd_test send_ussd_data_181 = {
> 	.pdu = send_ussd_181,
> 	.pdu_len = sizeof(send_ussd_181),
> 	.qualifier = 0x00,
>+	.alpha_id = "",
> 	.ussd = "ABCDEFGHIJKLMNOPQRSTUVWXYZ-abcdefghijklmnopqrstuvwxyz-"
> 		"1234567890"
> };
>@@ -14621,6 +14628,7 @@ static struct run_at_command_test
>run_at_command_data_121 = {
> 	.pdu = run_at_command_121,
> 	.pdu_len = sizeof(run_at_command_121),
> 	.qualifier = 0x00,
>+	.alpha_id = "",
> 	.at_command = "AT+CGMI"
> };
>
>@@ -15319,6 +15327,7 @@ static struct send_dtmf_test send_dtmf_data_131 = {
> 	.pdu = send_dtmf_131,
> 	.pdu_len = sizeof(send_dtmf_131),
> 	.qualifier = 0x00,
>+	.alpha_id = "",
> 	.dtmf = "1cccccccccc2"
> };
>
>@@ -16039,6 +16048,7 @@ static struct launch_browser_test
>launch_browser_data_121 = {
> 	.pdu = launch_browser_121,
> 	.pdu_len = sizeof(launch_browser_121),
> 	.qualifier = 0x00,
>+	.alpha_id = "",
> 	.url = "http://xxx.yyy.zzz"
> };
>
>--
>1.7.0.4



Regards,
-Yang

Re: [PATCH 3/3] Interpret "" Alpha Id as empty data object

[Denis Kenzior]

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

On 07/09/2010 09:52 AM, Yang Gu wrote:
> There needs to be a way to distinguish between no alphaid and "empty
> data object" because on some occasions they have different meanings.  In
> the Call Control envelope, no Alpha Identifier means the terminal can
> inform the user about the call being modified by SIM while empty data
> object means no hint should be given.

I applied this one, however I noticed that lots of unit tests for things
besides call control were affected.  Question to both of you: Should we
g_free & NULL empty Alpha Identifiers in commands that do not allow this
to be empty?

Regards,
-Denis

RE: [PATCH 3/3] Interpret "" Alpha Id as empty data object

[Gu, Yang]

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

Hi Denis,

>-----Original Message-----
>From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf Of
>Denis Kenzior
>Sent: Saturday, July 10, 2010 1:32 AM
>To: ofono(a)ofono.org
>Cc: Zaborowski, Andrew; Yang Gu
>Subject: Re: [PATCH 3/3] Interpret "" Alpha Id as empty data object
>
>On 07/09/2010 09:52 AM, Yang Gu wrote:
>> There needs to be a way to distinguish between no alphaid and "empty
>> data object" because on some occasions they have different meanings.  In
>> the Call Control envelope, no Alpha Identifier means the terminal can
>> inform the user about the call being modified by SIM while empty data
>> object means no hint should be given.
>
>I applied this one, however I noticed that lots of unit tests for things
>besides call control were affected.  Question to both of you: Should we
>g_free & NULL empty Alpha Identifiers in commands that do not allow this
>to be empty?

If we know the alpha_id in some ber-tlvs couldn't be empty, we need an extra check and report the error. But the question is do we know any?
Currently I think the empty alpha_id has similar meaning for the application.

>
>Regards,
>-Denis
>_______________________________________________
>ofono mailing list
>ofono(a)ofono.org
>http://lists.ofono.org/listinfo/ofono

Regards,
-Yang

RE: [PATCH 1/3] Make fetch command function as external

[Gu, Yang]

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

Hi,

>-----Original Message-----
>From: Yang Gu [mailto:gyagp0(a)gmail.com]
>Sent: Friday, July 09, 2010 10:52 PM
>To: ofono(a)ofono.org
>Cc: Gu, Yang
>Subject: [PATCH 1/3] Make fetch command function as external
>+
>+void at_sim_fetch_command(GAtChat *chat, struct ofono_stk *stk, int length)
>+{
>+	char buf[64];
>+	struct cb_data *cbd = cb_data_new(chat, stk);
>+
>+	snprintf(buf, sizeof(buf), "AT+CSIM=10,A0120000%02hhX", length);
>+	g_at_chat_send(chat, buf, csim_prefix, csim_fetch_cb, cbd, g_free);

Both chat and stk need to be passed to callback function, so I use cb_data here. Any comment?



Regards,
-Yang

Re: [PATCH 1/3] Make fetch command function as external

[Denis Kenzior]

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

Hi Yang,

>  static gboolean sim_status_poll(gpointer user_data)
> @@ -317,3 +266,4 @@ void atmodem_poll_enable(struct ofono_modem *modem, GAtChat *chat)
>  		sim_watch(sim_atom,
>  				OFONO_ATOM_WATCH_CONDITION_REGISTERED, spd);
>  }
> +

Please note that I also get an error applying this patch:

Applying: Make fetch command function as external
/home/denkenz/ofono-master/.git/rebase-apply/patch:101: new blank line
at EOF.
+
fatal: 1 line adds whitespace errors.

> +void at_sim_fetch_command(GAtChat *chat, struct ofono_stk *stk, int length)
> +{
> +	char buf[64];
> +	struct cb_data *cbd = cb_data_new(chat, stk);
> +
> +	snprintf(buf, sizeof(buf), "AT+CSIM=10,A0120000%02hhX", length);
> +	g_at_chat_send(chat, buf, csim_prefix, csim_fetch_cb, cbd, g_free);
> +}
> +

There's no reason to pass in the chat object.  The stk_data hanging off
ofono_stk object already has a chat inside it.  So simply using that one
is enough.  This also means you don't need to create a cbd, passing stk
into g_at_chat_send is enough.

Regards,
-Denis

[PATCH 1/3] Make fetch command function as external

[Yang Gu]

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

---
 Makefile.am                |    1 +
 drivers/atmodem/sim-poll.c |   55 +------------------------------------------
 drivers/atmodem/stk.c      |   51 ++++++++++++++++++++++++++++++++++++++++
 drivers/atmodem/stk.h      |   22 +++++++++++++++++
 4 files changed, 76 insertions(+), 53 deletions(-)
 create mode 100644 drivers/atmodem/stk.h

diff --git a/Makefile.am b/Makefile.am
index 24aa886..e256841 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -143,6 +143,7 @@ builtin_sources += $(gatchat_sources) \
 				drivers/atmodem/network-registration.c \
 				drivers/atmodem/sim.c \
 				drivers/atmodem/stk.c \
+				drivers/atmodem/stk.h \
 				drivers/atmodem/sim-poll.c \
 				drivers/atmodem/sim-poll.h \
 				drivers/atmodem/ussd.c \
diff --git a/drivers/atmodem/sim-poll.c b/drivers/atmodem/sim-poll.c
index f1a83e3..3f1a355 100644
--- a/drivers/atmodem/sim-poll.c
+++ b/drivers/atmodem/sim-poll.c
@@ -39,6 +39,7 @@
 
 #include "atmodem.h"
 #include "sim-poll.h"
+#include "stk.h"
 
 struct sim_poll_data {
 	GAtChat *chat;
@@ -58,58 +59,6 @@ struct sim_poll_data {
 static const char *csim_prefix[] = { "+CSIM:", NULL };
 
 static gboolean sim_status_poll(gpointer user_data);
-static void sim_fetch_command(struct sim_poll_data *spd, int length);
-
-static void at_csim_fetch_cb(gboolean ok, GAtResult *result,
-		gpointer user_data)
-{
-	struct sim_poll_data *spd = user_data;
-	GAtResultIter iter;
-	const guint8 *response;
-	gint rlen, len;
-
-	if (!ok)
-		return;
-
-	g_at_result_iter_init(&iter, result);
-
-	if (!g_at_result_iter_next(&iter, "+CSIM:"))
-		return;
-
-	if (!g_at_result_iter_next_number(&iter, &rlen))
-		return;
-
-	if (!g_at_result_iter_next_hexstring(&iter, &response, &len))
-		return;
-
-	if (rlen != len * 2 || len < 2)
-		return;
-
-	/* Check that SW1 indicates success */
-	if (response[len - 2] != 0x90 && response[len - 2] != 0x91)
-		return;
-
-	if (response[len - 2] == 0x90 && response[len - 1] != 0)
-		return;
-
-	DBG("csim_fetch_cb: %i", len);
-
-	ofono_stk_proactive_command_notify(spd->stk, len - 2, response);
-
-	/* Can this happen? */
-	if (response[len - 2] == 0x91)
-		sim_fetch_command(spd, response[len - 1]);
-}
-
-static void sim_fetch_command(struct sim_poll_data *spd, int length)
-{
-	char buf[64];
-
-	snprintf(buf, sizeof(buf), "AT+CSIM=10,A0120000%02hhX", length);
-
-	g_at_chat_send(spd->chat, buf, csim_prefix,
-			at_csim_fetch_cb, spd, NULL);
-}
 
 static void sim_status_poll_schedule(struct sim_poll_data *spd)
 {
@@ -196,7 +145,7 @@ static void at_csim_status_cb(gboolean ok, GAtResult *result,
 		return;
 
 	/* We have a proactive command pending, FETCH it */
-	sim_fetch_command(spd, response[len - 1]);
+	at_sim_fetch_command(spd->stk, response[len - 1]);
 }
 
 static gboolean sim_status_poll(gpointer user_data)
diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
index aede668..1283cca 100644
--- a/drivers/atmodem/stk.c
+++ b/drivers/atmodem/stk.c
@@ -38,6 +38,7 @@
 #include "gatresult.h"
 
 #include "atmodem.h"
+#include "stk.h"
 
 struct stk_data {
 	GAtChat *chat;
@@ -45,6 +46,56 @@ struct stk_data {
 
 static const char *csim_prefix[] = { "+CSIM:", NULL };
 
+static void csim_fetch_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct ofono_stk *stk = user_data;
+	GAtResultIter iter;
+	const guint8 *response;
+	gint rlen, len;
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CSIM:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &rlen))
+		return;
+
+	if (!g_at_result_iter_next_hexstring(&iter, &response, &len))
+		return;
+
+	if (rlen != len * 2 || len < 2)
+		return;
+
+	/* Check that SW1 indicates success */
+	if (response[len - 2] != 0x90 && response[len - 2] != 0x91)
+		return;
+
+	if (response[len - 2] == 0x90 && response[len - 1] != 0)
+		return;
+
+	DBG("csim_fetch_cb: %i", len);
+
+	ofono_stk_proactive_command_notify(stk, len - 2, response);
+
+	/* Can this happen? */
+	if (response[len - 2] == 0x91)
+		at_sim_fetch_command(stk, response[len - 1]);
+}
+
+void at_sim_fetch_command(struct ofono_stk *stk, int length)
+{
+	char buf[64];
+	struct stk_data *sd = ofono_stk_get_data(stk);
+
+	snprintf(buf, sizeof(buf), "AT+CSIM=10,A0120000%02hhX", length);
+	g_at_chat_send(sd->chat, buf, csim_prefix, csim_fetch_cb, stk, NULL);
+}
+
 static void at_csim_envelope_cb(gboolean ok, GAtResult *result,
 				gpointer user_data)
 {
diff --git a/drivers/atmodem/stk.h b/drivers/atmodem/stk.h
new file mode 100644
index 0000000..265ac2e
--- /dev/null
+++ b/drivers/atmodem/stk.h
@@ -0,0 +1,22 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  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
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+void at_sim_fetch_command(struct ofono_stk *stk, int length);
-- 
1.7.0.4

[PATCH 2/3] Support phonesim proactive command notification

[Yang Gu]

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

---
 drivers/atmodem/stk.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
index 1283cca..3850f15 100644
--- a/drivers/atmodem/stk.c
+++ b/drivers/atmodem/stk.c
@@ -39,9 +39,11 @@
 
 #include "atmodem.h"
 #include "stk.h"
+#include "vendor.h"
 
 struct stk_data {
 	GAtChat *chat;
+	unsigned int vendor;
 };
 
 static const char *csim_prefix[] = { "+CSIM:", NULL };
@@ -262,10 +264,32 @@ error:
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static void phonesim_tcmd_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_stk *stk = user_data;
+	struct stk_data *data = ofono_stk_get_data(stk);
+	GAtResultIter iter;
+	int length;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "*TCMD:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &length))
+		return;
+
+	at_sim_fetch_command(data->chat, stk, length);
+}
+
 static gboolean at_stk_register(gpointer user)
 {
 	struct ofono_stk *stk = user;
+	struct stk_data *sd = ofono_stk_get_data(stk);
 
+	if (sd->vendor == OFONO_VENDOR_PHONESIM)
+		g_at_chat_register(sd->chat, "*TCMD", phonesim_tcmd_notify,
+							FALSE, stk, NULL);
 	ofono_stk_register(stk);
 
 	return FALSE;
@@ -278,6 +302,7 @@ static int at_stk_probe(struct ofono_stk *stk, unsigned int vendor, void *data)
 
 	sd = g_new0(struct stk_data, 1);
 	sd->chat = chat;
+	sd->vendor = vendor;
 
 	ofono_stk_set_data(stk, sd);
 	g_idle_add(at_stk_register, stk);
-- 
1.7.0.4

Re: [PATCH 2/3] Support phonesim proactive command notification

[Denis Kenzior]

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

Hi Yang,

> +static void phonesim_tcmd_notify(GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_stk *stk = user_data;
> +	struct stk_data *data = ofono_stk_get_data(stk);
> +	GAtResultIter iter;
> +	int length;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "*TCMD:"))
> +		return;
> +
> +	if (!g_at_result_iter_next_number(&iter, &length))
> +		return;
> +
> +	at_sim_fetch_command(data->chat, stk, length);
> +}
> +

drivers/atmodem/stk.c: In function ‘phonesim_tcmd_notify’:
drivers/atmodem/stk.c:282: error: passing argument 1 of
‘at_sim_fetch_command’ from incompatible pointer type
drivers/atmodem/stk.c:92: note: expected ‘struct ofono_stk *’ but
argument is of type ‘struct GAtChat *’
drivers/atmodem/stk.c:282: error: passing argument 2 of
‘at_sim_fetch_command’ makes integer from pointer without a cast
drivers/atmodem/stk.c:92: note: expected ‘int’ but argument is of type
‘struct ofono_stk *’
drivers/atmodem/stk.c:282: error: too many arguments to function
‘at_sim_fetch_command’

I fixed this up for you since it was trivial, however please at least
compile-check your patches ;)

Regards,
-Denis