[isi-voicecall-fix PATCHv3 1/6] voicecall: fix dial result handling

[isi-voicecall-fix PATCHv3 1/6] voicecall: fix dial result handling

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

If there is an existing active call when dialing the existing call will
be automatically put on hold. The dialing result handling depended on
the voicecall driver putting the call on hold before the dial command
callback is called.

However, this is not true on isimodem driver, where the dial request
returns immediately before the existing call have changed its
status. The dial result handling now checks if the active call has been
resulted from the dial request.
---
 src/voicecall.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index bd64432..e3ce2a5 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -68,6 +68,7 @@ struct voicecall {
 	char *message;
 	uint8_t icon_id;
 	gboolean untracked;
+	gboolean dial_result_handled;
 };
 
 struct dial_request {
@@ -1096,9 +1097,20 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
 		v = l->data;
 
 		if (v->call->status == CALL_STATUS_DIALING ||
-				v->call->status == CALL_STATUS_ALERTING ||
-				v->call->status == CALL_STATUS_ACTIVE)
+				v->call->status == CALL_STATUS_ALERTING)
 			return v;
+
+		/*
+		 * Dial request may return before existing active call
+		 * is put on hold or after dialed call has got active
+		 */
+		if (v->call->status == CALL_STATUS_ACTIVE &&
+				v->call->direction ==
+				CALL_DIRECTION_MOBILE_ORIGINATED &&
+				!v->dial_result_handled) {
+			v->dial_result_handled = TRUE;
+			return v;
+		}
 	}
 
 	call = synthesize_outgoing_call(vc, number);
@@ -1119,6 +1131,8 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
 
 	*need_to_emit = TRUE;
 
+	v->dial_result_handled = TRUE;
+
 	return v;
 }
 
-- 
1.7.1

[isi-voicecall-fix PATCHv3 2/6] isi/voicecall: fix status reporting

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Report early incoming calls as waiting or incoming, depending on the
state of other calls.

Report MT_RELEASED or MO_RELEASED via ofono_voicecall_notify(),
TERMINATED calls via ofono_voicecall_disconnected().
---
 drivers/isimodem/voicecall.c |  401 +++++++++++++++++++++---------------------
 1 files changed, 203 insertions(+), 198 deletions(-)

diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
index c3365f6..c450f12 100644
--- a/drivers/isimodem/voicecall.c
+++ b/drivers/isimodem/voicecall.c
@@ -62,13 +62,6 @@ struct isi_voicecall {
 
 /* ------------------------------------------------------------------------- */
 
-static void isi_call_notify(struct ofono_voicecall *ovc,
-				struct isi_call *call);
-static void isi_call_release(struct ofono_voicecall *, struct isi_call *);
-static struct ofono_call isi_call_as_ofono_call(struct isi_call const *);
-static int isi_call_status_to_clcc(struct isi_call const *call);
-static struct isi_call *isi_call_set_idle(struct isi_call *call);
-
 typedef void GIsiIndication(GIsiClient *client,
 		const void *restrict data, size_t len,
 		uint16_t object, void *opaque);
@@ -80,9 +73,6 @@ typedef gboolean GIsiResponse(GIsiClient *client,
 				void const *restrict data, size_t len,
 				uint16_t object, void *opaque);
 
-static GIsiVerify isi_call_verify_cb;
-static gboolean isi_call_register(gpointer);
-
 enum {
 	ISI_CALL_TIMEOUT = 1000,
 };
@@ -205,6 +195,174 @@ static gboolean isi_ctx_return_success(struct isi_call_req_context *irc)
 }
 
 /* ------------------------------------------------------------------------- */
+/* Notify */
+
+enum clcc_status {
+	CLCC_STATUS_ACTIVE = 0,
+	CLCC_STATUS_HOLD = 1,
+	CLCC_STATUS_DIALING = 2,
+	CLCC_STATUS_ALERTING = 3,
+	CLCC_STATUS_INCOMING = 4,
+	CLCC_STATUS_WAITING = 5,
+	CLCC_STATUS_DISCONNECTED = 6, /* Nonstandard */
+};
+
+static int isi_call_waiting_or_incoming(struct isi_voicecall const *ivc)
+{
+	int id;
+
+	for (id = 1; id <= 7; id++) {
+		switch (ivc->calls[id].status) {
+		case CALL_STATUS_ANSWERED:
+		case CALL_STATUS_ACTIVE:
+		case CALL_STATUS_HOLD_INITIATED:
+		case CALL_STATUS_HOLD:
+		case CALL_STATUS_RETRIEVE_INITIATED:
+		case CALL_STATUS_RECONNECT_PENDING:
+		case CALL_STATUS_SWAP_INITIATED:
+			return CLCC_STATUS_WAITING;
+		}
+	}
+
+	return CLCC_STATUS_INCOMING;
+}
+
+/** Get +CLCC status */
+static int isi_call_status_to_clcc(struct isi_voicecall const *ivc,
+					struct isi_call const *call)
+{
+	switch (call->status) {
+	case CALL_STATUS_CREATE:
+		return CLCC_STATUS_DIALING;
+
+	case CALL_STATUS_COMING:
+		return isi_call_waiting_or_incoming(ivc);
+
+	case CALL_STATUS_PROCEEDING:
+		if ((call->mode_info & CALL_MODE_ORIGINATOR))
+			return isi_call_waiting_or_incoming(ivc); /* MT */
+		else
+			return CLCC_STATUS_DIALING; /* MO */
+
+	case CALL_STATUS_MO_ALERTING:
+		return CLCC_STATUS_ALERTING;
+
+	case CALL_STATUS_MT_ALERTING:
+		return CLCC_STATUS_INCOMING;
+
+	case CALL_STATUS_WAITING:
+		return CLCC_STATUS_WAITING;
+
+	case CALL_STATUS_ANSWERED:
+	case CALL_STATUS_ACTIVE:
+	case CALL_STATUS_HOLD_INITIATED:
+	case CALL_STATUS_RECONNECT_PENDING:
+	case CALL_STATUS_SWAP_INITIATED:
+		return CLCC_STATUS_ACTIVE;
+
+	case CALL_STATUS_HOLD:
+	case CALL_STATUS_RETRIEVE_INITIATED:
+		return CLCC_STATUS_HOLD;
+
+	case CALL_STATUS_MO_RELEASE:
+	case CALL_STATUS_MT_RELEASE:
+	case CALL_STATUS_TERMINATED:
+	case CALL_STATUS_IDLE:
+		return CLCC_STATUS_DISCONNECTED;
+	}
+
+	return CLCC_STATUS_ACTIVE;
+}
+
+static struct ofono_call isi_call_as_ofono_call(struct isi_voicecall const *ivc,
+						struct isi_call const *call)
+{
+	struct ofono_call ocall = { call->id };
+	struct ofono_phone_number *number = &ocall.phone_number;
+
+	ocall.type = 0;	/* Voice call */
+	ocall.direction = call->mode_info & CALL_MODE_ORIGINATOR;
+	ocall.status = isi_call_status_to_clcc(ivc, call);
+	memcpy(number->number, call->address, sizeof number->number);
+	number->type = 0x80 | call->addr_type;
+	ocall.clip_validity = call->presentation & 3;
+
+	if (ocall.clip_validity == 0 && strlen(number->number) == 0)
+		ocall.clip_validity = 2;
+
+	return ocall;
+}
+
+static struct isi_call *isi_call_set_idle(struct isi_call *call)
+{
+	uint8_t id;
+
+	id = call->id;
+	memset(call, 0, sizeof *call);
+	call->id = id;
+
+	return call;
+}
+
+static void isi_call_disconnected(struct ofono_voicecall *ovc,
+					struct isi_call *call)
+{
+	struct ofono_error error = { OFONO_ERROR_TYPE_NO_ERROR, 0 };
+	enum ofono_disconnect_reason reason = call->reason;
+
+	if (!reason)
+		reason = OFONO_DISCONNECT_REASON_ERROR;
+
+	DBG("disconnected id=%u reason=%u", call->id, reason);
+	ofono_voicecall_disconnected(ovc, call->id, reason, &error);
+	isi_call_set_idle(call);
+}
+
+static void isi_call_notify(struct ofono_voicecall *ovc,
+				struct isi_call *call)
+{
+	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
+	struct isi_call_req_context *irc, **queue;
+	struct ofono_call ocall;
+
+	DBG("called with status=%s (0x%02X)",
+		call_status_name(call->status), call->status);
+
+	for (queue = &ivc->queue; (irc = *queue);) {
+		irc->step(irc, call->status);
+
+		if (*queue == irc)
+			queue = &irc->next;
+	}
+
+	ocall = isi_call_as_ofono_call(ivc, call);
+
+	DBG("id=%u,\"%s\",%u,\"%s\",%u,%u",
+		ocall.id,
+		ocall.direction ? "mt" : "mo",
+		ocall.status,
+		ocall.phone_number.number,
+		ocall.phone_number.type,
+		ocall.clip_validity);
+
+	ofono_voicecall_notify(ovc, &ocall);
+
+	switch (call->status) {
+	case CALL_STATUS_MO_RELEASE:
+		call->reason = OFONO_DISCONNECT_REASON_LOCAL_HANGUP;
+		break;
+
+	case CALL_STATUS_MT_RELEASE:
+		call->reason = OFONO_DISCONNECT_REASON_REMOTE_HANGUP;
+		break;
+
+	case CALL_STATUS_IDLE:
+	case CALL_STATUS_TERMINATED:
+		isi_call_disconnected(ovc, call);
+	}
+}
+
+/* ------------------------------------------------------------------------- */
 /* Decoding subblocks */
 
 static void isi_call_any_address_sb_proc(struct isi_voicecall *ivc,
@@ -491,16 +649,8 @@ static void isi_call_status_ind_cb(GIsiClient *client,
 		}
 	}
 
-	if (old != call->status) {
-		if (call->status == CALL_STATUS_IDLE) {
-			call->status = CALL_STATUS_TERMINATED;
-			isi_call_notify(ovc, call);
-			isi_call_set_idle(call);
-			return;
-		}
-	}
-
-	isi_call_notify(ovc, call);
+	if (old != call->status)
+		isi_call_notify(ovc, call);
 }
 
 static struct isi_call_req_context *
@@ -801,150 +951,6 @@ static gboolean isi_call_dtmf_send_resp(GIsiClient *client,
 		return isi_ctx_return_failure(irc);
 }
 
-
-/* ------------------------------------------------------------------------- */
-/* Notify */
-
-static void isi_call_notify(struct ofono_voicecall *ovc,
-				struct isi_call *call)
-{
-	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
-	struct isi_call_req_context *irc, **queue;
-	struct ofono_call ocall;
-
-	DBG("called with status=%s (0x%02X)",
-		call_status_name(call->status), call->status);
-
-	for (queue = &ivc->queue; (irc = *queue);) {
-		irc->step(irc, call->status);
-		if (*queue == irc)
-			queue = &irc->next;
-	}
-
-	switch (call->status) {
-	case CALL_STATUS_IDLE:
-	case CALL_STATUS_MO_RELEASE:
-	case CALL_STATUS_MT_RELEASE:
-	case CALL_STATUS_TERMINATED:
-		isi_call_release(ovc, call);
-		return;
-	}
-
-	ocall = isi_call_as_ofono_call(call);
-
-	DBG("id=%u,%s,%u,\"%s\",%u,%u",
-		ocall.id,
-		ocall.direction ? "terminated" : "originated",
-		ocall.status,
-		ocall.phone_number.number,
-		ocall.phone_number.type,
-		ocall.clip_validity);
-
-	ofono_voicecall_notify(ovc, &ocall);
-}
-
-static void isi_call_release(struct ofono_voicecall *ovc,
-				struct isi_call *call)
-{
-	struct ofono_error error = { OFONO_ERROR_TYPE_NO_ERROR, 0 };
-	enum ofono_disconnect_reason reason;
-
-	switch (call->status) {
-	case CALL_STATUS_IDLE:
-		reason = OFONO_DISCONNECT_REASON_UNKNOWN;
-		break;
-	case CALL_STATUS_MO_RELEASE:
-		reason = OFONO_DISCONNECT_REASON_LOCAL_HANGUP;
-		break;
-	case CALL_STATUS_MT_RELEASE:
-		reason = OFONO_DISCONNECT_REASON_REMOTE_HANGUP;
-		break;
-	case CALL_STATUS_TERMINATED:
-	default:
-		reason = OFONO_DISCONNECT_REASON_ERROR;
-		break;
-	}
-
-	if (!call->reason) {
-		call->reason = reason;
-		DBG("disconnected id=%u reason=%u", call->id, reason);
-		ofono_voicecall_disconnected(ovc, call->id, reason, &error);
-	}
-
-	if (!reason)
-		isi_call_set_idle(call);
-}
-
-static struct ofono_call isi_call_as_ofono_call(struct isi_call const *call)
-{
-	struct ofono_call ocall = { call->id };
-	struct ofono_phone_number *number = &ocall.phone_number;
-
-	ocall.type = 0;	/* Voice call */
-	ocall.direction = call->mode_info & CALL_MODE_ORIGINATOR;
-	ocall.status = isi_call_status_to_clcc(call);
-	memcpy(number->number, call->address, sizeof number->number);
-	number->type = 0x80 | call->addr_type;
-	ocall.clip_validity = call->presentation & 3;
-	if (ocall.clip_validity == 0 && strlen(number->number) == 0)
-		ocall.clip_validity = 2;
-
-	return ocall;
-}
-
-/** Get +CLCC status */
-static int isi_call_status_to_clcc(struct isi_call const *call)
-{
-	switch (call->status) {
-	case CALL_STATUS_CREATE:
-		return 2;
-	case CALL_STATUS_COMING:
-		return 4;
-	case CALL_STATUS_PROCEEDING:
-		if ((call->mode_info & CALL_MODE_ORIGINATOR))
-			return 4; /* MT */
-		else
-			return 2; /* MO */
-	case CALL_STATUS_MO_ALERTING:
-		return 3;
-	case CALL_STATUS_MT_ALERTING:
-		return 4;
-	case CALL_STATUS_WAITING:
-		return 5;
-
-	case CALL_STATUS_ANSWERED:
-	case CALL_STATUS_ACTIVE:
-	case CALL_STATUS_MO_RELEASE:
-	case CALL_STATUS_MT_RELEASE:
-	case CALL_STATUS_HOLD_INITIATED:
-		return 0;
-
-	case CALL_STATUS_HOLD:
-	case CALL_STATUS_RETRIEVE_INITIATED:
-		return 1;
-
-	case CALL_STATUS_RECONNECT_PENDING:
-	case CALL_STATUS_TERMINATED:
-	case CALL_STATUS_SWAP_INITIATED:
-		return 0;
-	}
-
-	return 0;
-}
-
-static struct isi_call *isi_call_set_idle(struct isi_call *call)
-{
-	uint8_t id;
-
-	if (call) {
-		id = call->id;
-		memset(call, 0, sizeof *call);
-		call->id = id;
-	}
-
-	return call;
-}
-
 /* ---------------------------------------------------------------------- */
 
 static void isi_dial(struct ofono_voicecall *ovc,
@@ -1244,31 +1250,27 @@ static void isi_send_tones(struct ofono_voicecall *ovc, const char *tones,
 	isi_call_dtmf_send_req(ovc, CALL_ID_ALL, tones, cb, data);;
 }
 
-static int isi_voicecall_probe(struct ofono_voicecall *ovc,
-				unsigned int vendor, void *user)
+static gboolean isi_call_register(gpointer _ovc)
 {
-	GIsiModem *idx = user;
-	struct isi_voicecall *ivc = g_try_new0(struct isi_voicecall, 1);
-	int id;
-
-	if (!ivc)
-		return -ENOMEM;
+	struct ofono_voicecall *ovc = _ovc;
+	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
+	const char *debug = getenv("OFONO_ISI_DEBUG");
 
-	for (id = 0; id <= 7; id++)
-		ivc->calls[id].id = id;
+	if (debug && (strcmp(debug, "all") == 0 || strcmp(debug, "call") == 0))
+		g_isi_client_set_debug(ivc->client, call_debug, NULL);
 
-	ivc->client = g_isi_client_create(idx, PN_CALL);
-	if (!ivc->client) {
-		g_free(ivc);
-		return -ENOMEM;
-	}
+	g_isi_subscribe(ivc->client,
+			CALL_STATUS_IND, isi_call_status_ind_cb,
+			ovc);
 
-	ofono_voicecall_set_data(ovc, ivc);
+	if (!isi_call_status_req(ovc, CALL_ID_ALL,
+					CALL_STATUS_MODE_ADDR_AND_ORIGIN,
+					NULL, NULL))
+		DBG("Failed to request call status");
 
-	if (!g_isi_verify(ivc->client, isi_call_verify_cb, ovc))
-		DBG("Unable to verify reachability");
+	ofono_voicecall_register(ovc);
 
-	return 0;
+	return FALSE;
 }
 
 static void isi_call_verify_cb(GIsiClient *client,
@@ -1287,28 +1289,31 @@ static void isi_call_verify_cb(GIsiClient *client,
 	g_idle_add(isi_call_register, ovc);
 }
 
-static gboolean isi_call_register(gpointer _ovc)
+static int isi_voicecall_probe(struct ofono_voicecall *ovc,
+				unsigned int vendor, void *user)
 {
-	struct ofono_voicecall *ovc = _ovc;
-	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
+	GIsiModem *idx = user;
+	struct isi_voicecall *ivc = g_try_new0(struct isi_voicecall, 1);
+	int id;
 
-	const char *debug = getenv("OFONO_ISI_DEBUG");
+	if (!ivc)
+		return -ENOMEM;
 
-	if (debug && (strcmp(debug, "all") == 0 || strcmp(debug, "call") == 0))
-		g_isi_client_set_debug(ivc->client, call_debug, NULL);
+	for (id = 1; id <= 7; id++)
+		ivc->calls[id].id = id;
 
-	g_isi_subscribe(ivc->client,
-			CALL_STATUS_IND, isi_call_status_ind_cb,
-			ovc);
+	ivc->client = g_isi_client_create(idx, PN_CALL);
+	if (!ivc->client) {
+		g_free(ivc);
+		return -ENOMEM;
+	}
 
-	if (!isi_call_status_req(ovc, CALL_ID_ALL,
-					CALL_STATUS_MODE_ADDR_AND_ORIGIN,
-					NULL, NULL))
-		DBG("Failed to request call status");
+	ofono_voicecall_set_data(ovc, ivc);
 
-	ofono_voicecall_register(ovc);
+	if (!g_isi_verify(ivc->client, isi_call_verify_cb, ovc))
+		DBG("Unable to verify reachability");
 
-	return FALSE;
+	return 0;
 }
 
 static void isi_voicecall_remove(struct ofono_voicecall *call)
-- 
1.7.1

[isi-voicecall-fix PATCHv3 3/6] isi/voicecall: save call id when queueing requests

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

---
 drivers/isimodem/voicecall.c |   64 +++++++++++++++++++++++++++---------------
 1 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
index c450f12..60052d4 100644
--- a/drivers/isimodem/voicecall.c
+++ b/drivers/isimodem/voicecall.c
@@ -80,41 +80,57 @@ enum {
 /* ------------------------------------------------------------------------- */
 /* Request context for voicecall cb */
 
-struct isi_call_req_context;
-
 typedef void isi_call_req_step(struct isi_call_req_context *, int reason);
 
 struct isi_call_req_context {
-	struct isi_call_req_context *next, **prev;
+	struct isi_call_req_context *next;
+	struct isi_call_req_context **prev;
 	isi_call_req_step *step;
+	int id;
 	struct ofono_voicecall *ovc;
 	ofono_voicecall_cb_t cb;
 	void *data;
 };
 
-static struct isi_call_req_context *
-isi_call_req(struct ofono_voicecall *ovc,
-		void const *restrict req,
-		size_t len,
-		GIsiResponse *handler,
-		ofono_voicecall_cb_t cb, void *data)
+static struct isi_call_req_context *isi_call_req_new(
+	struct ofono_voicecall *ovc,
+	ofono_voicecall_cb_t cb,
+	void *data)
 {
-	struct isi_voicecall *ivc;
 	struct isi_call_req_context *irc;
 
-	ivc = ofono_voicecall_get_data(ovc);
-
 	irc = g_try_new0(struct isi_call_req_context, 1);
+	if (irc == NULL) {
+		if (cb)
+			CALLBACK_WITH_FAILURE(cb, data);
+		return NULL;
+	}
 
-	if (irc) {
-		irc->ovc = ovc;
-		irc->cb = cb;
-		irc->data = data;
+	irc->ovc = ovc;
+	irc->cb = cb;
+	irc->data = data;
 
-		if (g_isi_request_make(ivc->client, req, len,
-					ISI_CALL_TIMEOUT, handler, irc))
-			return irc;
-	}
+	return irc;
+}
+
+static struct isi_call_req_context *isi_call_req(struct ofono_voicecall *ovc,
+						void const *restrict req,
+						size_t len,
+						GIsiResponse *handler,
+						ofono_voicecall_cb_t cb,
+						void *data)
+{
+	struct isi_voicecall *ivc;
+	struct isi_call_req_context *irc;
+
+	irc = isi_call_req_new(ovc, cb, data);
+	if (!irc)
+		return NULL;
+
+	ivc = ofono_voicecall_get_data(ovc);
+	if (g_isi_send(ivc->client, req, len,
+			ISI_CALL_TIMEOUT, handler, irc, NULL))
+		return irc;
 
 	g_free(irc);
 
@@ -125,7 +141,8 @@ isi_call_req(struct ofono_voicecall *ovc,
 }
 
 static void isi_ctx_queue(struct isi_call_req_context *irc,
-				isi_call_req_step *next)
+				isi_call_req_step *next,
+				int id)
 {
 	if (irc->prev == NULL) {
 		struct isi_voicecall *ivc = ofono_voicecall_get_data(irc->ovc);
@@ -139,6 +156,7 @@ static void isi_ctx_queue(struct isi_call_req_context *irc,
 	}
 
 	irc->step = next;
+	irc->id = id;
 }
 
 static void isi_ctx_remove(struct isi_call_req_context *irc)
@@ -1078,9 +1096,9 @@ static void isi_release_all_active(struct ofono_voicecall *ovc,
 		if (irc == NULL)
 			;
 		else if (waiting)
-			isi_ctx_queue(irc, isi_wait_and_answer);
+			isi_ctx_queue(irc, isi_wait_and_answer, 0);
 		else if (hold)
-			isi_ctx_queue(irc, isi_wait_and_retrieve);
+			isi_ctx_queue(irc, isi_wait_and_retrieve, 0);
 	} else
 		CALLBACK_WITH_FAILURE(cb, data);
 }
-- 
1.7.1

[isi-voicecall-fix PATCHv3 4/6] isi/voicecall: fix answering early incoming calls

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

The voicecall driver must wait until the incoming call is mt-alerting or
waiting before answering.
---
 drivers/isimodem/voicecall.c |   49 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
index 60052d4..76aa8f5 100644
--- a/drivers/isimodem/voicecall.c
+++ b/drivers/isimodem/voicecall.c
@@ -671,18 +671,65 @@ static void isi_call_status_ind_cb(GIsiClient *client,
 		isi_call_notify(ovc, call);
 }
 
+static void isi_wait_incoming(struct isi_call_req_context *irc, int event);
+
 static struct isi_call_req_context *
 isi_call_answer_req(struct ofono_voicecall *ovc,
 			uint8_t call_id, ofono_voicecall_cb_t cb, void *data)
 {
+	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
+	int id;
+
 	uint8_t const req[] = {
 		CALL_ANSWER_REQ, call_id, 0
 	};
 	size_t rlen = sizeof req;
 
+	for (id = 1; id <= 7; id++) {
+		struct isi_call_req_context *irc;
+
+		if (!(ivc->calls[id].mode_info & CALL_MODE_ORIGINATOR))
+			continue;
+
+		if (ivc->calls[id].status != CALL_STATUS_PROCEEDING &&
+				ivc->calls[id].status != CALL_STATUS_COMING)
+			continue;
+
+		irc = isi_call_req_new(ovc, cb, data);
+		if (irc)
+			isi_ctx_queue(irc, isi_wait_incoming, id);
+
+		return irc;
+	}
+
 	return isi_call_req(ovc, req, rlen, isi_call_answer_resp, cb, data);
 }
 
+static void isi_wait_incoming(struct isi_call_req_context *irc,
+				int event)
+{
+	struct isi_voicecall *ivc;
+
+	DBG("irc=%p event=%u", (void *)irc, event);
+
+	switch (event) {
+	case CALL_STATUS_MT_ALERTING:
+	case CALL_STATUS_WAITING:
+		isi_call_answer_req(irc->ovc, irc->id, irc->cb, irc->data);
+		isi_ctx_free(irc);
+		return;
+	}
+
+	ivc = ofono_voicecall_get_data(irc->ovc);
+	switch (ivc->calls[irc->id].status) {
+	case CALL_STATUS_MO_RELEASE:
+	case CALL_STATUS_MT_RELEASE:
+	case CALL_STATUS_TERMINATED:
+	case CALL_STATUS_IDLE:
+		isi_ctx_return_failure(irc);
+	}
+}
+
 static gboolean isi_call_answer_resp(GIsiClient *client,
 					void const *restrict data,
 					size_t len,
@@ -1018,7 +1065,7 @@ static void isi_hangup_current(struct ofono_voicecall *ovc,
 	 * active calls or calls in progress.
 	 */
 	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
-	int id = 0;
+	int id;
 
 	for (id = 1; id <= 7; id++) {
 		if (ivc->calls[id].call_id & CALL_ID_WAITING)
-- 
1.7.1

[isi-voicecall-fix PATCHv3 5/6] isi/voicecall: release COMING calls with BUSY cause

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Very early incoming calls were not released with BUSY cause.
---
 drivers/isimodem/voicecall.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
index 76aa8f5..cdd5d11 100644
--- a/drivers/isimodem/voicecall.c
+++ b/drivers/isimodem/voicecall.c
@@ -1220,6 +1220,7 @@ static void isi_release_specific(struct ofono_voicecall *ovc, int id,
 		uint8_t cause = CALL_CAUSE_RELEASE_BY_USER;
 
 		switch (status->status) {
+		case CALL_STATUS_COMING:
 		case CALL_STATUS_MT_ALERTING:
 		case CALL_STATUS_WAITING:
 			cause = CALL_CAUSE_BUSY_USER_REQUEST;
-- 
1.7.1

[isi-voicecall-fix PATCHv3 6/6] isi/voicecall: fix isi_release_all_active()

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Handle to-be-waiting calls properly.
---
 drivers/isimodem/voicecall.c |   66 +++++++++++++++++++++++++++++------------
 1 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
index cdd5d11..360994a 100644
--- a/drivers/isimodem/voicecall.c
+++ b/drivers/isimodem/voicecall.c
@@ -292,6 +292,23 @@ static int isi_call_status_to_clcc(struct isi_voicecall const *ivc,
 	return CLCC_STATUS_ACTIVE;
 }
 
+static gboolean is_call_waiting(struct isi_voicecall const *ivc,
+				struct isi_call const *call)
+{
+	if (!(call->mode_info & CALL_MODE_ORIGINATOR))
+		return FALSE;
+
+	switch (call->status) {
+	case CALL_STATUS_COMING:
+	case CALL_STATUS_PROCEEDING:
+		return isi_call_waiting_or_incoming(ivc) == CLCC_STATUS_WAITING;
+	case CALL_STATUS_WAITING:
+		return TRUE;
+	}
+
+	return FALSE;
+}
+
 static struct ofono_call isi_call_as_ofono_call(struct isi_voicecall const *ivc,
 						struct isi_call const *call)
 {
@@ -1119,35 +1136,40 @@ static isi_call_req_step isi_wait_and_answer, isi_wait_and_retrieve;
 static void isi_release_all_active(struct ofono_voicecall *ovc,
 					ofono_voicecall_cb_t cb, void *data)
 {
-	/* AT+CHLD=1 */
 	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
-	int id = 0, waiting = 0, active = 0, hold = 0;
+	struct isi_call_req_context *irc;
+	int id;
+	int waiting_id = 0;
+	int active = 0;
+	int hold = 0;
 
 	for (id = 1; id <= 7; id++) {
-		if (ivc->calls[id].call_id & CALL_ID_WAITING)
-			waiting++;
+		if (is_call_waiting(ivc, &ivc->calls[id]))
+			waiting_id = id;
+
 		if (ivc->calls[id].call_id & CALL_ID_HOLD)
 			hold++;
+
 		if (ivc->calls[id].call_id & CALL_ID_ACTIVE)
 			active++;
 	}
 
-	if (active) {
-		struct isi_call_req_context *irc;
-
-		irc = isi_call_release_req(ovc, CALL_ID_ACTIVE,
-						CALL_CAUSE_TYPE_CLIENT,
-						CALL_CAUSE_RELEASE_BY_USER,
-						cb, data);
-
-		if (irc == NULL)
-			;
-		else if (waiting)
-			isi_ctx_queue(irc, isi_wait_and_answer, 0);
-		else if (hold)
-			isi_ctx_queue(irc, isi_wait_and_retrieve, 0);
-	} else
+	if (!active) {
 		CALLBACK_WITH_FAILURE(cb, data);
+		return;
+	}
+
+	irc = isi_call_release_req(ovc, CALL_ID_ACTIVE,
+					CALL_CAUSE_TYPE_CLIENT,
+					CALL_CAUSE_RELEASE_BY_USER,
+					cb, data);
+	if (!irc)
+		return;
+
+	if (waiting_id)
+		isi_ctx_queue(irc, isi_wait_and_answer, waiting_id);
+	else if (hold)
+		isi_ctx_queue(irc, isi_wait_and_retrieve, 0);
 }
 
 static void isi_wait_and_answer(struct isi_call_req_context *irc,
@@ -1155,8 +1177,10 @@ static void isi_wait_and_answer(struct isi_call_req_context *irc,
 {
 	DBG("irc=%p event=%u", (void *)irc, event);
 	switch (event) {
+	case CALL_STATUS_MO_RELEASE:
+	case CALL_STATUS_MT_RELEASE:
 	case CALL_STATUS_TERMINATED:
-		isi_answer(irc->ovc, irc->cb, irc->data);
+		isi_call_answer_req(irc->ovc, irc->id, irc->cb, irc->data);
 		isi_ctx_free(irc);
 		break;
 	}
@@ -1167,6 +1191,8 @@ static void isi_wait_and_retrieve(struct isi_call_req_context *irc,
 {
 	DBG("irc=%p event=%u", (void *)irc, event);
 	switch (event) {
+	case CALL_STATUS_MO_RELEASE:
+	case CALL_STATUS_MT_RELEASE:
 	case CALL_STATUS_TERMINATED:
 		isi_retrieve(irc->ovc, irc->cb, irc->data);
 		isi_ctx_free(irc);
-- 
1.7.1

Re: [isi-voicecall-fix PATCHv3 5/6] isi/voicecall: release COMING calls with BUSY cause

[Denis Kenzior]

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

Hi Pekka,

On 11/16/2010 11:05 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Very early incoming calls were not released with BUSY cause.
> ---
>  drivers/isimodem/voicecall.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [isi-voicecall-fix PATCHv3 4/6] isi/voicecall: fix answering early incoming calls

[Denis Kenzior]

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

Hi Pekka,

On 11/16/2010 11:05 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> The voicecall driver must wait until the incoming call is mt-alerting or
> waiting before answering.
> ---
>  drivers/isimodem/voicecall.c |   49 +++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 48 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
> index 60052d4..76aa8f5 100644
> --- a/drivers/isimodem/voicecall.c
> +++ b/drivers/isimodem/voicecall.c
> @@ -671,18 +671,65 @@ static void isi_call_status_ind_cb(GIsiClient *client,
>  		isi_call_notify(ovc, call);
>  }
>  
> +static void isi_wait_incoming(struct isi_call_req_context *irc, int event);
> +

Why is there a forward declaration for a static function?

>  static struct isi_call_req_context *
>  isi_call_answer_req(struct ofono_voicecall *ovc,
>  			uint8_t call_id, ofono_voicecall_cb_t cb, void *data)
>  {
> +	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
> +	int id;
> +
>  	uint8_t const req[] = {
>  		CALL_ANSWER_REQ, call_id, 0
>  	};
>  	size_t rlen = sizeof req;
>  
> +	for (id = 1; id <= 7; id++) {
> +		struct isi_call_req_context *irc;
> +
> +		if (!(ivc->calls[id].mode_info & CALL_MODE_ORIGINATOR))
> +			continue;
> +
> +		if (ivc->calls[id].status != CALL_STATUS_PROCEEDING &&
> +				ivc->calls[id].status != CALL_STATUS_COMING)
> +			continue;
> +
> +		irc = isi_call_req_new(ovc, cb, data);
> +		if (irc)

Why not simply

if (irc == NULL)
	return NULL

> +			isi_ctx_queue(irc, isi_wait_incoming, id);
> +
> +		return irc;
> +	}
> +
>  	return isi_call_req(ovc, req, rlen, isi_call_answer_resp, cb, data);
>  }
>  
> +static void isi_wait_incoming(struct isi_call_req_context *irc,
> +				int event)
> +{
> +	struct isi_voicecall *ivc;
> +
> +	DBG("irc=%p event=%u", (void *)irc, event);
> +
> +	switch (event) {
> +	case CALL_STATUS_MT_ALERTING:
> +	case CALL_STATUS_WAITING:
> +		isi_call_answer_req(irc->ovc, irc->id, irc->cb, irc->data);
> +		isi_ctx_free(irc);
> +		return;
> +	}
> +
> +	ivc = ofono_voicecall_get_data(irc->ovc);

Rule M1 again

> +	switch (ivc->calls[irc->id].status) {
> +	case CALL_STATUS_MO_RELEASE:
> +	case CALL_STATUS_MT_RELEASE:
> +	case CALL_STATUS_TERMINATED:
> +	case CALL_STATUS_IDLE:
> +		isi_ctx_return_failure(irc);
> +	}
> +}
> +
>  static gboolean isi_call_answer_resp(GIsiClient *client,
>  					void const *restrict data,
>  					size_t len,
> @@ -1018,7 +1065,7 @@ static void isi_hangup_current(struct ofono_voicecall *ovc,
>  	 * active calls or calls in progress.
>  	 */
>  	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
> -	int id = 0;
> +	int id;
>  
>  	for (id = 1; id <= 7; id++) {
>  		if (ivc->calls[id].call_id & CALL_ID_WAITING)

Re: [isi-voicecall-fix PATCHv3 4/6] isi/voicecall: fix answering early incoming calls

[Pekka Pessi]

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

Hi Denis,

2010/11/22 Denis Kenzior <denkenz@gmail.com>:
>> +static void isi_wait_incoming(struct isi_call_req_context *irc, int event);
>> +
>
> Why is there a forward declaration for a static function?

Because there is a dependency loop: isi_call_answer_req uses
isi_wait_incoming and isi_wait_incoming calls isi_call_answer_req.

-- 
Pekka.Pessi mail at nokia.com

Re: [isi-voicecall-fix PATCHv3 4/6] isi/voicecall: fix answering early incoming calls

[Denis Kenzior]

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

Hi Pekka,

On 11/22/2010 10:22 AM, Pekka Pessi wrote:
> Hi Denis,
> 
> 2010/11/22 Denis Kenzior <denkenz@gmail.com>:
>>> +static void isi_wait_incoming(struct isi_call_req_context *irc, int event);
>>> +
>>
>> Why is there a forward declaration for a static function?
> 
> Because there is a dependency loop: isi_call_answer_req uses
> isi_wait_incoming and isi_wait_incoming calls isi_call_answer_req.
> 

Ok, fair enough.

Regards,
-Denis

Re: [isi-voicecall-fix PATCHv3 3/6] isi/voicecall: save call id when queueing requests

[Denis Kenzior]

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

Hi Pekka,

On 11/16/2010 11:05 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> ---
>  drivers/isimodem/voicecall.c |   64 +++++++++++++++++++++++++++---------------
>  1 files changed, 41 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
> index c450f12..60052d4 100644
> --- a/drivers/isimodem/voicecall.c
> +++ b/drivers/isimodem/voicecall.c
> @@ -80,41 +80,57 @@ enum {
>  /* ------------------------------------------------------------------------- */
>  /* Request context for voicecall cb */
>  
> -struct isi_call_req_context;
> -
>  typedef void isi_call_req_step(struct isi_call_req_context *, int reason);
>  
>  struct isi_call_req_context {
> -	struct isi_call_req_context *next, **prev;
> +	struct isi_call_req_context *next;
> +	struct isi_call_req_context **prev;
>  	isi_call_req_step *step;
> +	int id;
>  	struct ofono_voicecall *ovc;
>  	ofono_voicecall_cb_t cb;
>  	void *data;
>  };
>  
> -static struct isi_call_req_context *
> -isi_call_req(struct ofono_voicecall *ovc,
> -		void const *restrict req,
> -		size_t len,
> -		GIsiResponse *handler,
> -		ofono_voicecall_cb_t cb, void *data)
> +static struct isi_call_req_context *isi_call_req_new(
> +	struct ofono_voicecall *ovc,
> +	ofono_voicecall_cb_t cb,
> +	void *data)

Please indent this some more

>  {
> -	struct isi_voicecall *ivc;
>  	struct isi_call_req_context *irc;
>  
> -	ivc = ofono_voicecall_get_data(ovc);
> -
>  	irc = g_try_new0(struct isi_call_req_context, 1);
> +	if (irc == NULL) {
> +		if (cb)
> +			CALLBACK_WITH_FAILURE(cb, data);

doc/docing-style.txt item M1.

> +		return NULL;
> +	}
>  
> -	if (irc) {
> -		irc->ovc = ovc;
> -		irc->cb = cb;
> -		irc->data = data;
> +	irc->ovc = ovc;
> +	irc->cb = cb;
> +	irc->data = data;
>  
> -		if (g_isi_request_make(ivc->client, req, len,
> -					ISI_CALL_TIMEOUT, handler, irc))
> -			return irc;
> -	}
> +	return irc;
> +}
> +
> +static struct isi_call_req_context *isi_call_req(struct ofono_voicecall *ovc,
> +						void const *restrict req,
> +						size_t len,
> +						GIsiResponse *handler,
> +						ofono_voicecall_cb_t cb,
> +						void *data)
> +{
> +	struct isi_voicecall *ivc;
> +	struct isi_call_req_context *irc;
> +
> +	irc = isi_call_req_new(ovc, cb, data);
> +	if (!irc)
> +		return NULL;
> +
> +	ivc = ofono_voicecall_get_data(ovc);

doc/docing-style.txt item M1.

> +	if (g_isi_send(ivc->client, req, len,
> +			ISI_CALL_TIMEOUT, handler, irc, NULL))
> +		return irc;
>  
>  	g_free(irc);
>  
> @@ -125,7 +141,8 @@ isi_call_req(struct ofono_voicecall *ovc,
>  }
>  
>  static void isi_ctx_queue(struct isi_call_req_context *irc,
> -				isi_call_req_step *next)
> +				isi_call_req_step *next,
> +				int id)
>  {
>  	if (irc->prev == NULL) {
>  		struct isi_voicecall *ivc = ofono_voicecall_get_data(irc->ovc);
> @@ -139,6 +156,7 @@ static void isi_ctx_queue(struct isi_call_req_context *irc,
>  	}
>  
>  	irc->step = next;
> +	irc->id = id;
>  }
>  
>  static void isi_ctx_remove(struct isi_call_req_context *irc)
> @@ -1078,9 +1096,9 @@ static void isi_release_all_active(struct ofono_voicecall *ovc,
>  		if (irc == NULL)
>  			;
>  		else if (waiting)
> -			isi_ctx_queue(irc, isi_wait_and_answer);
> +			isi_ctx_queue(irc, isi_wait_and_answer, 0);
>  		else if (hold)
> -			isi_ctx_queue(irc, isi_wait_and_retrieve);
> +			isi_ctx_queue(irc, isi_wait_and_retrieve, 0);
>  	} else
>  		CALLBACK_WITH_FAILURE(cb, data);
>  }

Regards,
-Denis

Re: [isi-voicecall-fix PATCHv3 2/6] isi/voicecall: fix status reporting

[Denis Kenzior]

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

Hi Pekka,

On 11/16/2010 11:05 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Report early incoming calls as waiting or incoming, depending on the
> state of other calls.
> 
> Report MT_RELEASED or MO_RELEASED via ofono_voicecall_notify(),
> TERMINATED calls via ofono_voicecall_disconnected().
> ---
>  drivers/isimodem/voicecall.c |  401 +++++++++++++++++++++---------------------
>  1 files changed, 203 insertions(+), 198 deletions(-)
> 
> diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
> index c3365f6..c450f12 100644
> --- a/drivers/isimodem/voicecall.c
> +++ b/drivers/isimodem/voicecall.c
> @@ -62,13 +62,6 @@ struct isi_voicecall {
>  
>  /* ------------------------------------------------------------------------- */
>  
> -static void isi_call_notify(struct ofono_voicecall *ovc,
> -				struct isi_call *call);
> -static void isi_call_release(struct ofono_voicecall *, struct isi_call *);
> -static struct ofono_call isi_call_as_ofono_call(struct isi_call const *);
> -static int isi_call_status_to_clcc(struct isi_call const *call);
> -static struct isi_call *isi_call_set_idle(struct isi_call *call);
> -
>  typedef void GIsiIndication(GIsiClient *client,
>  		const void *restrict data, size_t len,
>  		uint16_t object, void *opaque);
> @@ -80,9 +73,6 @@ typedef gboolean GIsiResponse(GIsiClient *client,
>  				void const *restrict data, size_t len,
>  				uint16_t object, void *opaque);
>  
> -static GIsiVerify isi_call_verify_cb;
> -static gboolean isi_call_register(gpointer);
> -
>  enum {
>  	ISI_CALL_TIMEOUT = 1000,
>  };
> @@ -205,6 +195,174 @@ static gboolean isi_ctx_return_success(struct isi_call_req_context *irc)
>  }
>  
>  /* ------------------------------------------------------------------------- */
> +/* Notify */
> +
> +enum clcc_status {
> +	CLCC_STATUS_ACTIVE = 0,
> +	CLCC_STATUS_HOLD = 1,
> +	CLCC_STATUS_DIALING = 2,
> +	CLCC_STATUS_ALERTING = 3,
> +	CLCC_STATUS_INCOMING = 4,
> +	CLCC_STATUS_WAITING = 5,
> +	CLCC_STATUS_DISCONNECTED = 6, /* Nonstandard */
> +};

Please follow M11 completely here, particularly the tabs before the values.

> +
> +static int isi_call_waiting_or_incoming(struct isi_voicecall const *ivc)
> +{
> +	int id;
> +
> +	for (id = 1; id <= 7; id++) {
> +		switch (ivc->calls[id].status) {
> +		case CALL_STATUS_ANSWERED:
> +		case CALL_STATUS_ACTIVE:
> +		case CALL_STATUS_HOLD_INITIATED:
> +		case CALL_STATUS_HOLD:
> +		case CALL_STATUS_RETRIEVE_INITIATED:
> +		case CALL_STATUS_RECONNECT_PENDING:
> +		case CALL_STATUS_SWAP_INITIATED:
> +			return CLCC_STATUS_WAITING;
> +		}
> +	}
> +
> +	return CLCC_STATUS_INCOMING;
> +}
> +
> +/** Get +CLCC status */
> +static int isi_call_status_to_clcc(struct isi_voicecall const *ivc,
> +					struct isi_call const *call)
> +{
> +	switch (call->status) {
> +	case CALL_STATUS_CREATE:
> +		return CLCC_STATUS_DIALING;
> +
> +	case CALL_STATUS_COMING:
> +		return isi_call_waiting_or_incoming(ivc);
> +
> +	case CALL_STATUS_PROCEEDING:
> +		if ((call->mode_info & CALL_MODE_ORIGINATOR))
> +			return isi_call_waiting_or_incoming(ivc); /* MT */
> +		else
> +			return CLCC_STATUS_DIALING; /* MO */
> +
> +	case CALL_STATUS_MO_ALERTING:
> +		return CLCC_STATUS_ALERTING;
> +
> +	case CALL_STATUS_MT_ALERTING:
> +		return CLCC_STATUS_INCOMING;
> +
> +	case CALL_STATUS_WAITING:
> +		return CLCC_STATUS_WAITING;
> +
> +	case CALL_STATUS_ANSWERED:
> +	case CALL_STATUS_ACTIVE:
> +	case CALL_STATUS_HOLD_INITIATED:
> +	case CALL_STATUS_RECONNECT_PENDING:
> +	case CALL_STATUS_SWAP_INITIATED:
> +		return CLCC_STATUS_ACTIVE;
> +
> +	case CALL_STATUS_HOLD:
> +	case CALL_STATUS_RETRIEVE_INITIATED:
> +		return CLCC_STATUS_HOLD;
> +
> +	case CALL_STATUS_MO_RELEASE:
> +	case CALL_STATUS_MT_RELEASE:
> +	case CALL_STATUS_TERMINATED:
> +	case CALL_STATUS_IDLE:
> +		return CLCC_STATUS_DISCONNECTED;
> +	}
> +
> +	return CLCC_STATUS_ACTIVE;
> +}
> +
> +static struct ofono_call isi_call_as_ofono_call(struct isi_voicecall const *ivc,
> +						struct isi_call const *call)
> +{
> +	struct ofono_call ocall = { call->id };
> +	struct ofono_phone_number *number = &ocall.phone_number;
> +
> +	ocall.type = 0;	/* Voice call */
> +	ocall.direction = call->mode_info & CALL_MODE_ORIGINATOR;
> +	ocall.status = isi_call_status_to_clcc(ivc, call);
> +	memcpy(number->number, call->address, sizeof number->number);

Please watch out for the sizeof usage.

> +	number->type = 0x80 | call->addr_type;
> +	ocall.clip_validity = call->presentation & 3;
> +
> +	if (ocall.clip_validity == 0 && strlen(number->number) == 0)
> +		ocall.clip_validity = 2;
> +
> +	return ocall;
> +}
> +
> +static struct isi_call *isi_call_set_idle(struct isi_call *call)
> +{
> +	uint8_t id;
> +
> +	id = call->id;
> +	memset(call, 0, sizeof *call);

sizeof use again

> +	call->id = id;
> +
> +	return call;
> +}
> +
> +static void isi_call_disconnected(struct ofono_voicecall *ovc,
> +					struct isi_call *call)
> +{
> +	struct ofono_error error = { OFONO_ERROR_TYPE_NO_ERROR, 0 };
> +	enum ofono_disconnect_reason reason = call->reason;
> +
> +	if (!reason)
> +		reason = OFONO_DISCONNECT_REASON_ERROR;
> +
> +	DBG("disconnected id=%u reason=%u", call->id, reason);
> +	ofono_voicecall_disconnected(ovc, call->id, reason, &error);
> +	isi_call_set_idle(call);
> +}
> +
> +static void isi_call_notify(struct ofono_voicecall *ovc,
> +				struct isi_call *call)
> +{
> +	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
> +	struct isi_call_req_context *irc, **queue;
> +	struct ofono_call ocall;
> +
> +	DBG("called with status=%s (0x%02X)",
> +		call_status_name(call->status), call->status);
> +
> +	for (queue = &ivc->queue; (irc = *queue);) {
> +		irc->step(irc, call->status);
> +
> +		if (*queue == irc)
> +			queue = &irc->next;
> +	}
> +
> +	ocall = isi_call_as_ofono_call(ivc, call);
> +
> +	DBG("id=%u,\"%s\",%u,\"%s\",%u,%u",
> +		ocall.id,
> +		ocall.direction ? "mt" : "mo",
> +		ocall.status,
> +		ocall.phone_number.number,
> +		ocall.phone_number.type,
> +		ocall.clip_validity);
> +
> +	ofono_voicecall_notify(ovc, &ocall);
> +
> +	switch (call->status) {
> +	case CALL_STATUS_MO_RELEASE:
> +		call->reason = OFONO_DISCONNECT_REASON_LOCAL_HANGUP;
> +		break;
> +
> +	case CALL_STATUS_MT_RELEASE:
> +		call->reason = OFONO_DISCONNECT_REASON_REMOTE_HANGUP;
> +		break;
> +
> +	case CALL_STATUS_IDLE:
> +	case CALL_STATUS_TERMINATED:
> +		isi_call_disconnected(ovc, call);

Is the break statement missing here?

> +	}
> +}
> +
> +/* ------------------------------------------------------------------------- */
>  /* Decoding subblocks */
>  
>  static void isi_call_any_address_sb_proc(struct isi_voicecall *ivc,
> @@ -491,16 +649,8 @@ static void isi_call_status_ind_cb(GIsiClient *client,
>  		}
>  	}
>  
> -	if (old != call->status) {
> -		if (call->status == CALL_STATUS_IDLE) {
> -			call->status = CALL_STATUS_TERMINATED;
> -			isi_call_notify(ovc, call);
> -			isi_call_set_idle(call);
> -			return;
> -		}
> -	}
> -
> -	isi_call_notify(ovc, call);
> +	if (old != call->status)
> +		isi_call_notify(ovc, call);
>  }
>  
>  static struct isi_call_req_context *
> @@ -801,150 +951,6 @@ static gboolean isi_call_dtmf_send_resp(GIsiClient *client,
>  		return isi_ctx_return_failure(irc);
>  }
>  
> -
> -/* ------------------------------------------------------------------------- */
> -/* Notify */
> -
> -static void isi_call_notify(struct ofono_voicecall *ovc,
> -				struct isi_call *call)
> -{
> -	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
> -	struct isi_call_req_context *irc, **queue;
> -	struct ofono_call ocall;
> -
> -	DBG("called with status=%s (0x%02X)",
> -		call_status_name(call->status), call->status);
> -
> -	for (queue = &ivc->queue; (irc = *queue);) {
> -		irc->step(irc, call->status);
> -		if (*queue == irc)
> -			queue = &irc->next;
> -	}
> -
> -	switch (call->status) {
> -	case CALL_STATUS_IDLE:
> -	case CALL_STATUS_MO_RELEASE:
> -	case CALL_STATUS_MT_RELEASE:
> -	case CALL_STATUS_TERMINATED:
> -		isi_call_release(ovc, call);
> -		return;
> -	}
> -
> -	ocall = isi_call_as_ofono_call(call);
> -
> -	DBG("id=%u,%s,%u,\"%s\",%u,%u",
> -		ocall.id,
> -		ocall.direction ? "terminated" : "originated",
> -		ocall.status,
> -		ocall.phone_number.number,
> -		ocall.phone_number.type,
> -		ocall.clip_validity);
> -
> -	ofono_voicecall_notify(ovc, &ocall);
> -}
> -
> -static void isi_call_release(struct ofono_voicecall *ovc,
> -				struct isi_call *call)
> -{
> -	struct ofono_error error = { OFONO_ERROR_TYPE_NO_ERROR, 0 };
> -	enum ofono_disconnect_reason reason;
> -
> -	switch (call->status) {
> -	case CALL_STATUS_IDLE:
> -		reason = OFONO_DISCONNECT_REASON_UNKNOWN;
> -		break;
> -	case CALL_STATUS_MO_RELEASE:
> -		reason = OFONO_DISCONNECT_REASON_LOCAL_HANGUP;
> -		break;
> -	case CALL_STATUS_MT_RELEASE:
> -		reason = OFONO_DISCONNECT_REASON_REMOTE_HANGUP;
> -		break;
> -	case CALL_STATUS_TERMINATED:
> -	default:
> -		reason = OFONO_DISCONNECT_REASON_ERROR;
> -		break;
> -	}
> -
> -	if (!call->reason) {
> -		call->reason = reason;
> -		DBG("disconnected id=%u reason=%u", call->id, reason);
> -		ofono_voicecall_disconnected(ovc, call->id, reason, &error);
> -	}
> -
> -	if (!reason)
> -		isi_call_set_idle(call);
> -}
> -
> -static struct ofono_call isi_call_as_ofono_call(struct isi_call const *call)
> -{
> -	struct ofono_call ocall = { call->id };
> -	struct ofono_phone_number *number = &ocall.phone_number;
> -
> -	ocall.type = 0;	/* Voice call */
> -	ocall.direction = call->mode_info & CALL_MODE_ORIGINATOR;
> -	ocall.status = isi_call_status_to_clcc(call);
> -	memcpy(number->number, call->address, sizeof number->number);
> -	number->type = 0x80 | call->addr_type;
> -	ocall.clip_validity = call->presentation & 3;
> -	if (ocall.clip_validity == 0 && strlen(number->number) == 0)
> -		ocall.clip_validity = 2;
> -
> -	return ocall;
> -}
> -
> -/** Get +CLCC status */
> -static int isi_call_status_to_clcc(struct isi_call const *call)
> -{
> -	switch (call->status) {
> -	case CALL_STATUS_CREATE:
> -		return 2;
> -	case CALL_STATUS_COMING:
> -		return 4;
> -	case CALL_STATUS_PROCEEDING:
> -		if ((call->mode_info & CALL_MODE_ORIGINATOR))
> -			return 4; /* MT */
> -		else
> -			return 2; /* MO */
> -	case CALL_STATUS_MO_ALERTING:
> -		return 3;
> -	case CALL_STATUS_MT_ALERTING:
> -		return 4;
> -	case CALL_STATUS_WAITING:
> -		return 5;
> -
> -	case CALL_STATUS_ANSWERED:
> -	case CALL_STATUS_ACTIVE:
> -	case CALL_STATUS_MO_RELEASE:
> -	case CALL_STATUS_MT_RELEASE:
> -	case CALL_STATUS_HOLD_INITIATED:
> -		return 0;
> -
> -	case CALL_STATUS_HOLD:
> -	case CALL_STATUS_RETRIEVE_INITIATED:
> -		return 1;
> -
> -	case CALL_STATUS_RECONNECT_PENDING:
> -	case CALL_STATUS_TERMINATED:
> -	case CALL_STATUS_SWAP_INITIATED:
> -		return 0;
> -	}
> -
> -	return 0;
> -}
> -
> -static struct isi_call *isi_call_set_idle(struct isi_call *call)
> -{
> -	uint8_t id;
> -
> -	if (call) {
> -		id = call->id;
> -		memset(call, 0, sizeof *call);
> -		call->id = id;
> -	}
> -
> -	return call;
> -}
> -
>  /* ---------------------------------------------------------------------- */
>  
>  static void isi_dial(struct ofono_voicecall *ovc,
> @@ -1244,31 +1250,27 @@ static void isi_send_tones(struct ofono_voicecall *ovc, const char *tones,
>  	isi_call_dtmf_send_req(ovc, CALL_ID_ALL, tones, cb, data);;
>  }
>  
> -static int isi_voicecall_probe(struct ofono_voicecall *ovc,
> -				unsigned int vendor, void *user)
> +static gboolean isi_call_register(gpointer _ovc)
>  {
> -	GIsiModem *idx = user;
> -	struct isi_voicecall *ivc = g_try_new0(struct isi_voicecall, 1);
> -	int id;
> -
> -	if (!ivc)
> -		return -ENOMEM;
> +	struct ofono_voicecall *ovc = _ovc;
> +	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
> +	const char *debug = getenv("OFONO_ISI_DEBUG");
>  
> -	for (id = 0; id <= 7; id++)
> -		ivc->calls[id].id = id;
> +	if (debug && (strcmp(debug, "all") == 0 || strcmp(debug, "call") == 0))
> +		g_isi_client_set_debug(ivc->client, call_debug, NULL);
>  
> -	ivc->client = g_isi_client_create(idx, PN_CALL);
> -	if (!ivc->client) {
> -		g_free(ivc);
> -		return -ENOMEM;
> -	}
> +	g_isi_subscribe(ivc->client,
> +			CALL_STATUS_IND, isi_call_status_ind_cb,
> +			ovc);
>  
> -	ofono_voicecall_set_data(ovc, ivc);
> +	if (!isi_call_status_req(ovc, CALL_ID_ALL,
> +					CALL_STATUS_MODE_ADDR_AND_ORIGIN,
> +					NULL, NULL))
> +		DBG("Failed to request call status");
>  
> -	if (!g_isi_verify(ivc->client, isi_call_verify_cb, ovc))
> -		DBG("Unable to verify reachability");
> +	ofono_voicecall_register(ovc);
>  
> -	return 0;
> +	return FALSE;
>  }
>  
>  static void isi_call_verify_cb(GIsiClient *client,
> @@ -1287,28 +1289,31 @@ static void isi_call_verify_cb(GIsiClient *client,
>  	g_idle_add(isi_call_register, ovc);
>  }
>  
> -static gboolean isi_call_register(gpointer _ovc)
> +static int isi_voicecall_probe(struct ofono_voicecall *ovc,
> +				unsigned int vendor, void *user)
>  {
> -	struct ofono_voicecall *ovc = _ovc;
> -	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
> +	GIsiModem *idx = user;
> +	struct isi_voicecall *ivc = g_try_new0(struct isi_voicecall, 1);
> +	int id;
>  
> -	const char *debug = getenv("OFONO_ISI_DEBUG");
> +	if (!ivc)
> +		return -ENOMEM;
>  
> -	if (debug && (strcmp(debug, "all") == 0 || strcmp(debug, "call") == 0))
> -		g_isi_client_set_debug(ivc->client, call_debug, NULL);
> +	for (id = 1; id <= 7; id++)
> +		ivc->calls[id].id = id;
>  
> -	g_isi_subscribe(ivc->client,
> -			CALL_STATUS_IND, isi_call_status_ind_cb,
> -			ovc);
> +	ivc->client = g_isi_client_create(idx, PN_CALL);
> +	if (!ivc->client) {
> +		g_free(ivc);
> +		return -ENOMEM;
> +	}
>  
> -	if (!isi_call_status_req(ovc, CALL_ID_ALL,
> -					CALL_STATUS_MODE_ADDR_AND_ORIGIN,
> -					NULL, NULL))
> -		DBG("Failed to request call status");
> +	ofono_voicecall_set_data(ovc, ivc);
>  
> -	ofono_voicecall_register(ovc);
> +	if (!g_isi_verify(ivc->client, isi_call_verify_cb, ovc))
> +		DBG("Unable to verify reachability");
>  
> -	return FALSE;
> +	return 0;
>  }
>  
>  static void isi_voicecall_remove(struct ofono_voicecall *call)

Regards,
-Denis

Re: [isi-voicecall-fix PATCHv3 2/6] isi/voicecall: fix status reporting

[Pekka Pessi]

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

Hi Denis,

>> +enum clcc_status {
>> +     CLCC_STATUS_ACTIVE = 0,
>> +     CLCC_STATUS_HOLD = 1,
>> +     CLCC_STATUS_DIALING = 2,
>> +     CLCC_STATUS_ALERTING = 3,
>> +     CLCC_STATUS_INCOMING = 4,
>> +     CLCC_STATUS_WAITING = 5,
>> +     CLCC_STATUS_DISCONNECTED = 6, /* Nonstandard */
>> +};
>
> Please follow M11 completely here, particularly the tabs before the values.

You mean M11 preference? If you want something, be explicit, do not prefer.

I laud your goal to improve the coding style within oFono - and it
even might be best to do that step-by-step and avoid large patches
just to fix the style - but I'm lazy typist and I usually just kill
and yank pieces of code, like this one from from "src/common.h" patch
a78b3629, authored by some Denis Kenzior.

The current codebase contains a lot of code in outdated style and
copy'n'paste coding just does not cut it, it would be ++good to have a
custom checkpatch that would show the style problems without tedious
and error-prone manual inspection. I'd do that myself but my perl-fu
is bit rusty.

>> +     memcpy(number->number, call->address, sizeof number->number);
>
> Please watch out for the sizeof usage.

This needs an entry in doc/codingstyle.txt.

>> +     case CALL_STATUS_IDLE:
>> +     case CALL_STATUS_TERMINATED:
>> +             isi_call_disconnected(ovc, call);
>
> Is the break statement missing here?

Hm? Where? Between IDLE and TERMINATED?

-- 
Pekka.Pessi mail at nokia.com

Re: [isi-voicecall-fix PATCHv3 2/6] isi/voicecall: fix status reporting

[Denis Kenzior]

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

Hi Pekka,

On 11/22/2010 10:09 AM, Pekka Pessi wrote:
> Hi Denis,
> 
>>> +enum clcc_status {
>>> +     CLCC_STATUS_ACTIVE = 0,
>>> +     CLCC_STATUS_HOLD = 1,
>>> +     CLCC_STATUS_DIALING = 2,
>>> +     CLCC_STATUS_ALERTING = 3,
>>> +     CLCC_STATUS_INCOMING = 4,
>>> +     CLCC_STATUS_WAITING = 5,
>>> +     CLCC_STATUS_DISCONNECTED = 6, /* Nonstandard */
>>> +};
>>
>> Please follow M11 completely here, particularly the tabs before the values.
> 
> You mean M11 preference? If you want something, be explicit, do not prefer.

Fair enough, fixed.

> 
> I laud your goal to improve the coding style within oFono - and it
> even might be best to do that step-by-step and avoid large patches
> just to fix the style - but I'm lazy typist and I usually just kill
> and yank pieces of code, like this one from from "src/common.h" patch
> a78b3629, authored by some Denis Kenzior.

Nobody is denying that the existing code has style violations.  But new
patches should not be propagating past mistakes or introducing new ones.
 This is your chance to fix it properly and my chance to catch these.

> 
> The current codebase contains a lot of code in outdated style and
> copy'n'paste coding just does not cut it, it would be ++good to have a
> custom checkpatch that would show the style problems without tedious
> and error-prone manual inspection. I'd do that myself but my perl-fu
> is bit rusty.
> 

I have said this before, I'm not against this at all, as long as someone
volunteers to maintain it properly.  So far there has been no takers.

>>> +     memcpy(number->number, call->address, sizeof number->number);
>>
>> Please watch out for the sizeof usage.
> 
> This needs an entry in doc/codingstyle.txt.
> 

Feel free to send a patch, you're the expert on this by now ;)

>>> +     case CALL_STATUS_IDLE:
>>> +     case CALL_STATUS_TERMINATED:
>>> +             isi_call_disconnected(ovc, call);
>>
>> Is the break statement missing here?
> 
> Hm? Where? Between IDLE and TERMINATED?
> 

Yes, in general I'd like a break statement after every case block.
Otherwise, when adding new case statements it is quite easy to introduce
hard to spot bugs.

Regards,
-Denis

Re: [isi-voicecall-fix PATCHv3 1/6] voicecall: fix dial result handling

[Denis Kenzior]

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

Hi Pekka,

On 11/16/2010 11:05 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> If there is an existing active call when dialing the existing call will
> be automatically put on hold. The dialing result handling depended on
> the voicecall driver putting the call on hold before the dial command
> callback is called.
> 
> However, this is not true on isimodem driver, where the dial request
> returns immediately before the existing call have changed its
> status. The dial result handling now checks if the active call has been
> resulted from the dial request.
> ---
>  src/voicecall.c |   18 ++++++++++++++++--
>  1 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/src/voicecall.c b/src/voicecall.c
> index bd64432..e3ce2a5 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -68,6 +68,7 @@ struct voicecall {
>  	char *message;
>  	uint8_t icon_id;
>  	gboolean untracked;
> +	gboolean dial_result_handled;
>  };
>  
>  struct dial_request {
> @@ -1096,9 +1097,20 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
>  		v = l->data;
>  
>  		if (v->call->status == CALL_STATUS_DIALING ||
> -				v->call->status == CALL_STATUS_ALERTING ||
> -				v->call->status == CALL_STATUS_ACTIVE)
> +				v->call->status == CALL_STATUS_ALERTING)
>  			return v;
> +
> +		/*
> +		 * Dial request may return before existing active call
> +		 * is put on hold or after dialed call has got active
> +		 */
> +		if (v->call->status == CALL_STATUS_ACTIVE &&
> +				v->call->direction ==
> +				CALL_DIRECTION_MOBILE_ORIGINATED &&
> +				!v->dial_result_handled) {
> +			v->dial_result_handled = TRUE;
> +			return v;
> +		}

I really don't see how this can work, since you never reset
dial_result_handled to FALSE anywhere.

>  	}
>  
>  	call = synthesize_outgoing_call(vc, number);
> @@ -1119,6 +1131,8 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
>  
>  	*need_to_emit = TRUE;
>  
> +	v->dial_result_handled = TRUE;
> +

dial_handle_result is ever called once, why do you need to set this here?

>  	return v;
>  }
>  

This seems to be the wrong way to go, is there any way for you to delay
returning from the dial operation until the call is put on hold?

Regards,
-Denis

Re: [isi-voicecall-fix PATCHv3 1/6] voicecall: fix dial result handling

[Pekka Pessi]

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

Hi Denis,

2010/11/22 Denis Kenzior <denkenz@gmail.com>:
>> If there is an existing active call when dialing the existing call will
>> be automatically put on hold. The dialing result handling depended on
>> the voicecall driver putting the call on hold before the dial command
>> callback is called.
>>
>> However, this is not true on isimodem driver, where the dial request
>> returns immediately before the existing call have changed its
>> status. The dial result handling now checks if the active call has been
>> resulted from the dial request.
>> ---
>>  src/voicecall.c |   18 ++++++++++++++++--
>>  1 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/voicecall.c b/src/voicecall.c
>> index bd64432..e3ce2a5 100644
>> --- a/src/voicecall.c
>> +++ b/src/voicecall.c
>> @@ -68,6 +68,7 @@ struct voicecall {
>>       char *message;
>>       uint8_t icon_id;
>>       gboolean untracked;
>> +     gboolean dial_result_handled;
>>  };
>>
>>  struct dial_request {
>> @@ -1096,9 +1097,20 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
>>               v = l->data;
>>
>>               if (v->call->status == CALL_STATUS_DIALING ||
>> -                             v->call->status == CALL_STATUS_ALERTING ||
>> -                             v->call->status == CALL_STATUS_ACTIVE)
>> +                             v->call->status == CALL_STATUS_ALERTING)
>>                       return v;
>> +
>> +             /*
>> +              * Dial request may return before existing active call
>> +              * is put on hold or after dialed call has got active
>> +              */
>> +             if (v->call->status == CALL_STATUS_ACTIVE &&
>> +                             v->call->direction ==
>> +                             CALL_DIRECTION_MOBILE_ORIGINATED &&
>> +                             !v->dial_result_handled) {
>> +                     v->dial_result_handled = TRUE;
>> +                     return v;
>> +             }
>
> I really don't see how this can work, since you never reset
> dial_result_handled to FALSE anywhere.

Huh? voicecall_create() uses g_new0().

For each outbound call there should be exactly one dial request, once
that is returned in dial_handle_result(), the dial_result_handled
should be TRUE.

I'll fix the above code.

>>       }
>>
>>       call = synthesize_outgoing_call(vc, number);
>> @@ -1119,6 +1131,8 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
>>
>>       *need_to_emit = TRUE;
>>
>> +     v->dial_result_handled = TRUE;
>> +
>
> dial_handle_result is ever called once, why do you need to set this here?

This prevents mixing this call with the call dialed next.

> This seems to be the wrong way to go, is there any way for you to delay
> returning from the dial operation until the call is put on hold?

While it is technically possible it does not make much sense.

-- 
Pekka.Pessi mail at nokia.com

Re: [isi-voicecall-fix PATCHv3 1/6] voicecall: fix dial result handling

[Denis Kenzior]

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

Hi Pekka,

>>> +     gboolean dial_result_handled;
>>>  };

So I'm finally beginning to understand where you're going with this..

>>>
>>>  struct dial_request {
>>> @@ -1096,9 +1097,20 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
>>>               v = l->data;
>>>
>>>               if (v->call->status == CALL_STATUS_DIALING ||
>>> -                             v->call->status == CALL_STATUS_ALERTING ||
>>> -                             v->call->status == CALL_STATUS_ACTIVE)
>>> +                             v->call->status == CALL_STATUS_ALERTING)
>>>                       return v;

So if I understand correctly, you want to set dial_result_handled to
TRUE here as well, right?

>>> +
>>> +             /*
>>> +              * Dial request may return before existing active call
>>> +              * is put on hold or after dialed call has got active
>>> +              */
>>> +             if (v->call->status == CALL_STATUS_ACTIVE &&
>>> +                             v->call->direction ==
>>> +                             CALL_DIRECTION_MOBILE_ORIGINATED &&
>>> +                             !v->dial_result_handled) {
>>> +                     v->dial_result_handled = TRUE;
>>> +                     return v;
>>> +             }
>>
>> I really don't see how this can work, since you never reset
>> dial_result_handled to FALSE anywhere.
> 
> Huh? voicecall_create() uses g_new0().
> 

So that is the piece I was missing.  Now I understand, thanks.

> For each outbound call there should be exactly one dial request, once
> that is returned in dial_handle_result(), the dial_result_handled
> should be TRUE.
> 
> I'll fix the above code.

Are you referring to setting dial_result_handled to TRUE for
ALERTING/DIALING or something else?

> 
>>>       }
>>>
>>>       call = synthesize_outgoing_call(vc, number);
>>> @@ -1119,6 +1131,8 @@ static struct voicecall *dial_handle_result(struct ofono_voicecall *vc,
>>>
>>>       *need_to_emit = TRUE;
>>>
>>> +     v->dial_result_handled = TRUE;
>>> +
>>
>> dial_handle_result is ever called once, why do you need to set this here?
> 
> This prevents mixing this call with the call dialed next.
>

And this one makes sense now as well.  So I'm actually fine with the
fix.  Do you want to resend the patch?

Regards,
-Denis