[PATCH] doc: Play Tone command proposed d-bus API.

[PATCH] doc: Play Tone command proposed d-bus API.

[Andrzej Zaborowski]

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

This is a version with the tone identified by a string based on irc
discussion, I leave the decision to you about which version to use.
---
 doc/stk-api.txt |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/doc/stk-api.txt b/doc/stk-api.txt
index d0dd740..14819a7 100644
--- a/doc/stk-api.txt
+++ b/doc/stk-api.txt
@@ -200,6 +200,38 @@ Methods		byte RequestSelection(string title, byte icon_id,
 
 			Possible Errors: [service].Error.SimToolkit.EndSession
 
+		void PlayTone(string tone, boolean loop,
+				string text, byte icon_id)
+
+			Tells the agent to play an audio tone.  A repeatable
+			tone is played for the duration of the method call
+			(until it is cancelled), otherwise it is played once
+			and the method should return.  The text parameter
+			contains an optional text to be displayed to the
+			user.  The following tones are defined:
+				"dial tone"
+				"busy"
+				"congestion"
+				"radio path acknowledge"
+				"radio path not available"
+				"error"
+				"call waiting"
+				"ringing tone"
+				"general beep"
+				"positive acknowledgement"
+				"negative acknowledgement"
+				"user ringing tone"
+				"user sms alert"
+				"critical" (high priority)
+				"vibrate"
+				"happy"
+				"sad"
+				"urgent action"
+				"question"
+				"message received"
+
+			Possible Errors: [service].Error.SimToolkit.EndSession
+
 		void Cancel()
 
 			Asks the agent to cancel any ongoing operation in
-- 
1.7.1.86.g0e460.dirty

[PATCH 1/2][RfC] voicecall: __ofono_voicecall_send_tone internal api.

[Andrzej Zaborowski]

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

This provides a way for other atoms to send DTMF tones during a call.
---
 src/ofono.h     |    6 +++
 src/voicecall.c |  136 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+), 0 deletions(-)

diff --git a/src/ofono.h b/src/ofono.h
index d95f2f2..ce734c5 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -182,6 +182,7 @@ enum ofono_voicecall_interaction {
 };
 
 typedef void (*ofono_voicecall_dial_cb_t)(struct ofono_call *call, void *data);
+typedef void (*ofono_voicecall_tone_cb_t)(int error, void *data);
 
 ofono_bool_t __ofono_voicecall_is_busy(struct ofono_voicecall *vc,
 					enum ofono_voicecall_interaction type);
@@ -193,6 +194,11 @@ int __ofono_voicecall_dial(struct ofono_voicecall *vc,
 				ofono_voicecall_dial_cb_t cb, void *user_data);
 void __ofono_voicecall_dial_cancel(struct ofono_voicecall *vc);
 
+int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
+				const char *tone_str,
+				ofono_voicecall_tone_cb_t cb, void *user_data);
+void __ofono_voicecall_tone_cancel(struct ofono_voicecall *vc);
+
 #include <ofono/sms.h>
 
 struct sms;
diff --git a/src/voicecall.c b/src/voicecall.c
index a5ee2ed..3bd4dff 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -59,6 +59,7 @@ struct ofono_voicecall {
 	void *driver_data;
 	struct ofono_atom *atom;
 	struct dial_request *dial_req;
+	struct tone_request *tone_req;
 };
 
 struct voicecall {
@@ -82,12 +83,22 @@ struct dial_request {
 	struct ofono_phone_number ph;
 };
 
+struct tone_request {
+	char *tone_str;
+	char *left;
+	int delay;
+	guint source;
+	ofono_voicecall_tone_cb_t cb;
+	void *user_data;
+};
+
 static const char *default_en_list[] = { "911", "112", NULL };
 static const char *default_en_list_no_sim[] = { "119", "118", "999", "110",
 						"08", "000", NULL };
 
 static void generic_callback(const struct ofono_error *error, void *data);
 static void multirelease_callback(const struct ofono_error *err, void *data);
+static gboolean tone_request_run(gpointer user_data);
 
 static gint call_compare_by_id(gconstpointer a, gconstpointer b)
 {
@@ -229,6 +240,22 @@ static void dial_request_finish(struct ofono_voicecall *vc, gboolean callback)
 	vc->dial_req = NULL;
 }
 
+static void tone_request_finish(struct ofono_voicecall *vc, int error)
+{
+	struct tone_request *tone_req = vc->tone_req;
+
+	vc->tone_req = NULL;
+
+	if (tone_req->cb)
+		tone_req->cb(-error, tone_req->user_data);
+
+	if (tone_req->source)
+		g_source_remove(tone_req->source);
+
+	g_free(tone_req->tone_str);
+	g_free(tone_req);
+}
+
 static void append_voicecall_properties(struct voicecall *v,
 					DBusMessageIter *dict)
 {
@@ -574,6 +601,9 @@ static void voicecall_set_call_status(struct voicecall *call, int status)
 	if (status == CALL_STATUS_DISCONNECTED && call->vc->dial_req &&
 			call == call->vc->dial_req->call)
 		dial_request_finish(call->vc, TRUE);
+
+	if (old_status == CALL_STATUS_ACTIVE && call->vc->tone_req)
+		tone_request_finish(call->vc, ENOENT);
 }
 
 static void voicecall_set_call_lineid(struct voicecall *v,
@@ -1956,6 +1986,9 @@ static void voicecall_unregister(struct ofono_atom *atom)
 	if (vc->dial_req)
 		dial_request_finish(vc, TRUE);
 
+	if (vc->tone_req)
+		tone_request_finish(vc, ENOENT);
+
 	for (l = vc->call_list; l; l = l->next)
 		voicecall_dbus_unregister(vc, l->data);
 
@@ -2311,3 +2344,106 @@ void __ofono_voicecall_dial_cancel(struct ofono_voicecall *vc)
 
 	vc->dial_req->cb = NULL;
 }
+
+static void tone_request_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_voicecall *vc = data;
+
+	if (!vc->tone_req)
+		return;
+
+	if (error && error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		DBG("command failed with error: %s",
+				telephony_error_to_str(error));
+
+		tone_request_finish(vc, EIO);
+		return;
+	}
+
+	vc->tone_req->source = g_timeout_add_seconds(vc->tone_req->delay,
+							tone_request_run, vc);
+}
+
+static gboolean tone_request_run(gpointer user_data)
+{
+	struct ofono_voicecall *vc = user_data;
+	static const char *dtmf = "0123456789ABDEFabdef";
+	char tones[256], *i;
+
+	if (!vc->tone_req)
+		return FALSE;
+
+	vc->tone_req->source = 0;
+
+	if (*vc->tone_req->left == '\0') {
+		tone_request_finish(vc, 0);
+		return FALSE;
+	}
+
+	i = tones;
+	vc->tone_req->delay = 0;
+
+	/* Wait 3 seconds per tone */
+	while (*vc->tone_req->left && strchr(dtmf, *vc->tone_req->left)) {
+		*i++ = *vc->tone_req->left++;
+		vc->tone_req->delay += 3;
+	}
+
+	while (*vc->tone_req->left && *vc->tone_req->left != 'c') {
+		tone_request_finish(vc, EINVAL);
+		return FALSE;
+	}
+
+	while (*vc->tone_req->left == 'c') {
+		vc->tone_req->left++;
+		vc->tone_req->delay += 3;
+	}
+
+	if (i > tones) {
+		*i = '\0';
+
+		vc->driver->send_tones(vc, tones, tone_request_cb, vc);
+	} else
+		tone_request_cb(NULL, vc);
+
+	return FALSE;
+}
+
+int __ofono_voicecall_send_tone(struct ofono_voicecall *vc,
+				const char *tone_str,
+				ofono_voicecall_tone_cb_t cb, void *user_data)
+{
+	struct tone_request *req;
+
+	if (!vc->driver->send_tones)
+		return -ENOSYS;
+
+	/* Send DTMFs only if we have at least one connected call */
+	if (!voicecalls_can_dtmf(vc))
+		return -ENOENT;
+
+	if (vc->tone_req)
+		return -EBUSY;
+
+	req = g_try_new0(struct tone_request, 1);
+	req->tone_str = g_strdup(tone_str);
+	req->left = req->tone_str;
+	req->cb = cb;
+	req->user_data = user_data;
+
+	vc->tone_req = req;
+
+	tone_request_run(vc);
+
+	return 0;
+}
+
+void __ofono_voicecall_tone_cancel(struct ofono_voicecall *vc)
+{
+	if (!vc->tone_req || !vc->tone_req->cb)
+		return;
+
+	vc->tone_req->cb = NULL;
+
+	tone_request_finish(vc, 0);
+}
-- 
1.7.1.86.g0e460.dirty

[PATCH 2/2] stk: Handle the Send DTMF proactive command.

[Andrzej Zaborowski]

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

---
 src/stk.c |  136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 135 insertions(+), 1 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 3fda2af..b469467 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -434,8 +434,12 @@ static void default_agent_notify(gpointer user_data)
 {
 	struct ofono_stk *stk = user_data;
 
-	if (stk->current_agent == stk->default_agent && stk->respond_on_exit)
+	if (stk->current_agent == stk->default_agent && stk->respond_on_exit) {
+		if (stk->pending_cmd)
+			stk->cancel_cmd(stk);
+
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
+	}
 
 	stk->default_agent = NULL;
 	stk->current_agent = stk->session_agent;
@@ -449,6 +453,9 @@ static void session_agent_notify(gpointer user_data)
 	DBG("Session Agent removed");
 
 	if (stk->current_agent == stk->session_agent && stk->respond_on_exit) {
+		if (stk->pending_cmd)
+			stk->cancel_cmd(stk);
+
 		DBG("Sending Terminate response for session agent");
 		send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
 	}
@@ -1590,6 +1597,128 @@ static gboolean handle_command_set_up_call(const struct stk_command *cmd,
 	return FALSE;
 }
 
+static void send_dtmf_cancel(struct ofono_stk *stk)
+{
+	struct ofono_voicecall *vc;
+	struct ofono_atom *vc_atom;
+
+	stk->respond_on_exit = FALSE;
+
+	if (stk->pending_cmd->send_dtmf.alpha_id &&
+			stk->pending_cmd->send_dtmf.alpha_id[0])
+		stk_alpha_id_unset(stk);
+
+	vc_atom = __ofono_modem_find_atom(__ofono_atom_get_modem(stk->atom),
+						OFONO_ATOM_TYPE_VOICECALL);
+	if (!vc_atom)
+		return;
+
+	vc = __ofono_atom_get_data(vc_atom);
+	if (vc)
+		__ofono_voicecall_tone_cancel(vc);
+}
+
+static void dtmf_sent_cb(int err, void *user_data)
+{
+	struct ofono_stk *stk = user_data;
+
+	stk->respond_on_exit = FALSE;
+
+	if (stk->pending_cmd->send_dtmf.alpha_id &&
+			stk->pending_cmd->send_dtmf.alpha_id[0])
+		stk_alpha_id_unset(stk);
+
+	if (err == -ENOENT) {
+		struct stk_response rsp;
+		static unsigned char not_in_speech_call_result[] = { 0x07 };
+		static struct ofono_error error =
+			{ .type = OFONO_ERROR_TYPE_FAILURE };
+
+		memset(&rsp, 0, sizeof(rsp));
+
+		rsp.result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		rsp.result.additional_len = sizeof(not_in_speech_call_result);
+		rsp.result.additional = not_in_speech_call_result;
+
+		if (stk_respond(stk, &rsp, stk_command_cb))
+			stk_command_cb(&error, stk);
+
+		return;
+	}
+
+	if (err == -EINVAL) {
+		send_simple_response(stk, STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD);
+		return;
+	}
+
+	if (err == 0)
+		send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
+	else
+		send_simple_response(stk, STK_RESULT_TYPE_NOT_CAPABLE);
+}
+
+static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
+						struct stk_response *rsp,
+						struct ofono_stk *stk)
+{
+	static unsigned char not_in_speech_call_result[] = { 0x07 };
+	struct ofono_voicecall *vc = NULL;
+	struct ofono_atom *vc_atom;
+	int err;
+
+	vc_atom = __ofono_modem_find_atom(__ofono_atom_get_modem(stk->atom),
+						OFONO_ATOM_TYPE_VOICECALL);
+	if (vc_atom)
+		vc = __ofono_atom_get_data(vc_atom);
+
+	if (!vc) {
+		rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+		return TRUE;
+	}
+
+	err = __ofono_voicecall_send_tone(vc, cmd->send_dtmf.dtmf,
+						dtmf_sent_cb, stk);
+
+	if (err == -EBUSY) {
+		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		return TRUE;
+	}
+
+	if (err == -ENOSYS) {
+		rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+		return TRUE;
+	}
+
+	if (err == -ENOENT) {
+		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		rsp->result.additional_len = sizeof(not_in_speech_call_result);
+		rsp->result.additional = not_in_speech_call_result;
+		return TRUE;
+	}
+
+	if (err < 0) {
+		/*
+		 * We most likely got an out of memory error, tell SIM
+		 * to retry
+		 */
+		rsp->result.type = STK_RESULT_TYPE_TERMINAL_BUSY;
+		return TRUE;
+	}
+
+	if (cmd->send_dtmf.alpha_id && cmd->send_dtmf.alpha_id[0])
+		stk_alpha_id_set(stk, cmd->send_dtmf.alpha_id);
+
+	/*
+	 * Note that we don't strictly require an agent to be connected,
+	 * but to comply with 6.4.24 we need to send a End Session when
+	 * the user decides so.
+	 */
+	stk->respond_on_exit = TRUE;
+	stk->cancel_cmd = send_dtmf_cancel;
+
+	return FALSE;
+}
+
 static void stk_proactive_command_cancel(struct ofono_stk *stk)
 {
 	if (stk->immediate_response)
@@ -1741,6 +1870,11 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
 							&rsp, stk);
 		break;
 
+	case STK_COMMAND_TYPE_SEND_DTMF:
+		respond = handle_command_send_dtmf(stk->pending_cmd,
+							&rsp, stk);
+		break;
+
 	default:
 		rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
 		break;
-- 
1.7.1.86.g0e460.dirty

[PATCH] Fix sending User Cancel response to Set Up Call.

[Andrzej Zaborowski]

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

One of the clean-up commits changed the semantics of the dial request
callback's parameter (NULL if call setup failed, non-NULL if success
or user cancelled).
---
 src/stk.c       |    2 +-
 src/voicecall.c |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index b469467..b6ed4c3 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1419,7 +1419,7 @@ static void call_setup_connected(struct ofono_call *call, void *data)
 	static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
 	static unsigned char facility_rejected_result[] = { 0x9d };
 
-	if (!call) {
+	if (!call || call->status == CALL_STATUS_DISCONNECTED) {
 		memset(&rsp, 0, sizeof(rsp));
 
 		rsp.result.type = STK_RESULT_TYPE_NETWORK_UNAVAILABLE;
diff --git a/src/voicecall.c b/src/voicecall.c
index 3bd4dff..e536089 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -361,6 +361,16 @@ static DBusMessage *voicecall_deflect(DBusConnection *conn,
 	return NULL;
 }
 
+static void dial_request_user_cancel(struct ofono_voicecall *vc,
+					struct voicecall *call)
+{
+	if (!vc->dial_req)
+		return;
+
+	if (!call || call == vc->dial_req->call)
+		dial_request_finish(vc->dial_req->call->vc, TRUE);
+}
+
 static DBusMessage *voicecall_hangup(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
@@ -372,6 +382,8 @@ static DBusMessage *voicecall_hangup(DBusConnection *conn,
 	if (vc->pending)
 		return __ofono_error_busy(msg);
 
+	dial_request_user_cancel(vc, v);
+
 	switch (call->status) {
 	case CALL_STATUS_DISCONNECTED:
 		return __ofono_error_failed(msg);
@@ -1278,6 +1290,8 @@ static DBusMessage *manager_hangup_all(DBusConnection *conn,
 		return reply;
 	}
 
+	dial_request_user_cancel(vc, NULL);
+
 	vc->flags |= VOICECALLS_FLAG_MULTI_RELEASE;
 
 	vc->pending = dbus_message_ref(msg);
-- 
1.7.1.86.g0e460.dirty

Re: [PATCH] Fix sending User Cancel response to Set Up Call.

[Denis Kenzior]

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

Hi Andrew,

On 09/08/2010 12:59 PM, Andrzej Zaborowski wrote:
> One of the clean-up commits changed the semantics of the dial request
> callback's parameter (NULL if call setup failed, non-NULL if success
> or user cancelled).
> ---
>  src/stk.c       |    2 +-
>  src/voicecall.c |   14 ++++++++++++++
>  2 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/src/stk.c b/src/stk.c
> index b469467..b6ed4c3 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -1419,7 +1419,7 @@ static void call_setup_connected(struct ofono_call *call, void *data)
>  	static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
>  	static unsigned char facility_rejected_result[] = { 0x9d };
>  
> -	if (!call) {
> +	if (!call || call->status == CALL_STATUS_DISCONNECTED) {

If the call has been disconnected by the user, shouldn't we be sending
USER_CANCEL?  At least that is what the old behavior implies.

>  		memset(&rsp, 0, sizeof(rsp));
>  
>  		rsp.result.type = STK_RESULT_TYPE_NETWORK_UNAVAILABLE;
> diff --git a/src/voicecall.c b/src/voicecall.c
> index 3bd4dff..e536089 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -361,6 +361,16 @@ static DBusMessage *voicecall_deflect(DBusConnection *conn,
>  	return NULL;
>  }
>  
> +static void dial_request_user_cancel(struct ofono_voicecall *vc,
> +					struct voicecall *call)
> +{
> +	if (!vc->dial_req)
> +		return;
> +
> +	if (!call || call == vc->dial_req->call)
> +		dial_request_finish(vc->dial_req->call->vc, TRUE);
> +}
> +
>  static DBusMessage *voicecall_hangup(DBusConnection *conn,
>  					DBusMessage *msg, void *data)
>  {
> @@ -372,6 +382,8 @@ static DBusMessage *voicecall_hangup(DBusConnection *conn,
>  	if (vc->pending)
>  		return __ofono_error_busy(msg);
>  
> +	dial_request_user_cancel(vc, v);
> +

I don't see the need for this part, don't we already take care of this
in voicecall_set_call_status?

>  	switch (call->status) {
>  	case CALL_STATUS_DISCONNECTED:
>  		return __ofono_error_failed(msg);
> @@ -1278,6 +1290,8 @@ static DBusMessage *manager_hangup_all(DBusConnection *conn,
>  		return reply;
>  	}
>  
> +	dial_request_user_cancel(vc, NULL);
> +

Same as above...

>  	vc->flags |= VOICECALLS_FLAG_MULTI_RELEASE;
>  
>  	vc->pending = dbus_message_ref(msg);

Regards,
-Denis

Re: [PATCH] doc: Play Tone command proposed d-bus API.

[Marcel Holtmann]

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

Hi Andrew,

> This is a version with the tone identified by a string based on irc
> discussion, I leave the decision to you about which version to use.
> ---
>  doc/stk-api.txt |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/stk-api.txt b/doc/stk-api.txt
> index d0dd740..14819a7 100644
> --- a/doc/stk-api.txt
> +++ b/doc/stk-api.txt
> @@ -200,6 +200,38 @@ Methods		byte RequestSelection(string title, byte icon_id,
>  
>  			Possible Errors: [service].Error.SimToolkit.EndSession
>  
> +		void PlayTone(string tone, boolean loop,
> +				string text, byte icon_id)
> +
> +			Tells the agent to play an audio tone.  A repeatable
> +			tone is played for the duration of the method call
> +			(until it is cancelled), otherwise it is played once
> +			and the method should return.  The text parameter
> +			contains an optional text to be displayed to the
> +			user.  The following tones are defined:

string sounds here more than reasonable. For the looping part we have to
clarify when to return from this method. For non repeating tones it is
pretty clear. It should return when playing the tone finished. For the
repeating one we can return right away, or after the first iteration has
played. That needs to be properly described in what to expect from the
agent.

Also has PlayTone and PlayLoop method calls considered to have a clearer
semantic separation?

> +				"dial tone"
> +				"busy"
> +				"congestion"
> +				"radio path acknowledge"
> +				"radio path not available"
> +				"error"
> +				"call waiting"
> +				"ringing tone"
> +				"general beep"
> +				"positive acknowledgement"
> +				"negative acknowledgement"
> +				"user ringing tone"
> +				"user sms alert"
> +				"critical" (high priority)
> +				"vibrate"
> +				"happy"
> +				"sad"
> +				"urgent action"
> +				"question"
> +				"message received"

I prefer to have them as "call-waiting" with a - in between. Space are
sort of a bad idea for constant type strings. And we do similar things
for "cs-preferred" etc. But essentially I agree that strings are way
better.

Can we also fix the typo in acknowledgment ;)

The alternate text string comes from the SIM Toolkit, right?

Regards

Marcel

Re: [PATCH] Fix sending User Cancel response to Set Up Call.

[Andrzej Zaborowski]

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

On 9 September 2010 16:37, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Andrew,
>
> On 09/08/2010 12:59 PM, Andrzej Zaborowski wrote:
>> One of the clean-up commits changed the semantics of the dial request
>> callback's parameter (NULL if call setup failed, non-NULL if success
>> or user cancelled).
>> ---
>>  src/stk.c       |    2 +-
>>  src/voicecall.c |   14 ++++++++++++++
>>  2 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/stk.c b/src/stk.c
>> index b469467..b6ed4c3 100644
>> --- a/src/stk.c
>> +++ b/src/stk.c
>> @@ -1419,7 +1419,7 @@ static void call_setup_connected(struct ofono_call *call, void *data)
>>       static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
>>       static unsigned char facility_rejected_result[] = { 0x9d };
>>
>> -     if (!call) {
>> +     if (!call || call->status == CALL_STATUS_DISCONNECTED) {
>
> If the call has been disconnected by the user, shouldn't we be sending
> USER_CANCEL?  At least that is what the old behavior implies.

In that case yes, but if it's disconnected because connecting failed,
then we report this to the card.

I noticed that http://git.kernel.org/?p=network/ofono/ofono.git;a=commitdiff;h=ed562ba2b04b56b9a688d40906bb783027c4a1e6
changed the behaviour slightly (but the old behaviour was probably
wrong too).

>
>>               memset(&rsp, 0, sizeof(rsp));
>>
>>               rsp.result.type = STK_RESULT_TYPE_NETWORK_UNAVAILABLE;
>> diff --git a/src/voicecall.c b/src/voicecall.c
>> index 3bd4dff..e536089 100644
>> --- a/src/voicecall.c
>> +++ b/src/voicecall.c
>> @@ -361,6 +361,16 @@ static DBusMessage *voicecall_deflect(DBusConnection *conn,
>>       return NULL;
>>  }
>>
>> +static void dial_request_user_cancel(struct ofono_voicecall *vc,
>> +                                     struct voicecall *call)
>> +{
>> +     if (!vc->dial_req)
>> +             return;
>> +
>> +     if (!call || call == vc->dial_req->call)
>> +             dial_request_finish(vc->dial_req->call->vc, TRUE);
>> +}
>> +
>>  static DBusMessage *voicecall_hangup(DBusConnection *conn,
>>                                       DBusMessage *msg, void *data)
>>  {
>> @@ -372,6 +382,8 @@ static DBusMessage *voicecall_hangup(DBusConnection *conn,
>>       if (vc->pending)
>>               return __ofono_error_busy(msg);
>>
>> +     dial_request_user_cancel(vc, v);
>> +
>
> I don't see the need for this part, don't we already take care of this
> in voicecall_set_call_status?

At that point we can't tell where it was a network error / busy signal
/ etc, or the user cancelled the call (which has a different result
value sent to sim).

>
>>       switch (call->status) {
>>       case CALL_STATUS_DISCONNECTED:
>>               return __ofono_error_failed(msg);
>> @@ -1278,6 +1290,8 @@ static DBusMessage *manager_hangup_all(DBusConnection *conn,
>>               return reply;
>>       }
>>
>> +     dial_request_user_cancel(vc, NULL);
>> +
>
> Same as above...
>
>>       vc->flags |= VOICECALLS_FLAG_MULTI_RELEASE;
>>
>>       vc->pending = dbus_message_ref(msg);
>
> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>

Regards,
Andrew

Re: [PATCH] doc: Play Tone command proposed d-bus API.

[andrzej zaborowski]

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

Hi,

On 9 September 2010 18:23, Marcel Holtmann <marcel@holtmann.org> wrote:
>> +                     Tells the agent to play an audio tone.  A repeatable
>> +                     tone is played for the duration of the method call
>> +                     (until it is cancelled), otherwise it is played once
>> +                     and the method should return.  The text parameter
>> +                     contains an optional text to be displayed to the
>> +                     user.  The following tones are defined:
>
> string sounds here more than reasonable. For the looping part we have to
> clarify when to return from this method. For non repeating tones it is
> pretty clear. It should return when playing the tone finished. For the
> repeating one we can return right away, or after the first iteration has
> played. That needs to be properly described in what to expect from the
> agent.

My idea is for it to wait until the call is cancelled, and not return.
 Otherwise we'll have to send the duration in a 0.1s units.

>
> Also has PlayTone and PlayLoop method calls considered to have a clearer
> semantic separation?

That's a good idea, I'll change it to use the separate methods unless
someone objects.

>
>> +                             "dial tone"
>> +                             "busy"
>> +                             "congestion"
>> +                             "radio path acknowledge"
>> +                             "radio path not available"
>> +                             "error"
>> +                             "call waiting"
>> +                             "ringing tone"
>> +                             "general beep"
>> +                             "positive acknowledgement"
>> +                             "negative acknowledgement"
>> +                             "user ringing tone"
>> +                             "user sms alert"
>> +                             "critical" (high priority)
>> +                             "vibrate"
>> +                             "happy"
>> +                             "sad"
>> +                             "urgent action"
>> +                             "question"
>> +                             "message received"
>
> I prefer to have them as "call-waiting" with a - in between. Space are
> sort of a bad idea for constant type strings. And we do similar things
> for "cs-preferred" etc.

Ok.

> Can we also fix the typo in acknowledgment ;)

I think I copied it from the spec (I'll check), the interwebs claims
both forms are correct :)

> The alternate text string comes from the SIM Toolkit, right?

Yes, it's actually the AlphaId / Icon to be displayed *while* playing,
not instead.

Regards,
Andrew

Re: [PATCH] Fix sending User Cancel response to Set Up Call.

[Denis Kenzior]

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

Hi Andrew,

On 09/08/2010 12:59 PM, Andrzej Zaborowski wrote:
> One of the clean-up commits changed the semantics of the dial request
> callback's parameter (NULL if call setup failed, non-NULL if success
> or user cancelled).

Patch has been applied, thanks.

Regards,
-Denis