Re: [PATCH] Set session state to RESPONSE_SENT after sending response.

Re: [PATCH] Set session state to RESPONSE_SENT after sending response.

[Denis Kenzior]

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

Hi Andrew,

Good catch, patch has been applied.

> Hi Denis,
> I am not entirely sure this new state is needed though.  As the commit says
> the network may continue the dialog but it is not obliged to send any kind

The state is actually meant to un-confuse the USSD_STATE_ACTIVE if clause in 
ofono_ussd_notify.  Otherwise we treat it exactly like USSD_STATE_ACTIVE.

> of response to our reponse, at least this is not in the specs.  So if it
> doesn't send anything (neither a notification nor request) we will remain
> in this state and user will not be able to Initiate a new session for that

In theory the modem should time out the USSD request and send us a notification 
regardless.  I'm afraid we have to rely on the modem for this one, since the 
modem would refuse to send USSDs if a session is active anyway.

> time.  The other option, if I misunderstood the spec, would be to make the
> Respond method return the string.

I'm actually open to the idea, the reason 'Initiate' returns a string is that 
95% of the time there is no dialog, just straight request -> response.

Regards,
-Denis

[PATCH] Set session state to RESPONSE_SENT after sending response.

[Andrzej Zaborowski]

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

Hi Denis,
I am not entirely sure this new state is needed though.  As the commit says
the network may continue the dialog but it is not obliged to send any kind
of response to our reponse, at least this is not in the specs.  So if it
doesn't send anything (neither a notification nor request) we will remain
in this state and user will not be able to Initiate a new session for that
time.  The other option, if I misunderstood the spec, would be to make the
Respond method return the string.

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

diff --git a/src/ussd.c b/src/ussd.c
index 4f5a131..a2a4f5d 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -463,7 +463,7 @@ static void ussd_response_callback(const struct ofono_error *error, void *data)
 	DBusMessage *reply;
 
 	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
-		ussd_change_state(ussd, USSD_STATE_ACTIVE);
+		ussd_change_state(ussd, USSD_STATE_RESPONSE_SENT);
 		reply = dbus_message_new_method_return(ussd->pending);
 	} else {
 		ussd_change_state(ussd, USSD_STATE_IDLE);
-- 
1.6.1

Re: [PATCH] Set session state to RESPONSE_SENT after sending response.

[Andrzej Zaborowski]

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

On 27 February 2010 18:40, Denis Kenzior <denkenz@gmail.com> wrote:
>> time.  The other option, if I misunderstood the spec, would be to make the
>> Respond method return the string.
>
> I'm actually open to the idea, the reason 'Initiate' returns a string is that
> 95% of the time there is no dialog, just straight request -> response.

Right, here's a patch to return the string from Respond(), in the case
of a straight network request -> user response, it'll return an empty
string.

Regards

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Return-network-s-USSD-reponses-from-the-Respond-meth.patch --]
[-- Type: text/x-patch, Size: 2940 bytes --]

From 1a38df2e1f385c921091d785f6f02ab6640d4359 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Wed, 3 Mar 2010 09:35:11 +0100
Subject: [PATCH] Return network's USSD reponses from the Respond method instead of signalling

---
 doc/supplementaryservices-api.txt |    2 +-
 src/ussd.c                        |   31 ++++++++++++++++++++++++-------
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/doc/supplementaryservices-api.txt b/doc/supplementaryservices-api.txt
index 23796c1..115e2ff 100644
--- a/doc/supplementaryservices-api.txt
+++ b/doc/supplementaryservices-api.txt
@@ -16,7 +16,7 @@ Methods		string, variant Initiate(string command)
 			new command can be initiated until this one is
 			cancelled or ended.
 
-		void Respond(string reply)
+		string Respond(string reply)
 
 			Send a response to the network either when
 			it is awaiting further input after Initiate()
diff --git a/src/ussd.c b/src/ussd.c
index a2a4f5d..9ec7600 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -360,8 +360,20 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 		else
 			ussd_change_state(ussd, USSD_STATE_IDLE);
 
-	} else if (ussd->state == USSD_STATE_IDLE ||
-			ussd->state == USSD_STATE_RESPONSE_SENT) {
+	} else if (ussd->state == USSD_STATE_RESPONSE_SENT) {
+		reply = dbus_message_new_method_return(ussd->pending);
+
+		if (!str)
+			str = "";
+
+		dbus_message_append_args(reply, DBUS_TYPE_STRING, &str,
+						DBUS_TYPE_INVALID);
+
+		if (status == OFONO_USSD_STATUS_ACTION_REQUIRED)
+			ussd_change_state(ussd, USSD_STATE_USER_ACTION);
+		else
+			ussd_change_state(ussd, USSD_STATE_IDLE);
+	} else if (ussd->state == USSD_STATE_IDLE) {
 		const char *signal_name;
 		const char *path = __ofono_atom_get_path(ussd->atom);
 		int new_state;
@@ -462,14 +474,19 @@ static void ussd_response_callback(const struct ofono_error *error, void *data)
 	struct ofono_ussd *ussd = data;
 	DBusMessage *reply;
 
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		DBG("ussd response failed with error: %s",
+				telephony_error_to_str(error));
+
 	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
 		ussd_change_state(ussd, USSD_STATE_RESPONSE_SENT);
-		reply = dbus_message_new_method_return(ussd->pending);
-	} else {
-		ussd_change_state(ussd, USSD_STATE_IDLE);
-		reply = __ofono_error_failed(ussd->pending);
+		return;
 	}
 
+	if (!ussd->pending)
+		return;
+
+	reply = __ofono_error_failed(ussd->pending);
 	__ofono_dbus_pending_reply(&ussd->pending, reply);
 }
 
@@ -575,7 +592,7 @@ static DBusMessage *ussd_get_properties(DBusConnection *conn,
 static GDBusMethodTable ussd_methods[] = {
 	{ "Initiate",		"s",	"sv",		ussd_initiate,
 					G_DBUS_METHOD_FLAG_ASYNC },
-	{ "Respond",		"s",	"",		ussd_respond,
+	{ "Respond",		"s",	"s",		ussd_respond,
 					G_DBUS_METHOD_FLAG_ASYNC },
 	{ "Cancel",		"",	"",		ussd_cancel,
 					G_DBUS_METHOD_FLAG_ASYNC },
-- 
1.6.1