[PATCH, rfc] Handle network-initiated ussd requests.

[PATCH, rfc] Handle network-initiated ussd requests.

[Andrzej Zaborowski]

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

This adds the methods on the D-bus interface to allow the
client to handle USSD requests from the network, according to 22.090.
Unfortunately this document is not clear on every point and some
details can't be implemented.  This includes reporting unsupported
request to the network, unsupported language, ME busy etc, because
there isn't an AT command for that.

To be really pedantic, the Initiate() return value and the
NotificationReceived() and RequestReceived()  signals should also
supply the language of the string.  I also first added a "Timeout"
property with default value of 10 seconds after which oFono
automatically replies "not supported" to network-initiated requests, so
that it is compliant when no client listents to the signals, but removed
it because it isn't useful.
---
 doc/ussd-api.txt |   45 ++++++++++++++++++
 include/ussd.h   |    4 ++
 src/ussd.c       |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 180 insertions(+), 4 deletions(-)
 create mode 100644 doc/ussd-api.txt

diff --git a/doc/ussd-api.txt b/doc/ussd-api.txt
new file mode 100644
index 0000000..0f42b3a
--- /dev/null
+++ b/doc/ussd-api.txt
@@ -0,0 +1,45 @@
+SupplementaryServices hierarchy
+==========================
+
+Service		org.ofono
+Interface	org.ofono.SupplementaryServices
+Object path	[variable prefix]/{modem0,modem1,...}
+
+Methods		string, variant Initiate(string command)
+
+			Sends a USSD command string to the network
+			initiating a session.  When the request is handled
+			by the appropriate node of the network, the
+			method returns the response or an appropriate
+			error.  The network may be awaiting further response
+			from the ME after returning from this method and no
+			new command can be initiated until this one is
+			cancelled or ended.
+
+		void Respond(string reply)
+
+			Send a response to the network either when
+			it is awaiting further input after Initiate()
+			was called or after a network-initiated request.
+
+		void Cancel()
+
+			Cancel an ongoing USSD session, mobile- or
+			network-initiated.
+
+		void NotSupported()
+
+			Send a "not supported" response to a network's
+			request, ending current session.
+
+Signals		NotificationReceived(string attribute, variant value)
+
+			Signal is emitted on a network-initiated USSD
+			request for which no response is needed.
+
+		RequestReceived(string attribute, variant value)
+
+			Signal is emitted on a network-initiated USSD
+			request for which a response must be sent using
+			the Respond method unless it is cancelled or
+			the request is not supported.
diff --git a/include/ussd.h b/include/ussd.h
index 96e04cb..1f66de0 100644
--- a/include/ussd.h
+++ b/include/ussd.h
@@ -47,8 +47,12 @@ struct ofono_ussd_driver {
 	void (*remove)(struct ofono_ussd *ussd);
 	void (*request)(struct ofono_ussd *ussd, const char *str,
 				ofono_ussd_cb_t, void *data);
+	void (*respond)(struct ofono_ussd *ussd, const char *str,
+				ofono_ussd_cb_t, void *data);
 	void (*cancel)(struct ofono_ussd *ussd,
 				ofono_ussd_cb_t cb, void *data);
+	void (*respond_not_supported)(struct ofono_ussd *ussd,
+				ofono_ussd_cb_t cb, void *data);
 };
 
 void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str);
diff --git a/src/ussd.c b/src/ussd.c
index 4221dfa..6416ee9 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -274,6 +274,11 @@ out:
 	return ret;
 }
 
+static void ussd_not_supported_callback(const struct ofono_error *error,
+		void *data)
+{
+}
+
 void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
@@ -303,14 +308,14 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 		goto out;
 	}
 
+	if (!str)
+		str = "";
+
 	/* TODO: Rework this in the Agent framework */
 	if (ussd->state == USSD_STATE_ACTIVE) {
 
 		reply = dbus_message_new_method_return(ussd->pending);
 
-		if (!str)
-			str = "";
-
 		dbus_message_iter_init_append(reply, &iter);
 
 		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
@@ -329,10 +334,46 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 		else
 			ussd->state = USSD_STATE_IDLE;
 
+	} else if (ussd->state == USSD_STATE_IDLE && ussd->pending == NULL) {
+		DBusMessage *signal;
+		const char *signal_name;
+		const char *path = __ofono_atom_get_path(ussd->atom);
+
+		if (status == OFONO_USSD_STATUS_ACTION_REQUIRED) {
+			ussd->state = USSD_STATE_USER_ACTION;
+			signal_name = "RequestReceived";
+		} else {
+			ussd->state = USSD_STATE_IDLE;
+			signal_name = "NotificationReceived";
+		}
+
+		signal = dbus_message_new_signal(path,
+				SUPPLEMENTARY_SERVICES_INTERFACE,
+				signal_name);
+
+		dbus_message_iter_init_append(signal, &iter);
+
+		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
+						&ussdstr);
+
+		dbus_message_iter_open_container(&iter, DBUS_TYPE_VARIANT, sig,
+							&variant);
+
+		dbus_message_iter_append_basic(&variant, DBUS_TYPE_STRING,
+						&str);
+
+		dbus_message_iter_close_container(&iter, &variant);
+
+		g_dbus_send_message(conn, signal);
+		return;
 	} else {
 		ofono_error("Received an unsolicited USSD, ignoring for now...");
 		DBG("USSD is: status: %d, %s", status, str);
 
+		if (ussd->driver->respond_not_supported)
+			ussd->driver->respond_not_supported(ussd,
+					ussd_not_supported_callback, NULL);
+
 		return;
 	}
 
@@ -380,7 +421,7 @@ static DBusMessage *ussd_initiate(DBusConnection *conn, DBusMessage *msg,
 	if (ussd->flags & USSD_FLAG_PENDING)
 		return __ofono_error_busy(msg);
 
-	if (ussd->state == USSD_STATE_ACTIVE)
+	if (ussd->state != USSD_STATE_IDLE)
 		return __ofono_error_busy(msg);
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &str,
@@ -411,6 +452,64 @@ static DBusMessage *ussd_initiate(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
+static void ussd_response_callback(const struct ofono_error *error, void *data)
+{
+	struct ofono_ussd *ussd = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *reply;
+
+	ussd->flags &= ~USSD_FLAG_PENDING;
+
+	if (!ussd->pending)
+		return;
+
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+		ussd->state = USSD_STATE_IDLE;
+
+		reply = dbus_message_new_method_return(ussd->pending);
+	} else {
+		DBG("ussd response failed with error: %s",
+				telephony_error_to_str(error));
+
+		reply = __ofono_error_failed(ussd->pending);
+	}
+
+	g_dbus_send_message(conn, reply);
+
+	dbus_message_unref(ussd->pending);
+	ussd->pending = NULL;
+}
+
+static DBusMessage *ussd_respond(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_ussd *ussd = data;
+	const char *str;
+
+	if (ussd->flags & USSD_FLAG_PENDING)
+		return __ofono_error_busy(msg);
+
+	if (ussd->state != USSD_STATE_USER_ACTION)
+		return __ofono_error_not_active(msg);
+
+	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &str,
+					DBUS_TYPE_INVALID) == FALSE)
+		return __ofono_error_invalid_args(msg);
+
+	if (strlen(str) == 0)
+		return __ofono_error_invalid_format(msg);
+
+	if (!ussd->driver->respond)
+		return __ofono_error_not_implemented(msg);
+
+	ussd->flags |= USSD_FLAG_PENDING;
+	ussd->pending = dbus_message_ref(msg);
+
+	ussd->driver->respond(ussd, str, ussd_response_callback, ussd);
+
+	return NULL;
+}
+
 static void ussd_cancel_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_ussd *ussd = data;
@@ -458,15 +557,43 @@ static DBusMessage *ussd_cancel(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
+static DBusMessage *ussd_not_supported(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_ussd *ussd = data;
+
+	if (ussd->flags & USSD_FLAG_PENDING)
+		return __ofono_error_busy(msg);
+
+	if (ussd->state == USSD_STATE_IDLE)
+		return __ofono_error_not_active(msg);
+
+	if (!ussd->driver->respond_not_supported)
+		return __ofono_error_not_implemented(msg);
+
+	ussd->flags |= USSD_FLAG_PENDING;
+	ussd->pending = dbus_message_ref(msg);
+
+	ussd->driver->respond_not_supported(ussd, ussd_cancel_callback, ussd);
+
+	return NULL;
+}
+
 static GDBusMethodTable ussd_methods[] = {
 	{ "Initiate",	"s",	"sv",	ussd_initiate,
 					G_DBUS_METHOD_FLAG_ASYNC },
+	{ "Respond",	"s",	"",	ussd_respond,
+					G_DBUS_METHOD_FLAG_ASYNC },
 	{ "Cancel",	"",	"",	ussd_cancel,
 					G_DBUS_METHOD_FLAG_ASYNC },
+	{ "NotSupported",	"",	"",	ussd_not_supported,
+					G_DBUS_METHOD_FLAG_ASYNC },
 	{ }
 };
 
 static GDBusSignalTable ussd_signals[] = {
+	{ "NotificationReceived",	"sv" },
+	{ "RequestReceived",		"sv" },
 	{ }
 };
 
-- 
1.6.1

Re: [PATCH, rfc] Handle network-initiated ussd requests.

[Denis Kenzior]

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

Hi Andrew,

> This adds the methods on the D-bus interface to allow the
> client to handle USSD requests from the network, according to 22.090.
> Unfortunately this document is not clear on every point and some
> details can't be implemented.  This includes reporting unsupported
> request to the network, unsupported language, ME busy etc, because
> there isn't an AT command for that.
> 
> To be really pedantic, the Initiate() return value and the
> NotificationReceived() and RequestReceived()  signals should also
> supply the language of the string.  I also first added a "Timeout"
> property with default value of 10 seconds after which oFono
> automatically replies "not supported" to network-initiated requests, so
> that it is compliant when no client listents to the signals, but removed
> it because it isn't useful.
> ---
>  doc/ussd-api.txt |   45 ++++++++++++++++++
>  include/ussd.h   |    4 ++
>  src/ussd.c       |  135
>  ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files 
changed,
>  180 insertions(+), 4 deletions(-)
>  create mode 100644 doc/ussd-api.txt
> 
> diff --git a/doc/ussd-api.txt b/doc/ussd-api.txt
> new file mode 100644
> index 0000000..0f42b3a
> --- /dev/null
> +++ b/doc/ussd-api.txt
> @@ -0,0 +1,45 @@
> +SupplementaryServices hierarchy
> +==========================
> +
> +Service		org.ofono
> +Interface	org.ofono.SupplementaryServices
> +Object path	[variable prefix]/{modem0,modem1,...}
> +
> +Methods		string, variant Initiate(string command)
> +
> +			Sends a USSD command string to the network
> +			initiating a session.  When the request is handled
> +			by the appropriate node of the network, the
> +			method returns the response or an appropriate
> +			error.  The network may be awaiting further response
> +			from the ME after returning from this method and no
> +			new command can be initiated until this one is
> +			cancelled or ended.
> +
> +		void Respond(string reply)
> +
> +			Send a response to the network either when
> +			it is awaiting further input after Initiate()
> +			was called or after a network-initiated request.
> +
> +		void Cancel()
> +
> +			Cancel an ongoing USSD session, mobile- or
> +			network-initiated.
> +
> +		void NotSupported()
> +
> +			Send a "not supported" response to a network's
> +			request, ending current session.

Let us get rid of this for now

> +
> +Signals		NotificationReceived(string attribute, variant value)
> +
> +			Signal is emitted on a network-initiated USSD
> +			request for which no response is needed.

There is no reason to use string / variant pair here.  This was done on 
Initiate return to allow known supplementary service 
register/activate/interrogate/deactivate/erasure support.  Since we know this 
is a basic text USSD, we can just send a string here.

> +
> +		RequestReceived(string attribute, variant value)
> +
> +			Signal is emitted on a network-initiated USSD
> +			request for which a response must be sent using
> +			the Respond method unless it is cancelled or
> +			the request is not supported.

Same as above, a simple string will do.

> diff --git a/include/ussd.h b/include/ussd.h
> index 96e04cb..1f66de0 100644
> --- a/include/ussd.h
> +++ b/include/ussd.h
> @@ -47,8 +47,12 @@ struct ofono_ussd_driver {
>  	void (*remove)(struct ofono_ussd *ussd);
>  	void (*request)(struct ofono_ussd *ussd, const char *str,
>  				ofono_ussd_cb_t, void *data);
> +	void (*respond)(struct ofono_ussd *ussd, const char *str,
> +				ofono_ussd_cb_t, void *data);

Respond is the same as request, there's really no need for a separate method 
here.

>  	void (*cancel)(struct ofono_ussd *ussd,
>  				ofono_ussd_cb_t cb, void *data);
> +	void (*respond_not_supported)(struct ofono_ussd *ussd,
> +				ofono_ussd_cb_t cb, void *data);
>  };

No modem I know of actually implements this and it is not in the standards.  
Let us just use cancel for now.

> 
>  void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char
>  *str); diff --git a/src/ussd.c b/src/ussd.c
> index 4221dfa..6416ee9 100644
> --- a/src/ussd.c
> +++ b/src/ussd.c
> @@ -274,6 +274,11 @@ out:
>  	return ret;
>  }
> 
> +static void ussd_not_supported_callback(const struct ofono_error *error,
> +		void *data)
> +{
> +}
> +
>  void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char
>  *str) {
>  	DBusConnection *conn = ofono_dbus_get_connection();
> @@ -303,14 +308,14 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int
>  status, const char *str) goto out;
>  	}
> 
> +	if (!str)
> +		str = "";
> +

This should be in a separate cleanup patch.

>  	/* TODO: Rework this in the Agent framework */
>  	if (ussd->state == USSD_STATE_ACTIVE) {
> 
>  		reply = dbus_message_new_method_return(ussd->pending);
> 
> -		if (!str)
> -			str = "";
> -

As above.

>  		dbus_message_iter_init_append(reply, &iter);
> 
>  		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
> @@ -329,10 +334,46 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int
>  status, const char *str) else
>  			ussd->state = USSD_STATE_IDLE;
> 
> +	} else if (ussd->state == USSD_STATE_IDLE && ussd->pending == NULL) {
> +		DBusMessage *signal;
> +		const char *signal_name;
> +		const char *path = __ofono_atom_get_path(ussd->atom);
> +
> +		if (status == OFONO_USSD_STATUS_ACTION_REQUIRED) {
> +			ussd->state = USSD_STATE_USER_ACTION;
> +			signal_name = "RequestReceived";
> +		} else {
> +			ussd->state = USSD_STATE_IDLE;
> +			signal_name = "NotificationReceived";
> +		}
> +
> +		signal = dbus_message_new_signal(path,
> +				SUPPLEMENTARY_SERVICES_INTERFACE,
> +				signal_name);
> +
> +		dbus_message_iter_init_append(signal, &iter);
> +
> +		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING,
> +						&ussdstr);
> +
> +		dbus_message_iter_open_container(&iter, DBUS_TYPE_VARIANT, sig,
> +							&variant);
> +
> +		dbus_message_iter_append_basic(&variant, DBUS_TYPE_STRING,
> +						&str);
> +
> +		dbus_message_iter_close_container(&iter, &variant);
> +
> +		g_dbus_send_message(conn, signal);
> +		return;

g_dbus_emit_signal will be enough here.

>  	} else {
>  		ofono_error("Received an unsolicited USSD, ignoring for now...");
>  		DBG("USSD is: status: %d, %s", status, str);
> 
> +		if (ussd->driver->respond_not_supported)
> +			ussd->driver->respond_not_supported(ussd,
> +					ussd_not_supported_callback, NULL);
> +
>  		return;

There's no reason for this else clause to exist now.  The above else if 
condition is handling this case too.  Just look carefully at ussd driver for 
AT command modems, we always call back first, then issue ussd_notify.  This is 
the case for both modems with and without cancel support.

>  	}
> 
> @@ -380,7 +421,7 @@ static DBusMessage *ussd_initiate(DBusConnection *conn,
>  DBusMessage *msg, if (ussd->flags & USSD_FLAG_PENDING)
>  		return __ofono_error_busy(msg);
> 
> -	if (ussd->state == USSD_STATE_ACTIVE)
> +	if (ussd->state != USSD_STATE_IDLE)
>  		return __ofono_error_busy(msg);
> 
>  	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &str,
> @@ -411,6 +452,64 @@ static DBusMessage *ussd_initiate(DBusConnection
>  *conn, DBusMessage *msg, return NULL;
>  }
> 
> +static void ussd_response_callback(const struct ofono_error *error, void
>  *data) +{
> +	struct ofono_ussd *ussd = data;
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	DBusMessage *reply;
> +
> +	ussd->flags &= ~USSD_FLAG_PENDING;
> +
> +	if (!ussd->pending)
> +		return;
> +
> +	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
> +		ussd->state = USSD_STATE_IDLE;
> +
> +		reply = dbus_message_new_method_return(ussd->pending);
> +	} else {
> +		DBG("ussd response failed with error: %s",
> +				telephony_error_to_str(error));
> +
> +		reply = __ofono_error_failed(ussd->pending);
> +	}
> +
> +	g_dbus_send_message(conn, reply);
> +
> +	dbus_message_unref(ussd->pending);
> +	ussd->pending = NULL;

__ofono_dbus_pending_reply please.

> +}
> +
> +static DBusMessage *ussd_respond(DBusConnection *conn, DBusMessage *msg,
> +					void *data)
> +{
> +	struct ofono_ussd *ussd = data;
> +	const char *str;
> +
> +	if (ussd->flags & USSD_FLAG_PENDING)
> +		return __ofono_error_busy(msg);
> +
> +	if (ussd->state != USSD_STATE_USER_ACTION)
> +		return __ofono_error_not_active(msg);
> +
> +	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &str,
> +					DBUS_TYPE_INVALID) == FALSE)
> +		return __ofono_error_invalid_args(msg);
> +
> +	if (strlen(str) == 0)
> +		return __ofono_error_invalid_format(msg);
> +
> +	if (!ussd->driver->respond)
> +		return __ofono_error_not_implemented(msg);
> +
> +	ussd->flags |= USSD_FLAG_PENDING;

Don't use this flag anymore, I've taken it out.

> +	ussd->pending = dbus_message_ref(msg);
> +
> +	ussd->driver->respond(ussd, str, ussd_response_callback, ussd);

Use driver->request here.

> +
> +	return NULL;
> +}
> +
>  static void ussd_cancel_callback(const struct ofono_error *error, void
>  *data) {
>  	struct ofono_ussd *ussd = data;
> @@ -458,15 +557,43 @@ static DBusMessage *ussd_cancel(DBusConnection *conn,
>  DBusMessage *msg, return NULL;
>  }
> 
> +static DBusMessage *ussd_not_supported(DBusConnection *conn, DBusMessage
>  *msg, +					void *data)
> +{
> +	struct ofono_ussd *ussd = data;
> +
> +	if (ussd->flags & USSD_FLAG_PENDING)
> +		return __ofono_error_busy(msg);
> +
> +	if (ussd->state == USSD_STATE_IDLE)
> +		return __ofono_error_not_active(msg);
> +
> +	if (!ussd->driver->respond_not_supported)
> +		return __ofono_error_not_implemented(msg);
> +
> +	ussd->flags |= USSD_FLAG_PENDING;
> +	ussd->pending = dbus_message_ref(msg);
> +
> +	ussd->driver->respond_not_supported(ussd, ussd_cancel_callback, ussd);
> +
> +	return NULL;
> +}

Get rid of this method

> +
>  static GDBusMethodTable ussd_methods[] = {
>  	{ "Initiate",	"s",	"sv",	ussd_initiate,
>  					G_DBUS_METHOD_FLAG_ASYNC },
> +	{ "Respond",	"s",	"",	ussd_respond,
> +					G_DBUS_METHOD_FLAG_ASYNC },
>  	{ "Cancel",	"",	"",	ussd_cancel,
>  					G_DBUS_METHOD_FLAG_ASYNC },
> +	{ "NotSupported",	"",	"",	ussd_not_supported,
> +					G_DBUS_METHOD_FLAG_ASYNC },
>  	{ }
>  };
> 
>  static GDBusSignalTable ussd_signals[] = {
> +	{ "NotificationReceived",	"sv" },
> +	{ "RequestReceived",		"sv" },

Simple strings here

>  	{ }
>  };
> 

Regards,
-Denis

Re: [PATCH, rfc] Handle network-initiated ussd requests.

[Andrzej Zaborowski]

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

Hi,

On 15 February 2010 18:03, Denis Kenzior <denkenz@gmail.com> wrote:
>>       } else {
>>               ofono_error("Received an unsolicited USSD, ignoring for now...");
>>               DBG("USSD is: status: %d, %s", status, str);
>>
>> +             if (ussd->driver->respond_not_supported)
>> +                     ussd->driver->respond_not_supported(ussd,
>> +                                     ussd_not_supported_callback, NULL);
>> +
>>               return;
>
> There's no reason for this else clause to exist now.  The above else if
> condition is handling this case too.  Just look carefully at ussd driver for
> AT command modems, we always call back first, then issue ussd_notify.  This is
> the case for both modems with and without cancel support.

This is true but we might also get a request when we're in
USSD_STATE_USER_ACTION and then by my reading we should reply that the
user is busy i.e. send a cancel, what do you think?  I left the clause
in the patch but removed the unneeded "ussd->pending" check.

I included your other comments, thanks.

Regards

[-- Attachment #2: 0001-Handle-network-initiated-ussd-requests.patch --]
[-- Type: application/octet-stream, Size: 6042 bytes --]

From 22ffcd009b4666e7dbbef2fc6cfd3678bc7bba3f Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Mon, 15 Feb 2010 07:47:33 +0100
Subject: [PATCH] Handle network-initiated ussd requests.

This adds the methods on the D-bus interface to allow the
client to handle USSD requests from the network, according to 22.090.
Unfortunately this document is not clear on every point and some
details can't be implemented.  This includes reporting unsupported
request to the network, unsupported language, ME busy etc, because
there isn't an AT command for that.
---
 doc/ussd-api.txt |   40 +++++++++++++++++++++++
 src/ussd.c       |   91 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 129 insertions(+), 2 deletions(-)
 create mode 100644 doc/ussd-api.txt

diff --git a/doc/ussd-api.txt b/doc/ussd-api.txt
new file mode 100644
index 0000000..eff4965
--- /dev/null
+++ b/doc/ussd-api.txt
@@ -0,0 +1,40 @@
+SupplementaryServices hierarchy
+==========================
+
+Service		org.ofono
+Interface	org.ofono.SupplementaryServices
+Object path	[variable prefix]/{modem0,modem1,...}
+
+Methods		string, variant Initiate(string command)
+
+			Sends a USSD command string to the network
+			initiating a session.  When the request is handled
+			by the appropriate node of the network, the
+			method returns the response or an appropriate
+			error.  The network may be awaiting further response
+			from the ME after returning from this method and no
+			new command can be initiated until this one is
+			cancelled or ended.
+
+		void Respond(string reply)
+
+			Send a response to the network either when
+			it is awaiting further input after Initiate()
+			was called or after a network-initiated request.
+
+		void Cancel()
+
+			Cancel an ongoing USSD session, mobile- or
+			network-initiated.
+
+Signals		NotificationReceived(string message)
+
+			Signal is emitted on a network-initiated USSD
+			request for which no response is needed.
+
+		RequestReceived(string message)
+
+			Signal is emitted on a network-initiated USSD
+			request for which a response must be sent using
+			the Respond method unless it is cancelled or
+			the request is not supported.
diff --git a/src/ussd.c b/src/ussd.c
index 470634a..568e269 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -272,6 +272,11 @@ out:
 	return ret;
 }
 
+static void ussd_busy_cancel_callback(const struct ofono_error *error,
+		void *data)
+{
+}
+
 void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
@@ -327,10 +332,33 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 		else
 			ussd->state = USSD_STATE_IDLE;
 
+	} else if (ussd->state == USSD_STATE_IDLE) {
+		const char *signal_name;
+		const char *path = __ofono_atom_get_path(ussd->atom);
+
+		if (status == OFONO_USSD_STATUS_ACTION_REQUIRED) {
+			ussd->state = USSD_STATE_USER_ACTION;
+			signal_name = "RequestReceived";
+		} else {
+			ussd->state = USSD_STATE_IDLE;
+			signal_name = "NotificationReceived";
+		}
+
+		if (!str)
+			str = "";
+
+		g_dbus_emit_signal(conn, path,
+				SUPPLEMENTARY_SERVICES_INTERFACE, signal_name,
+				DBUS_TYPE_STRING, &str, DBUS_TYPE_INVALID);
+		return;
 	} else {
-		ofono_error("Received an unsolicited USSD, ignoring for now...");
+		ofono_error("Received an unsolicited USSD but can't handle...");
 		DBG("USSD is: status: %d, %s", status, str);
 
+		if (ussd->driver->cancel)
+			ussd->driver->cancel(ussd,
+					ussd_busy_cancel_callback, NULL);
+
 		return;
 	}
 
@@ -371,7 +399,7 @@ static DBusMessage *ussd_initiate(DBusConnection *conn, DBusMessage *msg,
 	if (ussd->pending)
 		return __ofono_error_busy(msg);
 
-	if (ussd->state == USSD_STATE_ACTIVE)
+	if (ussd->state != USSD_STATE_IDLE)
 		return __ofono_error_busy(msg);
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &str,
@@ -401,6 +429,61 @@ static DBusMessage *ussd_initiate(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
+static void ussd_response_callback(const struct ofono_error *error, void *data)
+{
+	struct ofono_ussd *ussd = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *reply;
+
+	if (!ussd->pending)
+		return;
+
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+		ussd->state = USSD_STATE_IDLE;
+
+		reply = dbus_message_new_method_return(ussd->pending);
+	} else {
+		DBG("ussd response failed with error: %s",
+				telephony_error_to_str(error));
+
+		reply = __ofono_error_failed(ussd->pending);
+	}
+
+	g_dbus_send_message(conn, reply);
+
+	reply = __ofono_error_failed(ussd->pending);
+	__ofono_dbus_pending_reply(&ussd->pending, reply);
+}
+
+static DBusMessage *ussd_respond(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_ussd *ussd = data;
+	const char *str;
+
+	if (ussd->pending)
+		return __ofono_error_busy(msg);
+
+	if (ussd->state != USSD_STATE_USER_ACTION)
+		return __ofono_error_not_active(msg);
+
+	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &str,
+					DBUS_TYPE_INVALID) == FALSE)
+		return __ofono_error_invalid_args(msg);
+
+	if (strlen(str) == 0)
+		return __ofono_error_invalid_format(msg);
+
+	if (!ussd->driver->request)
+		return __ofono_error_not_implemented(msg);
+
+	ussd->pending = dbus_message_ref(msg);
+
+	ussd->driver->request(ussd, str, ussd_response_callback, ussd);
+
+	return NULL;
+}
+
 static void ussd_cancel_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_ussd *ussd = data;
@@ -447,12 +530,16 @@ static DBusMessage *ussd_cancel(DBusConnection *conn, DBusMessage *msg,
 static GDBusMethodTable ussd_methods[] = {
 	{ "Initiate",	"s",	"sv",	ussd_initiate,
 					G_DBUS_METHOD_FLAG_ASYNC },
+	{ "Respond",	"s",	"",	ussd_respond,
+					G_DBUS_METHOD_FLAG_ASYNC },
 	{ "Cancel",	"",	"",	ussd_cancel,
 					G_DBUS_METHOD_FLAG_ASYNC },
 	{ }
 };
 
 static GDBusSignalTable ussd_signals[] = {
+	{ "NotificationReceived",	"s" },
+	{ "RequestReceived",		"s" },
 	{ }
 };
 
-- 
1.6.1


[-- Attachment #3: 0003-Subscribe-to-USSD-signals-in-test-script.patch --]
[-- Type: application/octet-stream, Size: 1215 bytes --]

From c924528f264bf4262cda1fc6f065da2806e6891d Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Fri, 12 Feb 2010 05:49:18 +0100
Subject: [PATCH 3/3] Subscribe to USSD signals in test script.

---
 test/test-ussd |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/test/test-ussd b/test/test-ussd
index 0ff197c..f09a9f5 100755
--- a/test/test-ussd
+++ b/test/test-ussd
@@ -6,6 +6,13 @@ import gobject
 import dbus
 import dbus.mainloop.glib
 
+def ussd_notification_received(content):
+	print("Network sent a Notification: " + content)
+
+def ussd_request_received(content):
+	print("Network sent a Request: " + content)
+	ss.Cancel()
+
 if __name__ == "__main__":
 	if (len(sys.argv) < 2):
 		print "Useage: %s <ussd-string>" % (sys.argv[0])
@@ -26,8 +33,10 @@ if __name__ == "__main__":
 	ss = dbus.Interface(bus.get_object('org.ofono', modems[0]),
 							'org.ofono.SupplementaryServices')
 
+	ss.connect_to_signal("NotificationReceived", ussd_notification_received)
+	ss.connect_to_signal("RequestReceived", ussd_request_received)
+
 	print ss.Initiate(sys.argv[1], timeout=100)
 
 	mainloop = gobject.MainLoop()
 	mainloop.run()
-
-- 
1.6.1

Re: [PATCH, rfc] Handle network-initiated ussd requests.

[Denis Kenzior]

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

Hi Andrew,

> Hi,
> 
> On 15 February 2010 18:03, Denis Kenzior <denkenz@gmail.com> wrote:
> >>       } else {
> >>               ofono_error("Received an unsolicited USSD, ignoring for
> >> now..."); DBG("USSD is: status: %d, %s", status, str);
> >>
> >> +             if (ussd->driver->respond_not_supported)
> >> +                     ussd->driver->respond_not_supported(ussd,
> >> +                                     ussd_not_supported_callback,
> >> NULL); +
> >>               return;
> >
> > There's no reason for this else clause to exist now.  The above else if
> > condition is handling this case too.  Just look carefully at ussd driver
> > for AT command modems, we always call back first, then issue ussd_notify.
> >  This is the case for both modems with and without cancel support.
> 
> This is true but we might also get a request when we're in
> USSD_STATE_USER_ACTION and then by my reading we should reply that the
> user is busy i.e. send a cancel, what do you think?  I left the clause
> in the patch but removed the unneeded "ussd->pending" check.

In everything I've researched I understood that there can't be two concurrent 
ussd sessions.  The only things that can happen during USSD_STATE_USER_ACTION:
- Network sends a timed out notification or terminates the ussd action
- someone else actually responds to the USSD (other local client has 
responded)

I think we should simply ignore the re-entrant case (which USSD operation are 
you canceling? First or second?) until we're told otherwise.  However, we 
_should_ handle the network termination / other local client has responded 
cases.

Also, it might be a good idea to have a State property that shows the current 
USSD request state.

Regards,
-Denis

Re: [PATCH, rfc] Handle network-initiated ussd requests.

[Andrzej Zaborowski]

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

Hi,

On 15 February 2010 22:24, Denis Kenzior <denkenz@gmail.com> wrote:
> In everything I've researched I understood that there can't be two concurrent
> ussd sessions.  The only things that can happen during USSD_STATE_USER_ACTION:
> - Network sends a timed out notification or terminates the ussd action
> - someone else actually responds to the USSD (other local client has
> responded)
>
> I think we should simply ignore the re-entrant case (which USSD operation are
> you canceling? First or second?) until we're told otherwise.  However, we
> _should_ handle the network termination / other local client has responded
> cases.
>
> Also, it might be a good idea to have a State property that shows the current
> USSD request state.

Ok, you're definitely right.  This new patch ignores the new USSD if
we're in USSD_STATE_USER_ACTION already.  I also added a USSDState
property as string.

Regards

[-- Attachment #2: 0001-Handle-network-initiated-ussd-requests.patch --]
[-- Type: application/octet-stream, Size: 9510 bytes --]

From 6c92ff58242a5c0a599437753ce197d30070a200 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Mon, 15 Feb 2010 11:05:16 +0100
Subject: [PATCH 12/13] Handle network-initiated ussd requests.

This adds the methods on the D-bus interface to allow the
client to handle USSD requests from the network, according to 22.090.
Unfortunately this document is not clear on every point and some
details can't be implemented.  This includes reporting unsupported
request to the network, unsupported language, ME busy etc, because
there isn't an AT command for that.
---
 doc/ussd-api.txt |   64 +++++++++++++++++++++
 src/ussd.c       |  162 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 216 insertions(+), 10 deletions(-)
 create mode 100644 doc/ussd-api.txt

diff --git a/doc/ussd-api.txt b/doc/ussd-api.txt
new file mode 100644
index 0000000..83d3cee
--- /dev/null
+++ b/doc/ussd-api.txt
@@ -0,0 +1,64 @@
+SupplementaryServices hierarchy
+==========================
+
+Service		org.ofono
+Interface	org.ofono.SupplementaryServices
+Object path	[variable prefix]/{modem0,modem1,...}
+
+Methods		string, variant Initiate(string command)
+
+			Sends a USSD command string to the network
+			initiating a session.  When the request is handled
+			by the appropriate node of the network, the
+			method returns the response or an appropriate
+			error.  The network may be awaiting further response
+			from the ME after returning from this method and no
+			new command can be initiated until this one is
+			cancelled or ended.
+
+		void Respond(string reply)
+
+			Send a response to the network either when
+			it is awaiting further input after Initiate()
+			was called or after a network-initiated request.
+
+		void Cancel()
+
+			Cancel an ongoing USSD session, mobile- or
+			network-initiated.
+
+		dict GetProperties()
+
+			Returns Supplementary Services related properties. See
+			the properties section for available properties.
+
+Signals		NotificationReceived(string message)
+
+			Signal is emitted on a network-initiated USSD
+			request for which no response is needed.
+
+		RequestReceived(string message)
+
+			Signal is emitted on a network-initiated USSD
+			request for which a response must be sent using
+			the Respond method unless it is cancelled or
+			the request is not supported.
+
+		PropertyChanged(string property, variant value)
+
+			Signal is emitted whenever a property has changed.
+			The new value is passed as the signal argument.
+
+Properties	string USSDState [readonly]
+
+			Reflects the state of current USSD session.  The
+			values have the following meanings:
+
+			"idle"			 No active USSD session.
+			"active"		 A session is active between the
+						 network and the ME, the ME is
+						 waiting for a reply from the
+						 network.
+			"awaiting-user-response" The network is waiting for the
+						 user's response, client must
+						 call Respond().
diff --git a/src/ussd.c b/src/ussd.c
index 470634a..9933533 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -272,6 +272,37 @@ out:
 	return ret;
 }
 
+static const char *ussd_get_state_string(struct ofono_ussd *ussd)
+{
+	switch (ussd->state) {
+	case USSD_STATE_IDLE:
+		return "idle";
+	case USSD_STATE_ACTIVE:
+		return "active";
+	case USSD_STATE_USER_ACTION:
+		return "awaiting-user-response";
+	}
+
+	return "";
+}
+
+static void ussd_change_state(struct ofono_ussd *ussd, int state)
+{
+	const char *value;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(ussd->atom);
+
+	if (state == ussd->state)
+		return;
+
+	ussd->state = state;
+
+	value = ussd_get_state_string(ussd);
+	ofono_dbus_signal_property_changed(conn, path,
+			SUPPLEMENTARY_SERVICES_INTERFACE,
+			"USSDState", DBUS_TYPE_STRING, &value);
+}
+
 void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
@@ -282,7 +313,7 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 	DBusMessageIter variant;
 
 	if (status == OFONO_USSD_STATUS_NOT_SUPPORTED) {
-		ussd->state = USSD_STATE_IDLE;
+		ussd_change_state(ussd, USSD_STATE_IDLE);
 
 		if (!ussd->pending)
 			return;
@@ -292,7 +323,7 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 	}
 
 	if (status == OFONO_USSD_STATUS_TIMED_OUT) {
-		ussd->state = USSD_STATE_IDLE;
+		ussd_change_state(ussd, USSD_STATE_IDLE);
 
 		if (!ussd->pending)
 			return;
@@ -323,12 +354,34 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, const char *str)
 		dbus_message_iter_close_container(&iter, &variant);
 
 		if (status == OFONO_USSD_STATUS_ACTION_REQUIRED)
-			ussd->state = USSD_STATE_USER_ACTION;
+			ussd_change_state(ussd, USSD_STATE_USER_ACTION);
 		else
-			ussd->state = USSD_STATE_IDLE;
+			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;
+
+		if (status == OFONO_USSD_STATUS_ACTION_REQUIRED) {
+			new_state = USSD_STATE_USER_ACTION;
+			signal_name = "RequestReceived";
+		} else {
+			new_state = USSD_STATE_IDLE;
+			signal_name = "NotificationReceived";
+		}
+
+		if (!str)
+			str = "";
+
+		g_dbus_emit_signal(conn, path,
+				SUPPLEMENTARY_SERVICES_INTERFACE, signal_name,
+				DBUS_TYPE_STRING, &str, DBUS_TYPE_INVALID);
 
+		ussd_change_state(ussd, new_state);
+		return;
 	} else {
-		ofono_error("Received an unsolicited USSD, ignoring for now...");
+		ofono_error("Received an unsolicited USSD but can't handle.");
 		DBG("USSD is: status: %d, %s", status, str);
 
 		return;
@@ -351,7 +404,7 @@ static void ussd_callback(const struct ofono_error *error, void *data)
 				telephony_error_to_str(error));
 
 	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
-		ussd->state = USSD_STATE_ACTIVE;
+		ussd_change_state(ussd, USSD_STATE_ACTIVE);
 		return;
 	}
 
@@ -371,7 +424,7 @@ static DBusMessage *ussd_initiate(DBusConnection *conn, DBusMessage *msg,
 	if (ussd->pending)
 		return __ofono_error_busy(msg);
 
-	if (ussd->state == USSD_STATE_ACTIVE)
+	if (ussd->state != USSD_STATE_IDLE)
 		return __ofono_error_busy(msg);
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &str,
@@ -401,12 +454,67 @@ static DBusMessage *ussd_initiate(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
+static void ussd_response_callback(const struct ofono_error *error, void *data)
+{
+	struct ofono_ussd *ussd = data;
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *reply;
+
+	if (!ussd->pending)
+		return;
+
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+		ussd_change_state(ussd, USSD_STATE_IDLE);
+
+		reply = dbus_message_new_method_return(ussd->pending);
+	} else {
+		DBG("ussd response failed with error: %s",
+				telephony_error_to_str(error));
+
+		reply = __ofono_error_failed(ussd->pending);
+	}
+
+	g_dbus_send_message(conn, reply);
+
+	reply = __ofono_error_failed(ussd->pending);
+	__ofono_dbus_pending_reply(&ussd->pending, reply);
+}
+
+static DBusMessage *ussd_respond(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_ussd *ussd = data;
+	const char *str;
+
+	if (ussd->pending)
+		return __ofono_error_busy(msg);
+
+	if (ussd->state != USSD_STATE_USER_ACTION)
+		return __ofono_error_not_active(msg);
+
+	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &str,
+					DBUS_TYPE_INVALID) == FALSE)
+		return __ofono_error_invalid_args(msg);
+
+	if (strlen(str) == 0)
+		return __ofono_error_invalid_format(msg);
+
+	if (!ussd->driver->request)
+		return __ofono_error_not_implemented(msg);
+
+	ussd->pending = dbus_message_ref(msg);
+
+	ussd->driver->request(ussd, str, ussd_response_callback, ussd);
+
+	return NULL;
+}
+
 static void ussd_cancel_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_ussd *ussd = data;
 	DBusMessage *reply;
 
-	ussd->state = USSD_STATE_IDLE;
+	ussd_change_state(ussd, USSD_STATE_IDLE);
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		DBG("ussd cancel failed with error: %s",
@@ -444,15 +552,49 @@ static DBusMessage *ussd_cancel(DBusConnection *conn, DBusMessage *msg,
 	return NULL;
 }
 
+static DBusMessage *ussd_get_properties(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_ussd *ussd = data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	const char *value;
+
+	reply = dbus_message_new_method_return(msg);
+	if (!reply)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					OFONO_PROPERTIES_ARRAY_SIGNATURE,
+					&dict);
+
+	value = ussd_get_state_string(ussd);
+	ofono_dbus_dict_append(&dict, "USSDState", DBUS_TYPE_STRING, &value);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
 static GDBusMethodTable ussd_methods[] = {
-	{ "Initiate",	"s",	"sv",	ussd_initiate,
+	{ "Initiate",		"s",	"sv",		ussd_initiate,
+					G_DBUS_METHOD_FLAG_ASYNC },
+	{ "Respond",		"s",	"",		ussd_respond,
 					G_DBUS_METHOD_FLAG_ASYNC },
-	{ "Cancel",	"",	"",	ussd_cancel,
+	{ "Cancel",		"",	"",		ussd_cancel,
 					G_DBUS_METHOD_FLAG_ASYNC },
+	{ "GetProperties",	"",	"a{sv}",	ussd_get_properties,
+					0 },
 	{ }
 };
 
 static GDBusSignalTable ussd_signals[] = {
+	{ "NotificationReceived",	"s" },
+	{ "RequestReceived",		"s" },
+	{ "PropertyChanged",		"sv" },
 	{ }
 };
 
-- 
1.6.1


[-- Attachment #3: 0003-Subscribe-to-USSD-signals-in-test-script.patch --]
[-- Type: application/octet-stream, Size: 1500 bytes --]

From ee058468cfa89140d7a3247387cdeed9c56252c4 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Mon, 15 Feb 2010 11:05:40 +0100
Subject: [PATCH 13/13] Subscribe to USSD signals in test script.

---
 test/test-ussd |   21 ++++++++++++++++++++-
 1 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/test/test-ussd b/test/test-ussd
index 0ff197c..656c73e 100755
--- a/test/test-ussd
+++ b/test/test-ussd
@@ -6,6 +6,18 @@ import gobject
 import dbus
 import dbus.mainloop.glib
 
+def ussd_notification_received(content):
+	print("Network sent a Notification: " + content)
+
+def ussd_request_received(content):
+	print("Network sent a Request: " + content)
+	ss.Cancel()
+
+def ussd_property_changed(name, value):
+	if name != "USSDState":
+		return
+	print("USSD session state is " + value)
+
 if __name__ == "__main__":
 	if (len(sys.argv) < 2):
 		print "Useage: %s <ussd-string>" % (sys.argv[0])
@@ -26,8 +38,15 @@ if __name__ == "__main__":
 	ss = dbus.Interface(bus.get_object('org.ofono', modems[0]),
 							'org.ofono.SupplementaryServices')
 
+	props = ss.GetProperties()
+	for p in props:
+		ussd_property_changed(p, props[p])
+
+	ss.connect_to_signal("NotificationReceived", ussd_notification_received)
+	ss.connect_to_signal("RequestReceived", ussd_request_received)
+	ss.connect_to_signal("PropertyChanged", ussd_property_changed)
+
 	print ss.Initiate(sys.argv[1], timeout=100)
 
 	mainloop = gobject.MainLoop()
 	mainloop.run()
-
-- 
1.6.1

Re: [PATCH, rfc] Handle network-initiated ussd requests.

[Denis Kenzior]

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

Hi Andrew,

These patches have been applied with minor tweaks afterward.

Regards,
-Denis