[PATCH 0/3] Patch Description

[PATCH 0/3] Patch Description

[Yang Gu]

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

This series of patch is to add provide local info support by requesting the terminal to send time and language info. Please comment on the following aspects as I'm not sure after reading the spec:
1. Timezone may be a number in the range -47 through +48. In struct sms_scts, timezone is defined as gint8, thus 0xFF should shand for -1, which is a valid input. Thus I think build_dataobj_datetime_timezone() in src/stkutil.c is not correct. But I'm still not sure what value should be passed to oFono when timezone is absent. 
2. DBUS_TYPE_BYTE represents an 8-bit unsigned integer, and D-Bus doesn't have a type related to 8-bit signed integer. So what's the best way to represent a timezone? 
3. Only one byte is used to represent the year. Is the following logic correct to get the year with one byte?
if (year_dbus >= 2000)
	year = year_dbus - 2000;
else
	year = year_dbus - 1900; 

Yang Gu (3):
  network: Use bit as size instead of byte
  stk: Handle provide local info proactive command
  test-stk: Add provide local info

 src/network.c      |    4 +-
 src/smsutil.c      |    6 +-
 src/stk.c          |  113 ++++++++++++++++++++++++++++++++++++++
 src/stkagent.c     |  153 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkagent.h     |   14 +++++
 src/stkutil.c      |    2 +-
 test/test-stk-menu |   36 ++++++++++++
 7 files changed, 322 insertions(+), 6 deletions(-)

-- 
1.7.2.3

[PATCH 1/3] network: Use bit as size instead of byte

[Yang Gu]

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

---
 src/network.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/network.c b/src/network.c
index d203bee..83014bb 100644
--- a/src/network.c
+++ b/src/network.c
@@ -129,7 +129,7 @@ static char **network_operator_technologies(struct network_operator_data *opd)
 	char **techs;
 	unsigned int i;
 
-	for (i = 0; i < sizeof(opd->techs); i++) {
+	for (i = 0; i < sizeof(opd->techs) * 8; i++) {
 		if (opd->techs & (1 << i))
 			ntechs += 1;
 	}
@@ -137,7 +137,7 @@ static char **network_operator_technologies(struct network_operator_data *opd)
 	techs = g_new0(char *, ntechs + 1);
 	ntechs = 0;
 
-	for (i = 0; i < sizeof(opd->techs); i++) {
+	for (i = 0; i < sizeof(opd->techs) * 8; i++) {
 		if (!(opd->techs & (1 << i)))
 			continue;
 
-- 
1.7.2.3

Re: [PATCH 1/3] network: Use bit as size instead of byte

[Denis Kenzior]

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

Hi Yang,

On 11/25/2010 06:29 AM, Yang Gu wrote:
> ---
>  src/network.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 

Good catch! Patch has been applied, thanks.

Regards,
-Denis

[PATCH 2/3] stk: Handle provide local info proactive command

[Yang Gu]

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

---
 src/smsutil.c  |    6 +-
 src/stk.c      |  113 +++++++++++++++++++++++++++++++++++++++++
 src/stkagent.c |  153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkagent.h |   14 +++++
 src/stkutil.c  |    2 +-
 5 files changed, 284 insertions(+), 4 deletions(-)

diff --git a/src/smsutil.c b/src/smsutil.c
index e6dbf5f..5394817 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -324,10 +324,10 @@ gboolean sms_encode_scts(const struct sms_scts *in, unsigned char *pdu,
 	if (in->year > 99)
 		return FALSE;
 
-	if (in->month > 12)
+	if (in->month > 12 || in->month == 0)
 		return FALSE;
 
-	if (in->day > 31)
+	if (in->day > 31 || in->day == 0)
 		return FALSE;
 
 	if (in->hour > 23)
@@ -339,7 +339,7 @@ gboolean sms_encode_scts(const struct sms_scts *in, unsigned char *pdu,
 	if (in->second > 59)
 		return FALSE;
 
-	if ((in->timezone > 12*4-1) || (in->timezone < -(12*4-1)))
+	if ((in->timezone > 12*4) || (in->timezone < -(12*4-1)))
 		return FALSE;
 
 	pdu = pdu + *offset;
diff --git a/src/stk.c b/src/stk.c
index ac2e646..1f3817f 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1992,6 +1992,114 @@ static gboolean handle_command_refresh(const struct stk_command *cmd,
 	return TRUE;
 }
 
+static void request_time_cb(enum stk_agent_result result, unsigned char year,
+				unsigned char month, unsigned char day,
+				unsigned char hour, unsigned char minute,
+				unsigned char second, char timezone,
+				void *user_data)
+{
+	struct ofono_stk *stk = user_data;
+	static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
+	struct stk_response rsp;
+
+	stk->respond_on_exit = FALSE;
+
+	switch (result) {
+	case STK_AGENT_RESULT_OK:
+		memset(&rsp, 0, sizeof(rsp));
+
+		rsp.result.type = STK_RESULT_TYPE_SUCCESS;
+		rsp.provide_local_info.datetime.year = year;
+		rsp.provide_local_info.datetime.month = month;
+		rsp.provide_local_info.datetime.day = day;
+		rsp.provide_local_info.datetime.hour = hour;
+		rsp.provide_local_info.datetime.minute = minute;
+		rsp.provide_local_info.datetime.second = second;
+		rsp.provide_local_info.datetime.timezone = timezone;
+
+		if (stk_respond(stk, &rsp, stk_command_cb))
+			stk_command_cb(&error, stk);
+
+		break;
+
+	case STK_AGENT_RESULT_BACK:
+		send_simple_response(stk, STK_RESULT_TYPE_GO_BACK);
+		break;
+
+	case STK_AGENT_RESULT_TIMEOUT:
+		send_simple_response(stk, STK_RESULT_TYPE_NO_RESPONSE);
+		break;
+
+	case STK_AGENT_RESULT_TERMINATE:
+		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
+		break;
+	}
+}
+
+static void request_lang_cb(enum stk_agent_result result, char *lang,
+				void *user_data)
+{
+	struct ofono_stk *stk = user_data;
+	static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
+	struct stk_response rsp;
+
+	stk->respond_on_exit = FALSE;
+
+	switch (result) {
+	case STK_AGENT_RESULT_OK:
+		memset(&rsp, 0, sizeof(rsp));
+
+		rsp.result.type = STK_RESULT_TYPE_SUCCESS;
+		rsp.provide_local_info.language = lang;
+
+		if (stk_respond(stk, &rsp, stk_command_cb))
+			stk_command_cb(&error, stk);
+
+		break;
+
+	case STK_AGENT_RESULT_BACK:
+		send_simple_response(stk, STK_RESULT_TYPE_GO_BACK);
+		break;
+
+	case STK_AGENT_RESULT_TIMEOUT:
+		send_simple_response(stk, STK_RESULT_TYPE_NO_RESPONSE);
+		break;
+
+	case STK_AGENT_RESULT_TERMINATE:
+		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
+		break;
+	}
+}
+
+static gboolean handle_command_provide_local_info(const struct stk_command *cmd,
+				struct stk_response *rsp, struct ofono_stk *stk)
+{
+	int timeout = stk->timeout * 1000;
+	int err;
+
+	switch (cmd->qualifier) {
+	case 3:
+		DBG("Date, time and time zone");
+		err = stk_agent_request_time(stk->current_agent,
+						request_time_cb,
+						stk, NULL, timeout);
+		return FALSE;
+
+	case 4:
+		DBG("Language setting");
+		err = stk_agent_request_lang(stk->current_agent,
+						request_lang_cb,
+						stk, NULL, timeout);
+		return FALSE;
+
+	default:
+		ofono_info("Unsupported Provide Local Info qualifier: %d",
+				cmd->qualifier);
+		rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+		return TRUE;
+	}
+}
+
 static void send_dtmf_cancel(struct ofono_stk *stk)
 {
 	struct ofono_voicecall *vc = NULL;
@@ -2424,6 +2532,11 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 							&rsp, stk);
 		break;
 
+	case STK_COMMAND_TYPE_PROVIDE_LOCAL_INFO:
+		respond = handle_command_provide_local_info(stk->pending_cmd,
+								&rsp, stk);
+		break;
+
 	case STK_COMMAND_TYPE_SEND_DTMF:
 		respond = handle_command_send_dtmf(stk->pending_cmd,
 							&rsp, stk);
diff --git a/src/stkagent.c b/src/stkagent.c
index 5cf83e4..78620d5 100644
--- a/src/stkagent.c
+++ b/src/stkagent.c
@@ -956,3 +956,156 @@ int stk_agent_loop_tone(struct stk_agent *agent, const char *text,
 
 	return 0;
 }
+
+static void get_time_cb(DBusPendingCall *call, void *data)
+{
+	struct stk_agent *agent = data;
+	stk_agent_time_cb cb = agent->user_cb;
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+	enum stk_agent_result result;
+	gboolean remove_agent;
+	int year_dbus;
+	unsigned char year;
+	unsigned char month;
+	unsigned char day;
+	unsigned char hour;
+	unsigned char minute;
+	unsigned char second;
+	int timezone_dbus;
+	char timezone;
+
+	if (check_error(agent, reply,
+			ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE,
+			&result) == -EINVAL) {
+		remove_agent = TRUE;
+		goto error;
+	}
+
+	if (result != STK_AGENT_RESULT_OK) {
+		cb(result, 0, 0, 0, 0, 0, 0, 0xFF, agent->user_data);
+		goto done;
+	}
+
+	if (dbus_message_get_args(reply, NULL,
+					DBUS_TYPE_INT32, &year_dbus,
+					DBUS_TYPE_BYTE, &month,
+					DBUS_TYPE_BYTE, &day,
+					DBUS_TYPE_BYTE, &hour,
+					DBUS_TYPE_BYTE, &minute,
+					DBUS_TYPE_BYTE, &second,
+					DBUS_TYPE_INT32, &timezone_dbus,
+					DBUS_TYPE_INVALID) == FALSE) {
+		ofono_error("Can't parse the reply to RequestTime()");
+		remove_agent = TRUE;
+		goto error;
+	}
+
+	if (year_dbus < 1900) {
+		ofono_error("Invalid year");
+		remove_agent = TRUE;
+		goto error;
+	}
+
+	if (year_dbus >= 2000)
+		year = year_dbus - 2000;
+	else
+		year = year_dbus - 1900;
+
+	timezone = timezone_dbus;
+
+	cb(result, year, month, day, hour, minute, second, timezone,
+							agent->user_data);
+
+	CALLBACK_END();
+}
+
+int stk_agent_request_time(struct stk_agent *agent, stk_agent_time_cb cb,
+				void *user_data, ofono_destroy_func destroy,
+				int timeout)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	agent->msg = dbus_message_new_method_call(agent->bus, agent->path,
+							OFONO_SIM_APP_INTERFACE,
+							"RequestTime");
+	if (agent->msg == NULL)
+		return -ENOMEM;
+
+	dbus_message_append_args(agent->msg, DBUS_TYPE_INVALID);
+
+	if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
+						timeout) == FALSE ||
+			agent->call == NULL)
+		return -EIO;
+
+	agent->user_cb = cb;
+	agent->user_data = user_data;
+	agent->user_destroy = destroy;
+
+	dbus_pending_call_set_notify(agent->call, get_time_cb, agent, NULL);
+
+	return 0;
+}
+
+static void get_lang_cb(DBusPendingCall *call, void *data)
+{
+	struct stk_agent *agent = data;
+	stk_agent_string_cb cb = agent->user_cb;
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+	enum stk_agent_result result;
+	gboolean remove_agent;
+	char *lang;
+
+	if (check_error(agent, reply,
+			ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE,
+			&result) == -EINVAL) {
+		remove_agent = TRUE;
+		goto error;
+	}
+
+	if (result != STK_AGENT_RESULT_OK) {
+		cb(result, NULL, agent->user_data);
+		goto done;
+	}
+
+	if (dbus_message_get_args(reply, NULL,
+					DBUS_TYPE_STRING, &lang,
+					DBUS_TYPE_INVALID) == FALSE ||
+			strlen(lang) != 2) {
+		ofono_error("Can't parse the reply to RequestLanguage()");
+		remove_agent = TRUE;
+		goto error;
+	}
+
+	cb(result, lang, agent->user_data);
+
+	CALLBACK_END();
+}
+
+int stk_agent_request_lang(struct stk_agent *agent, stk_agent_string_cb cb,
+				void *user_data, ofono_destroy_func destroy,
+				int timeout)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	agent->msg = dbus_message_new_method_call(agent->bus, agent->path,
+							OFONO_SIM_APP_INTERFACE,
+							"RequestLanguage");
+	if (agent->msg == NULL)
+		return -ENOMEM;
+
+	dbus_message_append_args(agent->msg, DBUS_TYPE_INVALID);
+
+	if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
+						timeout) == FALSE ||
+			agent->call == NULL)
+		return -EIO;
+
+	agent->user_cb = cb;
+	agent->user_data = user_data;
+	agent->user_destroy = destroy;
+
+	dbus_pending_call_set_notify(agent->call, get_lang_cb, agent, NULL);
+
+	return 0;
+}
diff --git a/src/stkagent.h b/src/stkagent.h
index c8e1886..a60cf23 100644
--- a/src/stkagent.h
+++ b/src/stkagent.h
@@ -59,6 +59,12 @@ typedef void (*stk_agent_string_cb)(enum stk_agent_result result,
 typedef void (*stk_agent_tone_cb)(enum stk_agent_result result,
 						void *user_data);
 
+typedef void (*stk_agent_time_cb)(enum stk_agent_result result,
+				unsigned char year, unsigned char month,
+				unsigned char day, unsigned char hour,
+				unsigned char minute, unsigned char second,
+				char timezone, void *user_data);
+
 struct stk_agent *stk_agent_new(const char *path, const char *sender,
 					ofono_bool_t remove_on_terminate);
 
@@ -136,3 +142,11 @@ int stk_agent_loop_tone(struct stk_agent *agent, const char *text,
 
 void append_menu_items_variant(DBusMessageIter *iter,
 				const struct stk_menu_item *items);
+
+int stk_agent_request_time(struct stk_agent *agent, stk_agent_time_cb cb,
+			void *user_data, ofono_destroy_func destroy,
+			int timeout);
+
+int stk_agent_request_lang(struct stk_agent *agent, stk_agent_string_cb cb,
+			void *user_data, ofono_destroy_func destroy,
+			int timeout);
diff --git a/src/stkutil.c b/src/stkutil.c
index 377ceff..48ce93b 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -4553,7 +4553,7 @@ static gboolean build_dataobj_datetime_timezone(struct stk_tlv_builder *tlv,
 		return TRUE;
 
 	/* Time zone information is optional */
-	if (scts->timezone == (gint8) 0xff) {
+	if (scts->timezone == -48) {
 		memcpy(&timestamp, scts, sizeof(timestamp));
 		timestamp.timezone = 0;
 		if (sms_encode_scts(&timestamp, value, &offset) != TRUE)
-- 
1.7.2.3

Re: [PATCH 2/3] stk: Handle provide local info proactive command

[andrzej zaborowski]

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

On 25 November 2010 13:29, Yang Gu <yang.gu@intel.com> wrote:
> ---
>  src/smsutil.c  |    6 +-
>  src/stk.c      |  113 +++++++++++++++++++++++++++++++++++++++++
>  src/stkagent.c |  153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/stkagent.h |   14 +++++
>  src/stkutil.c  |    2 +-
>  5 files changed, 284 insertions(+), 4 deletions(-)
>
> diff --git a/src/smsutil.c b/src/smsutil.c
> index e6dbf5f..5394817 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -324,10 +324,10 @@ gboolean sms_encode_scts(const struct sms_scts *in, unsigned char *pdu,
>        if (in->year > 99)
>                return FALSE;
>
> -       if (in->month > 12)
> +       if (in->month > 12 || in->month == 0)
>                return FALSE;
>
> -       if (in->day > 31)
> +       if (in->day > 31 || in->day == 0)
>                return FALSE;
>
>        if (in->hour > 23)
> @@ -339,7 +339,7 @@ gboolean sms_encode_scts(const struct sms_scts *in, unsigned char *pdu,
>        if (in->second > 59)
>                return FALSE;
>
> -       if ((in->timezone > 12*4-1) || (in->timezone < -(12*4-1)))
> +       if ((in->timezone > 12*4) || (in->timezone < -(12*4-1)))

Please ignore my comment about the range in the other, because you
already fixed this :)

>                return FALSE;
>
>        pdu = pdu + *offset;
> diff --git a/src/stk.c b/src/stk.c
> index ac2e646..1f3817f 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -1992,6 +1992,114 @@ static gboolean handle_command_refresh(const struct stk_command *cmd,
>        return TRUE;
>  }
>
> +static void request_time_cb(enum stk_agent_result result, unsigned char year,
> +                               unsigned char month, unsigned char day,
> +                               unsigned char hour, unsigned char minute,
> +                               unsigned char second, char timezone,
> +                               void *user_data)
> +{
> +       struct ofono_stk *stk = user_data;
> +       static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
> +       struct stk_response rsp;
> +
> +       stk->respond_on_exit = FALSE;
> +
> +       switch (result) {
> +       case STK_AGENT_RESULT_OK:
> +               memset(&rsp, 0, sizeof(rsp));
> +
> +               rsp.result.type = STK_RESULT_TYPE_SUCCESS;
> +               rsp.provide_local_info.datetime.year = year;
> +               rsp.provide_local_info.datetime.month = month;
> +               rsp.provide_local_info.datetime.day = day;
> +               rsp.provide_local_info.datetime.hour = hour;
> +               rsp.provide_local_info.datetime.minute = minute;
> +               rsp.provide_local_info.datetime.second = second;
> +               rsp.provide_local_info.datetime.timezone = timezone;
> +
> +               if (stk_respond(stk, &rsp, stk_command_cb))
> +                       stk_command_cb(&error, stk);
> +
> +               break;
> +
> +       case STK_AGENT_RESULT_BACK:
> +               send_simple_response(stk, STK_RESULT_TYPE_GO_BACK);
> +               break;

Note that "Backward move", "No reponse" and "User terminated" are not
allowed responses for Provide Local Info in TS102.223 6.11.

> +
> +       case STK_AGENT_RESULT_TIMEOUT:
> +               send_simple_response(stk, STK_RESULT_TYPE_NO_RESPONSE);
> +               break;
> +
> +       case STK_AGENT_RESULT_TERMINATE:
> +               send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
> +               break;
> +       }
> +}
> +
> +static void request_lang_cb(enum stk_agent_result result, char *lang,
> +                               void *user_data)
> +{
> +       struct ofono_stk *stk = user_data;
> +       static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
> +       struct stk_response rsp;
> +
> +       stk->respond_on_exit = FALSE;
> +
> +       switch (result) {
> +       case STK_AGENT_RESULT_OK:
> +               memset(&rsp, 0, sizeof(rsp));
> +
> +               rsp.result.type = STK_RESULT_TYPE_SUCCESS;
> +               rsp.provide_local_info.language = lang;
> +
> +               if (stk_respond(stk, &rsp, stk_command_cb))
> +                       stk_command_cb(&error, stk);
> +
> +               break;
> +
> +       case STK_AGENT_RESULT_BACK:
> +               send_simple_response(stk, STK_RESULT_TYPE_GO_BACK);
> +               break;
> +
> +       case STK_AGENT_RESULT_TIMEOUT:
> +               send_simple_response(stk, STK_RESULT_TYPE_NO_RESPONSE);
> +               break;
> +
> +       case STK_AGENT_RESULT_TERMINATE:
> +               send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
> +               break;
> +       }
> +}
> +
> +static gboolean handle_command_provide_local_info(const struct stk_command *cmd,
> +                               struct stk_response *rsp, struct ofono_stk *stk)
> +{
> +       int timeout = stk->timeout * 1000;
> +       int err;
> +
> +       switch (cmd->qualifier) {
> +       case 3:
> +               DBG("Date, time and time zone");
> +               err = stk_agent_request_time(stk->current_agent,
> +                                               request_time_cb,
> +                                               stk, NULL, timeout);
> +               return FALSE;
> +
> +       case 4:
> +               DBG("Language setting");
> +               err = stk_agent_request_lang(stk->current_agent,
> +                                               request_lang_cb,
> +                                               stk, NULL, timeout);
> +               return FALSE;
> +
> +       default:
> +               ofono_info("Unsupported Provide Local Info qualifier: %d",
> +                               cmd->qualifier);
> +               rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
> +               return TRUE;
> +       }
> +}
> +
>  static void send_dtmf_cancel(struct ofono_stk *stk)
>  {
>        struct ofono_voicecall *vc = NULL;
> @@ -2424,6 +2532,11 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
>                                                        &rsp, stk);
>                break;
>
> +       case STK_COMMAND_TYPE_PROVIDE_LOCAL_INFO:
> +               respond = handle_command_provide_local_info(stk->pending_cmd,
> +                                                               &rsp, stk);
> +               break;
> +
>        case STK_COMMAND_TYPE_SEND_DTMF:
>                respond = handle_command_send_dtmf(stk->pending_cmd,
>                                                        &rsp, stk);
> diff --git a/src/stkagent.c b/src/stkagent.c
> index 5cf83e4..78620d5 100644
> --- a/src/stkagent.c
> +++ b/src/stkagent.c
> @@ -956,3 +956,156 @@ int stk_agent_loop_tone(struct stk_agent *agent, const char *text,
>
>        return 0;
>  }
> +
> +static void get_time_cb(DBusPendingCall *call, void *data)
> +{
> +       struct stk_agent *agent = data;
> +       stk_agent_time_cb cb = agent->user_cb;
> +       DBusMessage *reply = dbus_pending_call_steal_reply(call);
> +       enum stk_agent_result result;
> +       gboolean remove_agent;
> +       int year_dbus;
> +       unsigned char year;
> +       unsigned char month;
> +       unsigned char day;
> +       unsigned char hour;
> +       unsigned char minute;
> +       unsigned char second;
> +       int timezone_dbus;
> +       char timezone;

I think you need to use "signed char" to achieve the same type as
int8_t/guin8 because the sign of "char" depends on the ABI.

> +
> +       if (check_error(agent, reply,
> +                       ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE,
> +                       &result) == -EINVAL) {

Go back and Terminate are not allowed for this command. (same comment as above)

> +               remove_agent = TRUE;
> +               goto error;
> +       }
> +
> +       if (result != STK_AGENT_RESULT_OK) {
> +               cb(result, 0, 0, 0, 0, 0, 0, 0xFF, agent->user_data);
> +               goto done;
> +       }
> +
> +       if (dbus_message_get_args(reply, NULL,
> +                                       DBUS_TYPE_INT32, &year_dbus,
> +                                       DBUS_TYPE_BYTE, &month,
> +                                       DBUS_TYPE_BYTE, &day,
> +                                       DBUS_TYPE_BYTE, &hour,
> +                                       DBUS_TYPE_BYTE, &minute,
> +                                       DBUS_TYPE_BYTE, &second,
> +                                       DBUS_TYPE_INT32, &timezone_dbus,
> +                                       DBUS_TYPE_INVALID) == FALSE) {
> +               ofono_error("Can't parse the reply to RequestTime()");
> +               remove_agent = TRUE;
> +               goto error;
> +       }
> +
> +       if (year_dbus < 1900) {
> +               ofono_error("Invalid year");
> +               remove_agent = TRUE;
> +               goto error;
> +       }

I believe the range is actually 1970-2069 or similar (only values 0-99
in the sms header are allowed)

> +
> +       if (year_dbus >= 2000)
> +               year = year_dbus - 2000;
> +       else
> +               year = year_dbus - 1900;
> +
> +       timezone = timezone_dbus;
> +
> +       cb(result, year, month, day, hour, minute, second, timezone,
> +                                                       agent->user_data);
> +
> +       CALLBACK_END();
> +}
> +
> +int stk_agent_request_time(struct stk_agent *agent, stk_agent_time_cb cb,
> +                               void *user_data, ofono_destroy_func destroy,
> +                               int timeout)
> +{
> +       DBusConnection *conn = ofono_dbus_get_connection();
> +
> +       agent->msg = dbus_message_new_method_call(agent->bus, agent->path,
> +                                                       OFONO_SIM_APP_INTERFACE,
> +                                                       "RequestTime");
> +       if (agent->msg == NULL)
> +               return -ENOMEM;
> +
> +       dbus_message_append_args(agent->msg, DBUS_TYPE_INVALID);
> +
> +       if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
> +                                               timeout) == FALSE ||
> +                       agent->call == NULL)
> +               return -EIO;
> +
> +       agent->user_cb = cb;
> +       agent->user_data = user_data;
> +       agent->user_destroy = destroy;
> +
> +       dbus_pending_call_set_notify(agent->call, get_time_cb, agent, NULL);
> +
> +       return 0;
> +}
> +
> +static void get_lang_cb(DBusPendingCall *call, void *data)
> +{
> +       struct stk_agent *agent = data;
> +       stk_agent_string_cb cb = agent->user_cb;
> +       DBusMessage *reply = dbus_pending_call_steal_reply(call);
> +       enum stk_agent_result result;
> +       gboolean remove_agent;
> +       char *lang;
> +
> +       if (check_error(agent, reply,
> +                       ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE,
> +                       &result) == -EINVAL) {
> +               remove_agent = TRUE;
> +               goto error;
> +       }
> +
> +       if (result != STK_AGENT_RESULT_OK) {
> +               cb(result, NULL, agent->user_data);
> +               goto done;
> +       }
> +
> +       if (dbus_message_get_args(reply, NULL,
> +                                       DBUS_TYPE_STRING, &lang,
> +                                       DBUS_TYPE_INVALID) == FALSE ||
> +                       strlen(lang) != 2) {
> +               ofono_error("Can't parse the reply to RequestLanguage()");
> +               remove_agent = TRUE;
> +               goto error;
> +       }
> +
> +       cb(result, lang, agent->user_data);
> +
> +       CALLBACK_END();
> +}
> +
> +int stk_agent_request_lang(struct stk_agent *agent, stk_agent_string_cb cb,
> +                               void *user_data, ofono_destroy_func destroy,
> +                               int timeout)
> +{
> +       DBusConnection *conn = ofono_dbus_get_connection();
> +
> +       agent->msg = dbus_message_new_method_call(agent->bus, agent->path,
> +                                                       OFONO_SIM_APP_INTERFACE,
> +                                                       "RequestLanguage");
> +       if (agent->msg == NULL)
> +               return -ENOMEM;
> +
> +       dbus_message_append_args(agent->msg, DBUS_TYPE_INVALID);
> +
> +       if (dbus_connection_send_with_reply(conn, agent->msg, &agent->call,
> +                                               timeout) == FALSE ||
> +                       agent->call == NULL)
> +               return -EIO;
> +
> +       agent->user_cb = cb;
> +       agent->user_data = user_data;
> +       agent->user_destroy = destroy;
> +
> +       dbus_pending_call_set_notify(agent->call, get_lang_cb, agent, NULL);
> +
> +       return 0;
> +}
> diff --git a/src/stkagent.h b/src/stkagent.h
> index c8e1886..a60cf23 100644
> --- a/src/stkagent.h
> +++ b/src/stkagent.h
> @@ -59,6 +59,12 @@ typedef void (*stk_agent_string_cb)(enum stk_agent_result result,
>  typedef void (*stk_agent_tone_cb)(enum stk_agent_result result,
>                                                void *user_data);
>
> +typedef void (*stk_agent_time_cb)(enum stk_agent_result result,
> +                               unsigned char year, unsigned char month,
> +                               unsigned char day, unsigned char hour,
> +                               unsigned char minute, unsigned char second,
> +                               char timezone, void *user_data);
> +
>  struct stk_agent *stk_agent_new(const char *path, const char *sender,
>                                        ofono_bool_t remove_on_terminate);
>
> @@ -136,3 +142,11 @@ int stk_agent_loop_tone(struct stk_agent *agent, const char *text,
>
>  void append_menu_items_variant(DBusMessageIter *iter,
>                                const struct stk_menu_item *items);
> +
> +int stk_agent_request_time(struct stk_agent *agent, stk_agent_time_cb cb,
> +                       void *user_data, ofono_destroy_func destroy,
> +                       int timeout);
> +
> +int stk_agent_request_lang(struct stk_agent *agent, stk_agent_string_cb cb,
> +                       void *user_data, ofono_destroy_func destroy,
> +                       int timeout);
> diff --git a/src/stkutil.c b/src/stkutil.c
> index 377ceff..48ce93b 100644
> --- a/src/stkutil.c
> +++ b/src/stkutil.c
> @@ -4553,7 +4553,7 @@ static gboolean build_dataobj_datetime_timezone(struct stk_tlv_builder *tlv,
>                return TRUE;
>
>        /* Time zone information is optional */
> -       if (scts->timezone == (gint8) 0xff) {
> +       if (scts->timezone == -48) {

Ah, -48 is also a good value :)

But wouldn't the parser also need to translate 0xff into -48?

Best regards

Re: [PATCH 2/3] stk: Handle provide local info proactive command

[Denis Kenzior]

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

Hi Yang,

On 11/25/2010 06:29 AM, Yang Gu wrote:
> ---
>  src/smsutil.c  |    6 +-

Please send patches against smsutil separately.

>  src/stk.c      |  113 +++++++++++++++++++++++++++++++++++++++++
>  src/stkagent.c |  153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/stkagent.h |   14 +++++
>  src/stkutil.c  |    2 +-

Same goes for stkutil

>  5 files changed, 284 insertions(+), 4 deletions(-)
> 
> diff --git a/src/smsutil.c b/src/smsutil.c
> index e6dbf5f..5394817 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -324,10 +324,10 @@ gboolean sms_encode_scts(const struct sms_scts *in, unsigned char *pdu,
>  	if (in->year > 99)
>  		return FALSE;
>  
> -	if (in->month > 12)
> +	if (in->month > 12 || in->month == 0)
>  		return FALSE;
>  
> -	if (in->day > 31)
> +	if (in->day > 31 || in->day == 0)
>  		return FALSE;
>  
>  	if (in->hour > 23)
> @@ -339,7 +339,7 @@ gboolean sms_encode_scts(const struct sms_scts *in, unsigned char *pdu,
>  	if (in->second > 59)
>  		return FALSE;
>  
> -	if ((in->timezone > 12*4-1) || (in->timezone < -(12*4-1)))
> +	if ((in->timezone > 12*4) || (in->timezone < -(12*4-1)))

Err, why would +48 be valid?

<snip>

> diff --git a/src/stkagent.c b/src/stkagent.c
> index 5cf83e4..78620d5 100644
> --- a/src/stkagent.c
> +++ b/src/stkagent.c
> @@ -956,3 +956,156 @@ int stk_agent_loop_tone(struct stk_agent *agent, const char *text,
>  
>  	return 0;
>  }
> +int stk_agent_request_time(struct stk_agent *agent, stk_agent_time_cb cb,
> +				void *user_data, ofono_destroy_func destroy,
> +				int timeout)

So I really don't think we need an agent for this one.  Just having
oFono grab the time and return the info should be enough.


<snip>

> +int stk_agent_request_lang(struct stk_agent *agent, stk_agent_string_cb cb,
> +				void *user_data, ofono_destroy_func destroy,
> +				int timeout)
> +{

I'm not an expert on i18n, but this seems to be something oFono can
handle internally.  Perhaps querying the LANG environment variable or
something?

<snip>

> diff --git a/src/stkutil.c b/src/stkutil.c
> index 377ceff..48ce93b 100644
> --- a/src/stkutil.c
> +++ b/src/stkutil.c
> @@ -4553,7 +4553,7 @@ static gboolean build_dataobj_datetime_timezone(struct stk_tlv_builder *tlv,
>  		return TRUE;
>  
>  	/* Time zone information is optional */
> -	if (scts->timezone == (gint8) 0xff) {
> +	if (scts->timezone == -48) {
>  		memcpy(&timestamp, scts, sizeof(timestamp));
>  		timestamp.timezone = 0;
>  		if (sms_encode_scts(&timestamp, value, &offset) != TRUE)

As mentioned previously, my vote is for has_tz here.

Regards,
-Denis

RE: [PATCH 2/3] stk: Handle provide local info proactive command

[Gu, Yang]

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

Hi Denis,

>> diff --git a/src/smsutil.c b/src/smsutil.c
>> index e6dbf5f..5394817 100644
>> --- a/src/smsutil.c
>> +++ b/src/smsutil.c
>> @@ -324,10 +324,10 @@ gboolean sms_encode_scts(const struct sms_scts *in,
>unsigned char *pdu,
>>  	if (in->year > 99)
>>  		return FALSE;
>>
>> -	if (in->month > 12)
>> +	if (in->month > 12 || in->month == 0)
>>  		return FALSE;
>>
>> -	if (in->day > 31)
>> +	if (in->day > 31 || in->day == 0)
>>  		return FALSE;
>>
>>  	if (in->hour > 23)
>> @@ -339,7 +339,7 @@ gboolean sms_encode_scts(const struct sms_scts *in,
>unsigned char *pdu,
>>  	if (in->second > 59)
>>  		return FALSE;
>>
>> -	if ((in->timezone > 12*4-1) || (in->timezone < -(12*4-1)))
>> +	if ((in->timezone > 12*4) || (in->timezone < -(12*4-1)))
>
>Err, why would +48 be valid?

I think all the timezone should be a circle (24 hours). If it can't be -48, it needs to contain +48. 
Also please see 9.2.6.9 in http://gsm-history.org/fileadmin/user_upload/Key_SMS_Documents/GSM_Rec_03.40_v_2.2.0.pdf, which says "The Time Zone may be a number in the range -47 to +48".



Regards,
-Yang

Re: [PATCH 2/3] stk: Handle provide local info proactive command

[Denis Kenzior]

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

Hi Yang,

On 11/28/2010 08:47 PM, Gu, Yang wrote:
> Hi Denis,
> 
>>> diff --git a/src/smsutil.c b/src/smsutil.c
>>> index e6dbf5f..5394817 100644
>>> --- a/src/smsutil.c
>>> +++ b/src/smsutil.c
>>> @@ -324,10 +324,10 @@ gboolean sms_encode_scts(const struct sms_scts *in,
>> unsigned char *pdu,
>>>  	if (in->year > 99)
>>>  		return FALSE;
>>>
>>> -	if (in->month > 12)
>>> +	if (in->month > 12 || in->month == 0)
>>>  		return FALSE;
>>>
>>> -	if (in->day > 31)
>>> +	if (in->day > 31 || in->day == 0)
>>>  		return FALSE;
>>>
>>>  	if (in->hour > 23)
>>> @@ -339,7 +339,7 @@ gboolean sms_encode_scts(const struct sms_scts *in,
>> unsigned char *pdu,
>>>  	if (in->second > 59)
>>>  		return FALSE;
>>>
>>> -	if ((in->timezone > 12*4-1) || (in->timezone < -(12*4-1)))
>>> +	if ((in->timezone > 12*4) || (in->timezone < -(12*4-1)))
>>
>> Err, why would +48 be valid?
> 
> I think all the timezone should be a circle (24 hours). If it can't be -48, it needs to contain +48. 
> Also please see 9.2.6.9 in http://gsm-history.org/fileadmin/user_upload/Key_SMS_Documents/GSM_Rec_03.40_v_2.2.0.pdf, which says "The Time Zone may be a number in the range -47 to +48".
> 

Ok, fair enough.  Though do note that the timezone field must account
for daylight savings time.  So in theory GMT+14 and GMT-12 are valid
timezone values as well.  See
http://en.wikipedia.org/wiki/List_of_time_zones_by_UTC_offset

Regards,
-Denis

[PATCH 3/3] test-stk: Add provide local info

[Yang Gu]

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

---
 test/test-stk-menu |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/test/test-stk-menu b/test/test-stk-menu
index 916a527..db3f8b7 100755
--- a/test/test-stk-menu
+++ b/test/test-stk-menu
@@ -134,6 +134,42 @@ class StkAgent(dbus.service.Object):
 			return False
 
 	@dbus.service.method("org.ofono.SimToolkitAgent",
+				in_signature="", out_signature="iyyyyyi")
+	def RequestTime(self):
+		print "Time should be in format: year, month, day, hour, minute, second[, timezone]"
+		userin = raw_input("Enter Time (t, b, time):")
+
+		if userin == 'b':
+			raise GoBack("User wishes to go back");
+		elif userin == 't':
+			raise EndSession("User wishes to terminate session");
+		else:
+			time = userin.split(',')
+
+			if len(time) != 6 and len(time) != 7:
+				raise GoBack("The input time is invalid");
+
+			if len(time) == 6:
+				timezone = -48;
+			else:
+				timezone = int(time[6])
+
+			return int(time[0]), int(time[1]), int(time[2]), int(time[3]), int(time[4]), int(time[5]), timezone
+
+	@dbus.service.method("org.ofono.SimToolkitAgent",
+				in_signature="", out_signature="s")
+	def RequestLanguage(self):
+		print "Follow ISO 639-1 to input language, that is, a pair of alpha-numeric characters"
+		userin = raw_input("Enter Language (t, b):")
+
+		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="", out_signature="")
 	def Cancel(self):
 		print "Cancel"
-- 
1.7.2.3

Re: [PATCH 0/3] Patch Description

[andrzej zaborowski]

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

Hi Yang,

On 25 November 2010 13:28, Yang Gu <yang.gu@intel.com> wrote:
> This series of patch is to add provide local info support by requesting the terminal to send time and language info. Please comment on the following aspects as I'm not sure after reading the spec:
> 1. Timezone may be a number in the range -47 through +48. In struct sms_scts, timezone is defined as gint8, thus 0xFF should shand for -1, which is a valid input. Thus I think build_dataobj_datetime_timezone() in src/stkutil.c is not correct. But I'm still not sure what value should be passed to oFono when timezone is absent.

I think you're right that build_dataobj_datetime_timezone() is wrong.
Also note that sms_decode_scts() and sms_encode_scts() only allow the
range -47 to 47, 48 would return an error.  I'm not sure what the
unknown time zone should be represented as, here are some options:

* 0 (same as no offset)
* 0xff because there's currently no GMT-00:15 time zone on earth
(http://en.wikipedia.org/wiki/List_of_time_zones_by_country)
* 0x80 (a currently unused value could be #defined as unknown time zone)
* the struct could be extended with a .has_tz boolean.

> 2. DBUS_TYPE_BYTE represents an 8-bit unsigned integer, and D-Bus doesn't have a type related to 8-bit signed integer. So what's the best way to represent a timezone?

Maybe instead of asking D-bus, ofono should use tzset() to retrieve
the time zone information and use localtime() for the other fields?

> 3. Only one byte is used to represent the year. Is the following logic correct to get the year with one byte?
> if (year_dbus >= 2000)
>        year = year_dbus - 2000;
> else
>        year = year_dbus - 1900;

I believe this is correct.

Best regards

RE: [PATCH 0/3] Patch Description

[Gu, Yang]

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

Hi Andrew,


>-----Original Message-----
>From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf Of
>andrzej zaborowski
>Sent: Friday, November 26, 2010 7:26 AM
>To: ofono(a)ofono.org
>Subject: Re: [PATCH 0/3] Patch Description
>
>Hi Yang,
>
>On 25 November 2010 13:28, Yang Gu <yang.gu@intel.com> wrote:
>> This series of patch is to add provide local info support by requesting the terminal to
>send time and language info. Please comment on the following aspects as I'm not
>sure after reading the spec:
>> 1. Timezone may be a number in the range -47 through +48. In struct sms_scts,
>timezone is defined as gint8, thus 0xFF should shand for -1, which is a valid input.
>Thus I think build_dataobj_datetime_timezone() in src/stkutil.c is not correct. But I'm
>still not sure what value should be passed to oFono when timezone is absent.
>
>I think you're right that build_dataobj_datetime_timezone() is wrong.
>Also note that sms_decode_scts() and sms_encode_scts() only allow the
>range -47 to 47, 48 would return an error.  I'm not sure what the
>unknown time zone should be represented as, here are some options:
>
>* 0 (same as no offset)
>* 0xff because there's currently no GMT-00:15 time zone on earth
>(http://en.wikipedia.org/wiki/List_of_time_zones_by_country)
>* 0x80 (a currently unused value could be #defined as unknown time zone)
>* the struct could be extended with a .has_tz boolean.

Personally I'd prefer has_tz Boolean, which is consistent with other structs. 

>
>> 2. DBUS_TYPE_BYTE represents an 8-bit unsigned integer, and D-Bus doesn't
>have a type related to 8-bit signed integer. So what's the best way to represent a
>timezone?
>
>Maybe instead of asking D-bus, ofono should use tzset() to retrieve
>the time zone information and use localtime() for the other fields?

If we don't need to ask D-Bus, we'd not bother defining the situation when timezone is absent. Denis, what's the requirement here?
Andrew, thanks for the comments in the other mail thread. I accept them all, but I will modify the patch until we're clear on this requirement. 


Regards,
-Yang

Re: [PATCH 0/3] Patch Description

[Denis Kenzior]

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

Hi Andrew,

On 11/25/2010 05:25 PM, andrzej zaborowski wrote:
> Hi Yang,
> 
> On 25 November 2010 13:28, Yang Gu <yang.gu@intel.com> wrote:
>> This series of patch is to add provide local info support by requesting the terminal to send time and language info. Please comment on the following aspects as I'm not sure after reading the spec:
>> 1. Timezone may be a number in the range -47 through +48. In struct sms_scts, timezone is defined as gint8, thus 0xFF should shand for -1, which is a valid input. Thus I think build_dataobj_datetime_timezone() in src/stkutil.c is not correct. But I'm still not sure what value should be passed to oFono when timezone is absent.
> 
> I think you're right that build_dataobj_datetime_timezone() is wrong.
> Also note that sms_decode_scts() and sms_encode_scts() only allow the
> range -47 to 47, 48 would return an error.  I'm not sure what the
> unknown time zone should be represented as, here are some options:
> 
> * 0 (same as no offset)
> * 0xff because there's currently no GMT-00:15 time zone on earth
> (http://en.wikipedia.org/wiki/List_of_time_zones_by_country)
> * 0x80 (a currently unused value could be #defined as unknown time zone)
> * the struct could be extended with a .has_tz boolean.

The has_tz variable gets my vote.  The rest looks ugly, and I don't
really see +48 as a valid value.

> 
>> 2. DBUS_TYPE_BYTE represents an 8-bit unsigned integer, and D-Bus doesn't have a type related to 8-bit signed integer. So what's the best way to represent a timezone?
> 
> Maybe instead of asking D-bus, ofono should use tzset() to retrieve
> the time zone information and use localtime() for the other fields?

That is my preference as well.  Perhaps one can use the tm_gmtoff value
from struct tm to figure out the timezone.  See sms_scts_to_time for
more details.

Regards,
-Denis