[PATCH 7/8] Add AT driver for STK atom.

[PATCH 7/8] Add AT driver for STK atom.

[Andrzej Zaborowski]

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

---
 Makefile.am               |    1 +
 drivers/atmodem/atmodem.c |    2 +
 drivers/atmodem/atmodem.h |    3 +
 drivers/atmodem/stk.c     |  258 +++++++++++++++++++++++++++++++++++++++++++++
 plugins/atgen.c           |    2 +
 plugins/phonesim.c        |    3 +
 6 files changed, 269 insertions(+), 0 deletions(-)
 create mode 100644 drivers/atmodem/stk.c

diff --git a/Makefile.am b/Makefile.am
index 7a18c41..a506dba 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -124,6 +124,7 @@ builtin_sources += $(gatchat_sources) \
 				drivers/atmodem/call-meter.c \
 				drivers/atmodem/network-registration.c \
 				drivers/atmodem/sim.c \
+				drivers/atmodem/stk.c \
 				drivers/atmodem/ussd.c \
 				drivers/atmodem/voicecall.c \
 				drivers/atmodem/call-barring.c \
diff --git a/drivers/atmodem/atmodem.c b/drivers/atmodem/atmodem.c
index 0ac8182..c88f6b2 100644
--- a/drivers/atmodem/atmodem.c
+++ b/drivers/atmodem/atmodem.c
@@ -45,6 +45,7 @@ static int atmodem_init(void)
 	at_ussd_init();
 	at_sms_init();
 	at_sim_init();
+	at_stk_init();
 	at_netreg_init();
 	at_cbs_init();
 	at_call_volume_init();
@@ -56,6 +57,7 @@ static int atmodem_init(void)
 
 static void atmodem_exit(void)
 {
+	at_stk_exit();
 	at_sim_exit();
 	at_sms_exit();
 	at_ussd_exit();
diff --git a/drivers/atmodem/atmodem.h b/drivers/atmodem/atmodem.h
index 1fb4bfa..2ee47f5 100644
--- a/drivers/atmodem/atmodem.h
+++ b/drivers/atmodem/atmodem.h
@@ -45,6 +45,9 @@ extern void at_call_barring_exit();
 extern void at_sim_init();
 extern void at_sim_exit();
 
+extern void at_stk_init();
+extern void at_stk_exit();
+
 extern void at_sms_init();
 extern void at_sms_exit();
 
diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
new file mode 100644
index 0000000..05c1138
--- /dev/null
+++ b/drivers/atmodem/stk.c
@@ -0,0 +1,258 @@
+/*
+ *
+ *  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
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#include <glib.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/stk.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "atmodem.h"
+
+struct stk_data {
+	GAtChat *chat;
+};
+
+static const char *csim_prefix[] = { "+CSIM:", NULL };
+
+static void at_csim_envelope_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	GAtResultIter iter;
+	ofono_stk_envelope_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	const guint8 *response;
+	gint rlen, len;
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok)
+		goto error;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CSIM:"))
+		goto error;
+
+	if (!g_at_result_iter_next_number(&iter, &rlen))
+		goto error;
+
+	if (!g_at_result_iter_next_hexstring(&iter, &response, &len))
+		goto error;
+
+	if (rlen != len * 2 || len < 2)
+		goto error;
+
+	if (response[len - 2] != 0x90 && response[len - 2] != 0x91)
+		goto error;
+
+	if (response[len - 2] == 0x90 && response[len - 1] != 0)
+		goto error;
+
+	DBG("csim_envelope_cb: %i", len);
+
+	cb(&error, response, len - 2, cbd->data);
+	return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, NULL, 0, cbd->data);
+}
+
+static void at_stk_envelope(struct ofono_stk *stk, int length,
+				const guint8 *command,
+				ofono_stk_envelope_cb_t cb, void *data)
+{
+	struct stk_data *sd = ofono_stk_get_data(stk);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char *buf = g_try_new(char, 64 + length * 2);
+	int len, ret;
+
+	if (!cbd || !buf)
+		goto error;
+
+	len = sprintf(buf, "AT+CSIM=%i,A0C20000%02hhX",
+			12 + length * 2, length);
+
+	for (; length; length--)
+		len += sprintf(buf + len, "%02hhX", *command++);
+
+	len += sprintf(buf + len, "FF");
+
+	ret = g_at_chat_send(sd->chat, buf, csim_prefix,
+				at_csim_envelope_cb, cbd, g_free);
+
+	g_free(buf);
+	buf = NULL;
+
+	if (ret > 0)
+		return;
+
+error:
+	if (buf)
+		g_free(buf);
+
+	if (cbd)
+		g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, NULL, 0, data);
+}
+
+static void at_csim_terminal_response_cb(gboolean ok, GAtResult *result,
+		gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	GAtResultIter iter;
+	ofono_stk_generic_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	const guint8 *response;
+	gint rlen, len;
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok)
+		goto error;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CSIM:"))
+		goto error;
+
+	if (!g_at_result_iter_next_number(&iter, &rlen))
+		goto error;
+
+	if (!g_at_result_iter_next_hexstring(&iter, &response, &len))
+		goto error;
+
+	if (rlen != len * 2 || len < 2)
+		goto error;
+
+	if (response[len - 2] != 0x90 && response[len - 2] != 0x91)
+		goto error;
+
+	if (response[len - 2] == 0x90 && response[len - 1] != 0)
+		goto error;
+
+	DBG("csim_terminal_response_cb: %i", len);
+
+	cb(&error, cbd->data);
+	return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+}
+
+static void at_stk_terminal_response(struct ofono_stk *stk, int length,
+		const unsigned char *value, ofono_stk_generic_cb_t cb,
+		void *data)
+{
+	struct stk_data *sd = ofono_stk_get_data(stk);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char *buf = g_try_new(char, 64 + length * 2);
+	int len, ret;
+
+	if (!cbd || !buf)
+		goto error;
+
+	len = sprintf(buf, "AT+CSIM=%i,A0140000%02hhX",
+			10 + length * 2, length);
+
+	for (; length; length--)
+		len += sprintf(buf + len, "%02hhX", *value++);
+
+	ret = g_at_chat_send(sd->chat, buf, csim_prefix,
+				at_csim_terminal_response_cb, cbd, g_free);
+
+	g_free(buf);
+	buf = NULL;
+
+	if (ret > 0)
+		return;
+
+error:
+	if (cbd)
+		g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
+static gboolean at_stk_register(gpointer user)
+{
+	struct ofono_stk *stk = user;
+
+	ofono_stk_register(stk);
+
+	return FALSE;
+}
+
+static int at_stk_probe(struct ofono_stk *stk, unsigned int vendor,
+		void *data)
+{
+	GAtChat *chat = data;
+	struct stk_data *sd;
+
+	sd = g_new0(struct stk_data, 1);
+	sd->chat = chat;
+
+	ofono_stk_set_data(stk, sd);
+	g_idle_add(at_stk_register, stk);
+
+	return 0;
+}
+
+static void at_stk_remove(struct ofono_stk *stk)
+{
+	struct stk_data *sd = ofono_stk_get_data(stk);
+
+	ofono_stk_set_data(stk, NULL);
+
+	g_free(sd);
+}
+
+static struct ofono_stk_driver driver = {
+	.name			= "atmodem",
+	.probe			= at_stk_probe,
+	.remove			= at_stk_remove,
+	.envelope		= at_stk_envelope,
+	.terminal_response	= at_stk_terminal_response,
+};
+
+void at_stk_init()
+{
+	ofono_stk_driver_register(&driver);
+}
+
+void at_stk_exit()
+{
+	ofono_stk_driver_unregister(&driver);
+}
diff --git a/plugins/atgen.c b/plugins/atgen.c
index ed963c2..262d32f 100644
--- a/plugins/atgen.c
+++ b/plugins/atgen.c
@@ -43,6 +43,7 @@
 #include <ofono/netreg.h>
 #include <ofono/phonebook.h>
 #include <ofono/sim.h>
+#include <ofono/stk.h>
 #include <ofono/sms.h>
 #include <ofono/ssn.h>
 #include <ofono/ussd.h>
@@ -163,6 +164,7 @@ static void atgen_pre_sim(struct ofono_modem *modem)
 	ofono_devinfo_create(modem, 0, "atmodem", chat);
 	sim = ofono_sim_create(modem, 0, "atmodem", chat);
 	ofono_voicecall_create(modem, 0, "atmodem", chat);
+	ofono_stk_create(modem, 0, "atmodem", chat);
 
 	if (sim)
 		ofono_sim_inserted_notify(sim, TRUE);
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 4c3b3ce..9153e1b 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -50,6 +50,7 @@
 #include <ofono/netreg.h>
 #include <ofono/phonebook.h>
 #include <ofono/sim.h>
+#include <ofono/stk.h>
 #include <ofono/sms.h>
 #include <ofono/ssn.h>
 #include <ofono/ussd.h>
@@ -289,6 +290,8 @@ 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);
+
 	if (sim)
 		ofono_sim_inserted_notify(sim, TRUE);
 }
-- 
1.6.1

Re: [PATCH 7/8] Add AT driver for STK atom.

[Denis Kenzior]

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

Hi Andrew,

> ---
>  Makefile.am               |    1 +
>  drivers/atmodem/atmodem.c |    2 +
>  drivers/atmodem/atmodem.h |    3 +
>  drivers/atmodem/stk.c     |  258
>  +++++++++++++++++++++++++++++++++++++++++++++ plugins/atgen.c           | 
>    2 +
>  plugins/phonesim.c        |    3 +
>  6 files changed, 269 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/atmodem/stk.c

I did apply the patch, however I have a question:

The terminal response goes something like this:

> +
> +	len = sprintf(buf, "AT+CSIM=%i,A0140000%02hhX",
> +			10 + length * 2, length);
> +
> +	for (; length; length--)
> +		len += sprintf(buf + len, "%02hhX", *value++);
> +

In the new stk envelope code it goes something like this:

> 
> +	len = sprintf(buf, "AT+CSIM=%i,A0C20000%02hhX",
> +			12 + length * 2, length);
> +
> +	for (; length; length--)
> +		len += sprintf(buf + len, "%02hhX", *command++);
> +
> +	len += sprintf(buf + len, "FF");

What is the point of this last 'FF'?  The code removed from the sim atom 
driver doesn't have it either...

Regards,
-Denis

Re: [PATCH 7/8] Add AT driver for STK atom.

[Andrzej Zaborowski]

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

Hi Denis,

On 15 April 2010 23:48, Denis Kenzior <denkenz@gmail.com> wrote:
> The terminal response goes something like this:
>
>> +
>> +     len = sprintf(buf, "AT+CSIM=%i,A0140000%02hhX",
>> +                     10 + length * 2, length);
>> +
>> +     for (; length; length--)
>> +             len += sprintf(buf + len, "%02hhX", *value++);
>> +
>
> In the new stk envelope code it goes something like this:
>
>>
>> +     len = sprintf(buf, "AT+CSIM=%i,A0C20000%02hhX",
>> +                     12 + length * 2, length);
>> +
>> +     for (; length; length--)
>> +             len += sprintf(buf + len, "%02hhX", *command++);
>> +
>> +     len += sprintf(buf + len, "FF");
>
> What is the point of this last 'FF'?  The code removed from the sim atom
> driver doesn't have it either...

Rereading the etsi ts102.221 the "Le" field in the Envelope APDU
(11.2.2.2) is not explictly defined "not present", instead it says
"empty or maximum length of expected data".  It doesn't say what the
default value is when empty so I conclude it's safer to have the field
present or the card may think it's not allowed to send any response at
all -- depending on the implementer's interpretation of the spec.

10.1.6 says the field is the maximum respose length expected, but then
some commands suggest it should be the exact length wanted and some
make the field obligatory.

Regards

Re: [PATCH 7/8] Add AT driver for STK atom.

[Denis Kenzior]

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

Hi Andrew,

> Hi Denis,
> 
> On 15 April 2010 23:48, Denis Kenzior <denkenz@gmail.com> wrote:
> > The terminal response goes something like this:
> >> +
> >> +     len = sprintf(buf, "AT+CSIM=%i,A0140000%02hhX",
> >> +                     10 + length * 2, length);
> >> +
> >> +     for (; length; length--)
> >> +             len += sprintf(buf + len, "%02hhX", *value++);
> >> +
> >
> > In the new stk envelope code it goes something like this:
> >> +     len = sprintf(buf, "AT+CSIM=%i,A0C20000%02hhX",
> >> +                     12 + length * 2, length);
> >> +
> >> +     for (; length; length--)
> >> +             len += sprintf(buf + len, "%02hhX", *command++);
> >> +
> >> +     len += sprintf(buf + len, "FF");
> >
> > What is the point of this last 'FF'?  The code removed from the sim atom
> > driver doesn't have it either...
> 
> Rereading the etsi ts102.221 the "Le" field in the Envelope APDU
> (11.2.2.2) is not explictly defined "not present", instead it says
> "empty or maximum length of expected data".  It doesn't say what the
> default value is when empty so I conclude it's safer to have the field
> present or the card may think it's not allowed to send any response at
> all -- depending on the implementer's interpretation of the spec.
> 
> 10.1.6 says the field is the maximum respose length expected, but then
> some commands suggest it should be the exact length wanted and some
> make the field obligatory.

So I re-read that part of the spec, and I can see why we might want it.  
However, this quote seemed rather interesting: "Le set to '00' indicates that 
the terminal expects to receive at most the maximum number of bytes, i.e. 256, 
in the response ADPU. The UICC may return any number of bytes in the range 1 
to 256."

Shouldn't that 'FF' be changed to '00'?  Also, you might want to include the 
relevant passage or Spec/Section reference in this code so we don't wonder 
where it came from in the future.

Regards,
-Denis

Re: [PATCH 7/8] Add AT driver for STK atom.

[Andrzej Zaborowski]

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

Hi,

On 20 April 2010 22:21, Denis Kenzior <denkenz@gmail.com> wrote:
>> Rereading the etsi ts102.221 the "Le" field in the Envelope APDU
>> (11.2.2.2) is not explictly defined "not present", instead it says
>> "empty or maximum length of expected data".  It doesn't say what the
>> default value is when empty so I conclude it's safer to have the field
>> present or the card may think it's not allowed to send any response at
>> all -- depending on the implementer's interpretation of the spec.
>>
>> 10.1.6 says the field is the maximum respose length expected, but then
>> some commands suggest it should be the exact length wanted and some
>> make the field obligatory.
>
> So I re-read that part of the spec, and I can see why we might want it.
> However, this quote seemed rather interesting: "Le set to '00' indicates that
> the terminal expects to receive at most the maximum number of bytes, i.e. 256,
> in the response ADPU. The UICC may return any number of bytes in the range 1
> to 256."
>
> Shouldn't that 'FF' be changed to '00'?  Also, you might want to include the
> relevant passage or Spec/Section reference in this code so we don't wonder
> where it came from in the future.

Attached patch adds a comment and changes the FF to 00.  I guess it is
largely theoretical right now.  The issue again with "any number of
bytes in the range 1 to 256" is that this prohibits an empty response,
which we want to allow too :)   Maybe this should be a parameter of
driver->envelope()

Best regards

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0012-Comment-on-the-Le-field-in-ENVELOPE-apdu.patch --]
[-- Type: text/x-patch, Size: 1444 bytes --]

From 78cd3b4882647a6c42f53572de1423d11464e342 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Tue, 20 Apr 2010 09:43:40 +0200
Subject: Comment on the Le field in ENVELOPE apdu.

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

diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
index 8cff4a2..cc02a8e 100644
--- a/drivers/atmodem/stk.c
+++ b/drivers/atmodem/stk.c
@@ -107,7 +107,19 @@ static void at_stk_envelope(struct ofono_stk *stk, int length,
 	for (; length; length--)
 		len += sprintf(buf + len, "%02hhX", *command++);
 
-	len += sprintf(buf + len, "FF");
+	/* We append the "Le" field.  ETSI ts102.221 is not very clear
+	 * on whether it's mandatory, so stay on the safe side.
+	 * Relevant sections:
+	 * 10.1.6 defines "Le" as optional, doesn't say whether absence
+	 * means no answer is wanted or only that it can have any
+	 * length.  If present the response data should consist of Le bytes.
+	 * Then suggests "00" allows any length between 1 and 256 bytes.
+	 * 11.2.2.2 says "Le" can be "empty" but again doesn't say what
+	 * value is assumed in this case.  If not "empty" the response
+	 * should consist of Le or fewer bytes.  This suggests we should
+	 * use "FF" as the value.
+	 */
+	len += sprintf(buf + len, "00");
 
 	ret = g_at_chat_send(sd->chat, buf, csim_prefix,
 				at_csim_envelope_cb, cbd, g_free);
-- 
1.6.1

Re: [PATCH 7/8] Add AT driver for STK atom.

[Denis Kenzior]

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

Hi Andrew,

> > So I re-read that part of the spec, and I can see why we might want it.
> > However, this quote seemed rather interesting: "Le set to '00' indicates
> > that the terminal expects to receive at most the maximum number of bytes,
> > i.e. 256, in the response ADPU. The UICC may return any number of bytes
> > in the range 1 to 256."
> >
> > Shouldn't that 'FF' be changed to '00'?  Also, you might want to include
> > the relevant passage or Spec/Section reference in this code so we don't
> > wonder where it came from in the future.
> 
> Attached patch adds a comment and changes the FF to 00.  I guess it is
> largely theoretical right now.  The issue again with "any number of
> bytes in the range 1 to 256" is that this prohibits an empty response,
> which we want to allow too :)   Maybe this should be a parameter of
> driver->envelope()

"The maximum number of bytes expected in the data part of the response APDU is 
presented in the parameter Le, which is optional. This means that if the 
terminal does not expect any data in the response APDU Le is absent from the 
command"

The question really becomes, do we actually want a response and know what to 
do with it?

Regards,
-Denis

Re: [PATCH 7/8] Add AT driver for STK atom.

[andrzej zaborowski]

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

On 20 April 2010 22:55, Denis Kenzior <denkenz@gmail.com> wrote:
>> > So I re-read that part of the spec, and I can see why we might want it.
>> > However, this quote seemed rather interesting: "Le set to '00' indicates
>> > that the terminal expects to receive at most the maximum number of bytes,
>> > i.e. 256, in the response ADPU. The UICC may return any number of bytes
>> > in the range 1 to 256."
>> >
>> > Shouldn't that 'FF' be changed to '00'?  Also, you might want to include
>> > the relevant passage or Spec/Section reference in this code so we don't
>> > wonder where it came from in the future.
>>
>> Attached patch adds a comment and changes the FF to 00.  I guess it is
>> largely theoretical right now.  The issue again with "any number of
>> bytes in the range 1 to 256" is that this prohibits an empty response,
>> which we want to allow too :)   Maybe this should be a parameter of
>> driver->envelope()
>
> "The maximum number of bytes expected in the data part of the response APDU is
> presented in the parameter Le, which is optional. This means that if the
> terminal does not expect any data in the response APDU Le is absent from the
> command"

Well, this is an "if" without "only then" so it doesn't tell us the
meaning of Le being absent.  Additionally the whole passage is later
invalidated in 11.2.2.2, so the SIM implementer really has all the
possible options.

>
> The question really becomes, do we actually want a response and know what to
> do with it?

We only use envelope in CBS data download right now which expects no
response, we could indicate that in a parameter.

Best regards

Re: [PATCH 7/8] Add AT driver for STK atom.

[Denis Kenzior]

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

Hi Andrew,

> Well, this is an "if" without "only then" so it doesn't tell us the
> meaning of Le being absent.  Additionally the whole passage is later
> invalidated in 11.2.2.2, so the SIM implementer really has all the
> possible options.

Hmm, I don't interpret it this way.  Le being absent means no response 
expected.  The '00' vs 'FF' is unclear, I agree there.

> 
> > The question really becomes, do we actually want a response and know what
> > to do with it?
> 
> We only use envelope in CBS data download right now which expects no
> response, we could indicate that in a parameter.

I suggest that we mention the Le confusion in the comment block and take out 
Le altogether since we can't do anything useful with the response data yet.  
Once we're ready to implement this properly, then we can add Le and figure out 
wheter 'FF' or '00' or something else is required.

For a hint of what the response can be used for, see 3GPP 31.124.  It seems 
the response from the SIM has to be set into an RP-ERROR message and given 
back to the network...

Regards,
-Denis