[PATCH] stk: Add busy error for the display text command

[PATCH] stk: Add busy error for the display text command

[Lucas, GuillaumeX]

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

From: Guillaume Lucas <guillaumex.lucas@intel.com>

According to the sequence 1.2 of the ETSI TS 102 384 a busy
screen error should be returns for the display text proactive
command when the ME is not able to display the text.
---
 doc/stk-api.txt |    1 +
 src/stk.c       |   15 +++++++++++++++
 src/stkagent.c  |   12 ++++++++++--
 src/stkagent.h  |    1 +
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/doc/stk-api.txt b/doc/stk-api.txt
index b9ca592..79daee6 100644
--- a/doc/stk-api.txt
+++ b/doc/stk-api.txt
@@ -116,6 +116,7 @@ Methods		byte RequestSelection(string title, byte icon_id,
 			cleared prior to the display of this text.
 
 			Possible Errors: [service].Error.SimToolkit.GoBack
+					 [service].Error.SimToolkit.Busy
 
 			Implementation notes:
 
diff --git a/src/stk.c b/src/stk.c
index a4abb7d..7b39f7e 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1203,6 +1203,9 @@ static void display_text_cb(enum stk_agent_result result, void *user_data)
 {
 	struct ofono_stk *stk = user_data;
 	gboolean confirm;
+	struct stk_response rsp;
+	static unsigned char screen_busy_result[] = { 0x01 };
+	static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
 
 	stk->respond_on_exit = FALSE;
 
@@ -1250,6 +1253,15 @@ static void display_text_cb(enum stk_agent_result result, void *user_data)
 			STK_RESULT_TYPE_NO_RESPONSE : STK_RESULT_TYPE_SUCCESS);
 		break;
 
+	case STK_AGENT_RESULT_BUSY:
+		memset(&rsp, 0, sizeof(rsp));
+		rsp.result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		rsp.result.additional_len = sizeof(screen_busy_result);
+		rsp.result.additional = screen_busy_result;
+		if (stk_respond(stk, &rsp, stk_command_cb))
+			stk_command_cb(&error, stk);
+		break;
+
 	case STK_AGENT_RESULT_TERMINATE:
 	default:
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
@@ -1366,6 +1378,7 @@ static void request_confirmation_cb(enum stk_agent_result result,
 		break;
 
 	case STK_AGENT_RESULT_TERMINATE:
+	default:
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
 		break;
 	}
@@ -1408,6 +1421,7 @@ static void request_key_cb(enum stk_agent_result result, char *string,
 		break;
 
 	case STK_AGENT_RESULT_TERMINATE:
+	default:
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
 		break;
 	}
@@ -1505,6 +1519,7 @@ static void request_string_cb(enum stk_agent_result result, char *string,
 		break;
 
 	case STK_AGENT_RESULT_TERMINATE:
+	default:
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
 		break;
 	}
diff --git a/src/stkagent.c b/src/stkagent.c
index 8315040..874b6dd 100644
--- a/src/stkagent.c
+++ b/src/stkagent.c
@@ -41,6 +41,7 @@
 enum allowed_error {
 	ALLOWED_ERROR_GO_BACK	= 0x1,
 	ALLOWED_ERROR_TERMINATE	= 0x2,
+	ALLOWED_ERROR_BUSY		= 0x4,
 };
 
 struct stk_agent {
@@ -62,6 +63,7 @@ struct stk_agent {
 #define ERROR_PREFIX OFONO_SERVICE ".Error"
 #define GOBACK_ERROR ERROR_PREFIX ".GoBack"
 #define TERMINATE_ERROR ERROR_PREFIX ".EndSession"
+#define BUSY_ERROR ERROR_PREFIX ".Busy"
 
 static void stk_agent_send_noreply(struct stk_agent *agent, const char *method)
 {
@@ -194,6 +196,12 @@ static int check_error(struct stk_agent *agent, DBusMessage *reply,
 		goto out;
 	}
 
+	if ((allowed_errors & ALLOWED_ERROR_BUSY) &&
+			g_str_equal(err.name, BUSY_ERROR)) {
+		*out_result = STK_AGENT_RESULT_BUSY;
+		goto out;
+	}
+
 	result = -EINVAL;
 
 out:
@@ -376,8 +384,8 @@ static void display_text_cb(DBusPendingCall *call, void *data)
 	gboolean remove_agent;
 
 	if (check_error(agent, reply,
-			ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE,
-			&result) == -EINVAL) {
+			ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE | 
+			ALLOWED_ERROR_BUSY, &result) == -EINVAL) {
 		remove_agent = TRUE;
 		goto error;
 	}
diff --git a/src/stkagent.h b/src/stkagent.h
index c8e1886..517bcfe 100644
--- a/src/stkagent.h
+++ b/src/stkagent.h
@@ -26,6 +26,7 @@ enum stk_agent_result {
 	STK_AGENT_RESULT_BACK,
 	STK_AGENT_RESULT_TERMINATE,
 	STK_AGENT_RESULT_TIMEOUT,
+	STK_AGENT_RESULT_BUSY,
 };
 
 struct stk_menu_item {
-- 
1.7.0.4

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH] stk: Add busy error for the display text command

[Denis Kenzior]

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

Hi Guillaume,

On 11/30/2010 07:21 AM, Lucas, GuillaumeX wrote:
> From: Guillaume Lucas <guillaumex.lucas@intel.com>
> 
> According to the sequence 1.2 of the ETSI TS 102 384 a busy
> screen error should be returns for the display text proactive
> command when the ME is not able to display the text.
> ---
>  doc/stk-api.txt |    1 +
>  src/stk.c       |   15 +++++++++++++++
>  src/stkagent.c  |   12 ++++++++++--
>  src/stkagent.h  |    1 +
>  4 files changed, 27 insertions(+), 2 deletions(-)
> 

Patch has been applied, however I fixed one minor booboo for you:

Applying: stk: Add busy error for the display text command
/home/denkenz/ofono-master/.git/rebase-apply/patch:113: trailing whitespace.
			ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE |
fatal: 1 line adds whitespace errors.
Patch failed at 0001 stk: Add busy error for the display text command

Regards,
-Denis

[PATCH] stk: Add busy error for the display text command

[Lucas, GuillaumeX]

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

From: Guillaume Lucas <guillaumex.lucas@intel.com>

According to the sequence 1.2 of the ETSI TS 102 384 a busy
screen error type must be returned for the display text 
proactive command when the message is not urgent and if the
ME is not able to display the text.
---
 doc/stk-api.txt |    1 +
 src/stk.c       |    7 +++++++
 src/stkagent.c  |   12 ++++++++++--
 src/stkagent.h  |    1 +
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/doc/stk-api.txt b/doc/stk-api.txt
index b9ca592..79daee6 100644
--- a/doc/stk-api.txt
+++ b/doc/stk-api.txt
@@ -116,6 +116,7 @@ Methods		byte RequestSelection(string title, byte icon_id,
 			cleared prior to the display of this text.
 
 			Possible Errors: [service].Error.SimToolkit.GoBack
+					 [service].Error.SimToolkit.Busy
 
 			Implementation notes:
 
diff --git a/src/stk.c b/src/stk.c
index ac2e646..e6b78a4 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1250,6 +1250,10 @@ static void display_text_cb(enum stk_agent_result result, void *user_data)
 			STK_RESULT_TYPE_NO_RESPONSE : STK_RESULT_TYPE_SUCCESS);
 		break;
 
+	case STK_AGENT_RESULT_BUSY:
+		send_simple_response(stk, STK_RESULT_TYPE_TERMINAL_BUSY);
+		break;
+
 	case STK_AGENT_RESULT_TERMINATE:
 	default:
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
@@ -1363,6 +1367,7 @@ static void request_confirmation_cb(enum stk_agent_result result,
 		break;
 
 	case STK_AGENT_RESULT_TERMINATE:
+	default:
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
 		break;
 	}
@@ -1405,6 +1410,7 @@ static void request_key_cb(enum stk_agent_result result, char *string,
 		break;
 
 	case STK_AGENT_RESULT_TERMINATE:
+	default:
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
 		break;
 	}
@@ -1502,6 +1508,7 @@ static void request_string_cb(enum stk_agent_result result, char *string,
 		break;
 
 	case STK_AGENT_RESULT_TERMINATE:
+	default:
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
 		break;
 	}
diff --git a/src/stkagent.c b/src/stkagent.c
index 5cf83e4..6e8aff6 100644
--- a/src/stkagent.c
+++ b/src/stkagent.c
@@ -41,6 +41,7 @@
 enum allowed_error {
 	ALLOWED_ERROR_GO_BACK	= 0x1,
 	ALLOWED_ERROR_TERMINATE	= 0x2,
+	ALLOWED_ERROR_BUSY	= 0x4,
 };
 
 struct stk_agent {
@@ -62,6 +63,7 @@ struct stk_agent {
 #define ERROR_PREFIX OFONO_SERVICE ".Error"
 #define GOBACK_ERROR ERROR_PREFIX ".GoBack"
 #define TERMINATE_ERROR ERROR_PREFIX ".EndSession"
+#define BUSY_ERROR ERROR_PREFIX ".Busy"
 
 static void stk_agent_send_noreply(struct stk_agent *agent, const char *method)
 {
@@ -194,6 +196,12 @@ static int check_error(struct stk_agent *agent, DBusMessage *reply,
 		goto out;
 	}
 
+	if ((allowed_errors & ALLOWED_ERROR_BUSY) &&
+			g_str_equal(err.name, BUSY_ERROR)) {
+		*out_result = STK_AGENT_RESULT_BUSY;
+		goto out;
+	}
+
 	result = -EINVAL;
 
 out:
@@ -376,8 +384,8 @@ static void display_text_cb(DBusPendingCall *call, void *data)
 	gboolean remove_agent;
 
 	if (check_error(agent, reply,
-			ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE,
-			&result) == -EINVAL) {
+			ALLOWED_ERROR_GO_BACK | ALLOWED_ERROR_TERMINATE | 
+			ALLOWED_ERROR_BUSY, &result) == -EINVAL) {
 		remove_agent = TRUE;
 		goto error;
 	}
diff --git a/src/stkagent.h b/src/stkagent.h
index c8e1886..517bcfe 100644
--- a/src/stkagent.h
+++ b/src/stkagent.h
@@ -26,6 +26,7 @@ enum stk_agent_result {
 	STK_AGENT_RESULT_BACK,
 	STK_AGENT_RESULT_TERMINATE,
 	STK_AGENT_RESULT_TIMEOUT,
+	STK_AGENT_RESULT_BUSY,
 };
 
 struct stk_menu_item {
-- 
1.7.0.4
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH] stk: Add busy error for the display text command

[andrzej zaborowski]

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

Hi Guillaume,

On 25 November 2010 09:44, Lucas, GuillaumeX <guillaumex.lucas@intel.com> wrote:
> From: Guillaume Lucas <guillaumex.lucas@intel.com>
>
> According to the sequence 1.2 of the ETSI TS 102 384 a busy
> screen error type must be returned for the display text
> proactive command when the message is not urgent and if the
> ME is not able to display the text.
> ---
>  doc/stk-api.txt |    1 +
>  src/stk.c       |    7 +++++++
>  src/stkagent.c  |   12 ++++++++++--
>  src/stkagent.h  |    1 +
>  4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/doc/stk-api.txt b/doc/stk-api.txt
> index b9ca592..79daee6 100644
> --- a/doc/stk-api.txt
> +++ b/doc/stk-api.txt
> @@ -116,6 +116,7 @@ Methods             byte RequestSelection(string title, byte icon_id,
>                        cleared prior to the display of this text.
>
>                        Possible Errors: [service].Error.SimToolkit.GoBack
> +                                        [service].Error.SimToolkit.Busy
>
>                        Implementation notes:
>
> diff --git a/src/stk.c b/src/stk.c
> index ac2e646..e6b78a4 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -1250,6 +1250,10 @@ static void display_text_cb(enum stk_agent_result result, void *user_data)
>                        STK_RESULT_TYPE_NO_RESPONSE : STK_RESULT_TYPE_SUCCESS);
>                break;
>
> +       case STK_AGENT_RESULT_BUSY:
> +               send_simple_response(stk, STK_RESULT_TYPE_TERMINAL_BUSY);
> +               break;
> +

According to TS 102.223 on Screen Busy we also need to append a
"additional info" field in the Terminal Busy response with a 1-byte
value 01 ("screen is busy").

Denis on IRC also wondered if this response should be allowed when the
message is flagged urgent.

>        case STK_AGENT_RESULT_TERMINATE:
>        default:
>                send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
> @@ -1363,6 +1367,7 @@ static void request_confirmation_cb(enum stk_agent_result result,
>                break;
>
>        case STK_AGENT_RESULT_TERMINATE:
> +       default:
>                send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
>                break;

Most of the other commands also allow a "Terminal busy" response in
table 6.1 in section 6.11, without giving a specific "additional info"
value.  I think it would be ok to treat them the same way as
DisplayText and supply the "screen is busy" value, what do you think?

Best regards

RE: [PATCH] stk: Add busy error for the display text command

[Lucas, GuillaumeX]

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

Hi Andrzej

> 
> Hi Guillaume,
> 
> On 25 November 2010 09:44, Lucas, GuillaumeX
> <guillaumex.lucas@intel.com> wrote:
> > From: Guillaume Lucas <guillaumex.lucas@intel.com>
> >
> > According to the sequence 1.2 of the ETSI TS 102 384 a busy
> > screen error type must be returned for the display text
> > proactive command when the message is not urgent and if the
> > ME is not able to display the text.
> > ---
> >  doc/stk-api.txt |    1 +
> >  src/stk.c       |    7 +++++++
> >  src/stkagent.c  |   12 ++++++++++--
> >  src/stkagent.h  |    1 +
> >  4 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/stk-api.txt b/doc/stk-api.txt
> > index b9ca592..79daee6 100644
> > --- a/doc/stk-api.txt
> > +++ b/doc/stk-api.txt
> > @@ -116,6 +116,7 @@ Methods             byte RequestSelection(string
> title, byte icon_id,
> >                        cleared prior to the display of this text.
> >
> >                        Possible Errors:
> [service].Error.SimToolkit.GoBack
> > +
>  [service].Error.SimToolkit.Busy
> >
> >                        Implementation notes:
> >
> > diff --git a/src/stk.c b/src/stk.c
> > index ac2e646..e6b78a4 100644
> > --- a/src/stk.c
> > +++ b/src/stk.c
> > @@ -1250,6 +1250,10 @@ static void display_text_cb(enum
> stk_agent_result result, void *user_data)
> >                        STK_RESULT_TYPE_NO_RESPONSE :
> STK_RESULT_TYPE_SUCCESS);
> >                break;
> >
> > +       case STK_AGENT_RESULT_BUSY:
> > +               send_simple_response(stk,
> STK_RESULT_TYPE_TERMINAL_BUSY);
> > +               break;
> > +
> 
> According to TS 102.223 on Screen Busy we also need to append a
> "additional info" field in the Terminal Busy response with a 1-byte
> value 01 ("screen is busy").

Yes you're right. I've seen this but after sending my patch.
I've another patch in my pipe regarding this but in top of the current busy patch.
I'm waiting the integration of the busy patch to submit it.

> 
> Denis on IRC also wondered if this response should be allowed when the
> message is flagged urgent.
> 

According to the spec I think no, this type of response is not allowed when the message is fluffed urgent.

> >        case STK_AGENT_RESULT_TERMINATE:
> >        default:
> >                send_simple_response(stk,
> STK_RESULT_TYPE_USER_TERMINATED);
> > @@ -1363,6 +1367,7 @@ static void request_confirmation_cb(enum
> stk_agent_result result,
> >                break;
> >
> >        case STK_AGENT_RESULT_TERMINATE:
> > +       default:
> >                send_simple_response(stk,
> STK_RESULT_TYPE_USER_TERMINATED);
> >                break;
> 
> Most of the other commands also allow a "Terminal busy" response in
> table 6.1 in section 6.11, without giving a specific "additional info"
> value.  I think it would be ok to treat them the same way as
> DisplayText and supply the "screen is busy" value, what do you think?
> 

You're right for the other commands but those cases are already handled by ofono.
In fact there is only for the display text proactive command that we can have this error for a busy screen reason.

Regards,
Guillaume
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

RE: [PATCH] stk: Add busy error for the display text command

[Jeevaka.Badrappan]

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

 
Hi Guillaume,

> > 
> > Denis on IRC also wondered if this response should be 
> allowed when the 
> > message is flagged urgent.
> > 
> 
> According to the spec I think no, this type of response is 
> not allowed when the message is fluffed urgent.
> 

As per the ETSI TS 102 223 V9.0.0 section 6.4.1, "screen is busy"
applies to high priority text as well. If urgent refers to 
higher priority text which is the case, then I think "screen busy" is
possible. Find below the information from specification.

"High priority text shall be displayed on the screen immediately, except
if there is a conflict of priority level of alerting
such as incoming calls or a low battery warning. In that situation, the
resolution is left to the terminal. If the command is
rejected in spite of the high priority, the terminal shall inform the
UICC using TERMINAL RESPONSE (terminal
currently unable to process command - screen is busy)."

Regards,
Jeevaka

Re: [PATCH] stk: Add busy error for the display text command

[Denis Kenzior]

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

Hi Guillaume,

>> According to TS 102.223 on Screen Busy we also need to append a
>> "additional info" field in the Terminal Busy response with a 1-byte
>> value 01 ("screen is busy").
> 
> Yes you're right. I've seen this but after sending my patch.
> I've another patch in my pipe regarding this but in top of the current busy patch.
> I'm waiting the integration of the busy patch to submit it.

This is not how it works ;)  Please resubmit this patch with the code to
send 'Screen is Busy' additional info included.  You can even submit
your other pending patches in a series along with this one...

So e.g.

patch 1/n -> reworked add busy error...
patch 2/n -> pending patch 1,
etc

Regards,
-Denis

RE: [PATCH] stk: Add busy error for the display text command

[Lucas, GuillaumeX]

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

Hi Denis,

> -----Original Message-----
> From: Denis Kenzior [mailto:denkenz(a)gmail.com]
> Sent: Tuesday, November 30, 2010 1:25 PM
> To: ofono(a)ofono.org
> Cc: Lucas, GuillaumeX
> Subject: Re: [PATCH] stk: Add busy error for the display text command
> 
> Hi Guillaume,
> 
> >> According to TS 102.223 on Screen Busy we also need to append a
> >> "additional info" field in the Terminal Busy response with a 1-byte
> >> value 01 ("screen is busy").
> >
> > Yes you're right. I've seen this but after sending my patch.
> > I've another patch in my pipe regarding this but in top of the
> current busy patch.
> > I'm waiting the integration of the busy patch to submit it.
> 
> This is not how it works ;)  Please resubmit this patch with the code
> to
> send 'Screen is Busy' additional info included.  You can even submit
> your other pending patches in a series along with this one...
> 
> So e.g.
> 
> patch 1/n -> reworked add busy error...
> patch 2/n -> pending patch 1,
> etc
> 

Ok, I'll resubmmit my patch with all the necessary changes.

Regards,
Guillaume
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.