[PATCH v2 0/2] Add handling of XCOLP and refactor call status handling

[PATCH v2 0/2] Add handling of XCOLP and refactor call status handling

[Jeevaka Badrappan]

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

Hi,

 Incase of ifxmodem, COLP is not an intermediate status for ATD. Vendor specific
AT command XCOLP is used for the connected line presentation. Since XCALLSTAT
reports the voice call status information and call id, creation of the call is
moved inside xcallstat_notify function.

Regards,
Jeevaka

Jeevaka Badrappan (2):
  ifxmodem: add handling of XCOLP notification
  ifxmodem: refactor handling of call status

 drivers/ifxmodem/voicecall.c |  248 +++++++++++++++++++++++++-----------------
 1 files changed, 146 insertions(+), 102 deletions(-)

[PATCH v2 1/2] ifxmodem: add handling of XCOLP notification

[Jeevaka Badrappan]

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

---
 drivers/ifxmodem/voicecall.c |   51 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/drivers/ifxmodem/voicecall.c b/drivers/ifxmodem/voicecall.c
index f66815a..d31fa70 100644
--- a/drivers/ifxmodem/voicecall.c
+++ b/drivers/ifxmodem/voicecall.c
@@ -688,6 +688,55 @@ static void ccwa_notify(GAtResult *result, gpointer user_data)
 		ofono_voicecall_notify(vc, call);
 }
 
+static void xcolp_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_voicecall *vc = user_data;
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	GAtResultIter iter;
+	const char *num;
+	int type, call_id;
+	GSList *l;
+	struct ofono_call *call;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+XCOLP:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &call_id))
+		return;
+
+	if (!g_at_result_iter_next_string(&iter, &num))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &type))
+		return;
+
+	if (strlen(num) == 0) {
+		ofono_error("XCOLP received with invalid number!!!");
+		return;
+	}
+
+	DBG("xcolp_notify: %d %s %d", call_id, num, type);
+
+	l = g_slist_find_custom(vd->calls,
+				GINT_TO_POINTER(call_id),
+				at_util_call_compare_by_id);
+	if (l == NULL) {
+		ofono_error("XCOLP for unknown call");
+		return;
+	}
+
+	call = l->data;
+
+	strncpy(call->phone_number.number, num,	OFONO_MAX_PHONE_NUMBER_LENGTH);
+	call->phone_number.number[OFONO_MAX_PHONE_NUMBER_LENGTH] = '\0';
+	call->phone_number.type = type;
+	call->clip_validity = CLIP_VALIDITY_VALID;
+
+	ofono_voicecall_notify(vc, call);
+}
+
 static void ifx_voicecall_initialized(gboolean ok, GAtResult *result,
 					gpointer user_data)
 {
@@ -702,6 +751,7 @@ static void ifx_voicecall_initialized(gboolean ok, GAtResult *result,
 	g_at_chat_register(vd->chat, "+XEM:", xem_notify, FALSE, vc, NULL);
 	g_at_chat_register(vd->chat, "+XCALLSTAT:", xcallstat_notify,
 							FALSE, vc, NULL);
+	g_at_chat_register(vd->chat, "+XCOLP:", xcolp_notify, FALSE, vc, NULL);
 
 	ofono_voicecall_register(vc);
 }
@@ -722,6 +772,7 @@ static int ifx_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
 
 	g_at_chat_send(chat, "AT+XCALLSTAT=1", none_prefix, NULL, NULL, NULL);
 	g_at_chat_send(chat, "AT+XEMC=1", none_prefix, NULL, NULL, NULL);
+	g_at_chat_send(vd->chat, "AT+XCOLP=1", none_prefix, NULL, NULL, NULL);
 
 	g_at_chat_send(vd->chat, "AT+CRC=1", none_prefix, NULL, NULL, NULL);
 	g_at_chat_send(vd->chat, "AT+CLIP=1", none_prefix, NULL, NULL, NULL);
-- 
1.7.0.4

[PATCH v2 2/2] ifxmodem: refactor handling of call status

[Jeevaka Badrappan]

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

---
 drivers/ifxmodem/voicecall.c |  197 ++++++++++++++++++++----------------------
 1 files changed, 95 insertions(+), 102 deletions(-)

diff --git a/drivers/ifxmodem/voicecall.c b/drivers/ifxmodem/voicecall.c
index d31fa70..209ab13 100644
--- a/drivers/ifxmodem/voicecall.c
+++ b/drivers/ifxmodem/voicecall.c
@@ -43,9 +43,6 @@
 
 static const char *none_prefix[] = { NULL };
 
-/* According to 27.007 COLP is an intermediate status for ATD */
-static const char *atd_prefix[] = { "+COLP:", NULL };
-
 struct voicecall_data {
 	GSList *calls;
 	unsigned int local_release;
@@ -120,7 +117,8 @@ static void xcallstat_notify(GAtResult *result, gpointer user_data)
 	int id;
 	int status;
 	GSList *l;
-	struct ofono_call *call;
+	struct ofono_call *new_call;
+	struct ofono_call *existing_call = NULL;
 
 	g_at_result_iter_init(&iter, result);
 
@@ -136,53 +134,82 @@ static void xcallstat_notify(GAtResult *result, gpointer user_data)
 	l = g_slist_find_custom(vd->calls, GINT_TO_POINTER(id),
 				at_util_call_compare_by_id);
 
-	if (l == NULL) {
-		/*
-		 * We should only receive XCALLSTAT on waiting and incoming
-		 * In the case of waiting, we will get the rest of the info
-		 * from CCWA indication.
-		 * In the case of incoming, we will get the info from CLIP
-		 * indications.
-		 */
-		if (status != CALL_STATUS_INCOMING &&
-				    status != CALL_STATUS_WAITING) {
-			ofono_info("Received an XCALLSTAT for an untracked"
-					" call, this indicates a bug!");
-			return;
-		}
-
+	if (l == NULL && status != CALL_STATUS_DIALING &&
+				status != CALL_STATUS_INCOMING &&
+				status != CALL_STATUS_WAITING) {
+		ofono_error("Received an XCALLSTAT for an untracked"
+				" call, this indicates a bug!");
 		return;
 	}
 
-	call = l->data;
+	if (l)
+		existing_call = l->data;
 
-	/* Check if call has been disconnected */
-	if (status == CALL_STATUS_DISCONNECTED) {
-		enum ofono_disconnect_reason r;
+	switch (status) {
+	case CALL_STATUS_DISCONNECTED:
+	{
+		enum ofono_disconnect_reason reason;
 
-		if (vd->local_release & (1 << call->id))
-			r = OFONO_DISCONNECT_REASON_LOCAL_HANGUP;
-		else
-			r = OFONO_DISCONNECT_REASON_REMOTE_HANGUP;
+		existing_call->status = status;
 
-		if (call->type == 0)
-			ofono_voicecall_disconnected(vc, call->id, r, NULL);
+		if (vd->local_release & (1 << existing_call->id))
+			reason = OFONO_DISCONNECT_REASON_LOCAL_HANGUP;
+		else
+			reason = OFONO_DISCONNECT_REASON_REMOTE_HANGUP;
 
-		vd->local_release &= ~(1 << call->id);
-		vd->calls = g_slist_remove(vd->calls, call);
-		g_free(call);
+		ofono_voicecall_disconnected(vc, existing_call->id,
+						reason, NULL);
 
-		return;
+		vd->local_release &= ~(1 << existing_call->id);
+		vd->calls = g_slist_remove(vd->calls, l->data);
+		g_free(existing_call);
+		break;
 	}
+	case CALL_STATUS_DIALING:
+	case CALL_STATUS_WAITING:
+	case CALL_STATUS_INCOMING:
+	{
+		int direction;
+
+		/* Handle the following situation:
+		 * Active Call + Waiting Call. Active Call is Released.
+		 * The Waiting call becomes Incoming. In this case, no
+		 * need to create a new call. Call status change will be
+		 * triggered from clip_notify.
+		 */
+		if (existing_call) {
+			existing_call->status = status;
+			return;
+		}
 
-	/* For connected status, simply reset back to active */
-	if (status == 7)
-		status = 0;
+		if (status == CALL_STATUS_DIALING)
+			direction = CALL_DIRECTION_MOBILE_ORIGINATED;
+		else
+			direction = CALL_DIRECTION_MOBILE_TERMINATED;
 
-	call->status = status;
+		new_call = create_call(vc, 0, direction, status,
+					NULL, 128, CLIP_VALIDITY_NOT_AVAILABLE);
+		if (new_call == NULL) {
+			ofono_error("Unable to malloc. "
+					"Call management is fubar");
+			return;
+		}
 
-	if (call->type == 0)
-		ofono_voicecall_notify(vc, call);
+		new_call->id = id;
+		break;
+	}
+	case CALL_STATUS_ALERTING:
+	case CALL_STATUS_ACTIVE:
+	case CALL_STATUS_HELD:
+	default:
+		/* For connected status, simply reset back to active */
+		if (status == 7)
+			status = 0;
+
+		existing_call->status = status;
+		ofono_voicecall_notify(vc, existing_call);
+		break;
+	}
 }
 
 static void xem_notify(GAtResult *result, gpointer user_data)
@@ -253,51 +280,12 @@ static void release_id_cb(gboolean ok, GAtResult *result,
 static void atd_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	struct ofono_voicecall *vc = cbd->user;
 	ofono_voicecall_cb_t cb = cbd->cb;
-	GAtResultIter iter;
-	const char *num;
-	int type = 128;
-	int validity = 2;
 	struct ofono_error error;
-	struct ofono_call *call;
 
 	decode_at_error(&error, g_at_result_final_response(result));
 
-	if (!ok) {
-		cb(&error, cbd->data);
-		return;
-	}
-
-	g_at_result_iter_init(&iter, result);
-
-	if (g_at_result_iter_next(&iter, "+COLP:")) {
-		g_at_result_iter_next_string(&iter, &num);
-		g_at_result_iter_next_number(&iter, &type);
-
-		if (strlen(num) > 0)
-			validity = 0;
-		else
-			validity = 2;
-
-		DBG("colp_notify: %s %d %d", num, type, validity);
-	}
-
-	/* Generate a voice call that was just dialed, we guess the ID */
-	call = create_call(vc, 0, 0, CALL_STATUS_DIALING, num, type, validity);
-	if (call == NULL) {
-		ofono_error("Unable to malloc, call tracking will fail!");
-		return;
-	}
-
-	/* Let oFono core will generate a call with the dialed number
-	 * inside its dial callback.
-	 */
 	cb(&error, cbd->data);
-
-	/* If we got COLP information, then notify the core */
-	if (validity != 2)
-		ofono_voicecall_notify(vc, call);
 }
 
 static void ifx_dial(struct ofono_voicecall *vc,
@@ -329,7 +317,7 @@ static void ifx_dial(struct ofono_voicecall *vc,
 
 	strcat(buf, ";");
 
-	if (g_at_chat_send(vd->chat, buf, atd_prefix,
+	if (g_at_chat_send(vd->chat, buf, none_prefix,
 				atd_cb, cbd, g_free) > 0)
 		return;
 
@@ -537,25 +525,28 @@ static void cring_notify(GAtResult *result, gpointer user_data)
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
 	GAtResultIter iter;
 	const char *line;
-	int type;
+	GSList *l;
+	struct ofono_call *call;
 
 	/* Handle the following situation:
 	 * Active Call + Waiting Call.  Active Call is Released.  The Waiting
-	 * call becomes Incoming and RING/CRING indications are signaled.
-	 * Sometimes these arrive before we managed to poll CLCC to find about
-	 * the stage change.  If this happens, simply ignore the RING/CRING
-	 * when a waiting call exists (cannot have waiting + incoming in GSM)
+	 * call becomes Incoming and CRING indications are signaled.
+	 * Sometimes these arrive before the actual state change notification.
+	 * If this happens, simply ignore the CRING when a waiting call
+	 * exists (cannot have waiting + incoming in GSM)
 	 */
 	if (g_slist_find_custom(vd->calls,
 				GINT_TO_POINTER(CALL_STATUS_WAITING),
 				at_util_call_compare_by_status))
 		return;
 
-	/* CRING can repeat, ignore if we already have an incoming call */
-	if (g_slist_find_custom(vd->calls,
+	l = g_slist_find_custom(vd->calls,
 				GINT_TO_POINTER(CALL_STATUS_INCOMING),
-				at_util_call_compare_by_status))
+				at_util_call_compare_by_status);
+	if (l == NULL) {
+		ofono_error("CRING received before XCALLSTAT!!!");
 		return;
+	}
 
 	g_at_result_iter_init(&iter, result);
 
@@ -566,14 +557,13 @@ static void cring_notify(GAtResult *result, gpointer user_data)
 	if (line == NULL)
 		return;
 
+	call = l->data;
+
 	/* Ignore everything that is not voice for now */
 	if (!strcasecmp(line, "VOICE"))
-		type = 0;
+		call->type = 0;
 	else
-		type = 9;
-
-	/* Generate an incoming call */
-	create_call(vc, type, 1, CALL_STATUS_INCOMING, NULL, 128, 2);
+		call->type = 9;
 
 	/* Assume the CLIP always arrives, and we signal the call there */
 	DBG("cring_notify");
@@ -642,13 +632,16 @@ static void ccwa_notify(GAtResult *result, gpointer user_data)
 	GAtResultIter iter;
 	const char *num;
 	int num_type, validity, cls;
+	GSList *l;
 	struct ofono_call *call;
 
-	/* Some modems resend CCWA, ignore it the second time around */
-	if (g_slist_find_custom(vd->calls,
+	l = g_slist_find_custom(vd->calls,
 				GINT_TO_POINTER(CALL_STATUS_WAITING),
-				at_util_call_compare_by_status))
+				at_util_call_compare_by_status);
+	if (l == NULL) {
+		ofono_error("CCWA received before XCALLSTAT!!!");
 		return;
+	}
 
 	g_at_result_iter_init(&iter, result);
 
@@ -677,12 +670,13 @@ static void ccwa_notify(GAtResult *result, gpointer user_data)
 
 	DBG("ccwa_notify: %s %d %d %d", num, num_type, cls, validity);
 
-	call = create_call(vc, class_to_call_type(cls), 1, CALL_STATUS_WAITING,
-				num, num_type, validity);
-	if (call == NULL) {
-		ofono_error("Unable to malloc. Call management is fubar");
-		return;
-	}
+	call = l->data;
+
+	call->type = class_to_call_type(cls);
+	strncpy(call->phone_number.number, num, OFONO_MAX_PHONE_NUMBER_LENGTH);
+	call->phone_number.number[OFONO_MAX_PHONE_NUMBER_LENGTH] = '\0';
+	call->phone_number.type = num_type;
+	call->clip_validity = validity;
 
 	if (call->type == 0) /* Only notify voice calls */
 		ofono_voicecall_notify(vc, call);
@@ -776,7 +770,6 @@ static int ifx_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
 
 	g_at_chat_send(vd->chat, "AT+CRC=1", none_prefix, NULL, NULL, NULL);
 	g_at_chat_send(vd->chat, "AT+CLIP=1", none_prefix, NULL, NULL, NULL);
-	g_at_chat_send(vd->chat, "AT+COLP=1", none_prefix, NULL, NULL, NULL);
 	g_at_chat_send(vd->chat, "AT+CNAP=1", none_prefix, NULL, NULL, NULL);
 	g_at_chat_send(vd->chat, "AT+CCWA=1", none_prefix,
 				ifx_voicecall_initialized, vc, NULL);
-- 
1.7.0.4

Re: [PATCH v2 1/2] ifxmodem: add handling of XCOLP notification

[Marcel Holtmann]

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

Hi Jeevaka,

> @@ -722,6 +772,7 @@ static int ifx_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
>  
>  	g_at_chat_send(chat, "AT+XCALLSTAT=1", none_prefix, NULL, NULL, NULL);
>  	g_at_chat_send(chat, "AT+XEMC=1", none_prefix, NULL, NULL, NULL);
> +	g_at_chat_send(vd->chat, "AT+XCOLP=1", none_prefix, NULL, NULL, NULL);
>  
>  	g_at_chat_send(vd->chat, "AT+CRC=1", none_prefix, NULL, NULL, NULL);
>  	g_at_chat_send(vd->chat, "AT+CLIP=1", none_prefix, NULL, NULL, NULL);

I just saw a bug here. We need to execute the first two commands also on
vd->chat actually. We do wanna execute them on the clone and not the
original one.

Regards

Marcel