[PATCH v3 0/4] Add handling of XCOLP and refactor call status handling

[PATCH v3 0/4] Add handling of XCOLP and refactor call status handling

[Jeevaka Badrappan]

[-- Attachment #1: Type: text/plain, Size: 717 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. This also
fixes the issue with gatchat usage.

Regards,
Jeevaka

Jeevaka Badrappan (4):
  ifxmodem: fix issue with gatchat usage
  ifxmodem: add handling of XCOLP notification
  ifxmodem: remove colp handling done inside atd_cb
  ifxmodem: move call creation to xcallstat_notify

 drivers/ifxmodem/voicecall.c |  253 +++++++++++++++++++++++++-----------------
 1 files changed, 153 insertions(+), 100 deletions(-)

[PATCH v3 1/4] ifxmodem: fix issue with gatchat usage

[Jeevaka Badrappan]

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

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

diff --git a/drivers/ifxmodem/voicecall.c b/drivers/ifxmodem/voicecall.c
index f66815a..21e1944 100644
--- a/drivers/ifxmodem/voicecall.c
+++ b/drivers/ifxmodem/voicecall.c
@@ -720,8 +720,8 @@ static int ifx_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
 
 	ofono_voicecall_set_data(vc, vd);
 
-	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+XCALLSTAT=1", none_prefix, NULL, NULL, NULL);
+	g_at_chat_send(vd->chat, "AT+XEMC=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

Re: [PATCH v3 1/4] ifxmodem: fix issue with gatchat usage

[Denis Kenzior]

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

Hi Jeevaka,

On 02/07/2011 03:07 PM, Jeevaka Badrappan wrote:
> ---
>  drivers/ifxmodem/voicecall.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v3 2/4] ifxmodem: add handling of XCOLP notification

[Jeevaka Badrappan]

[-- Attachment #1: Type: text/plain, Size: 2650 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 21e1944..772280a 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(vd->chat, "AT+XCALLSTAT=1", none_prefix, NULL, NULL, NULL);
 	g_at_chat_send(vd->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

Re: [PATCH v3 2/4] ifxmodem: add handling of XCOLP notification

[Denis Kenzior]

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

Hi Jeevaka,

On 02/07/2011 03:07 PM, Jeevaka Badrappan wrote:
> ---
>  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 21e1944..772280a 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);
>  }
>  

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v3 3/4] ifxmodem: remove colp handling done inside atd_cb

[Jeevaka Badrappan]

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

---
 drivers/ifxmodem/voicecall.c |   30 ++----------------------------
 1 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/drivers/ifxmodem/voicecall.c b/drivers/ifxmodem/voicecall.c
index 772280a..648f46b 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;
@@ -255,10 +252,6 @@ 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;
 
@@ -269,22 +262,8 @@ static void atd_cb(gboolean ok, GAtResult *result, gpointer user_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);
+	call = create_call(vc, 0, 0, CALL_STATUS_DIALING, NULL, 128, 2);
 	if (call == NULL) {
 		ofono_error("Unable to malloc, call tracking will fail!");
 		return;
@@ -294,10 +273,6 @@ static void atd_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	 * 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 +304,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;
 
@@ -776,7 +751,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 v3 3/4] ifxmodem: remove colp handling done inside atd_cb

[Denis Kenzior]

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

Hi Jeevaka,

On 02/07/2011 03:07 PM, Jeevaka Badrappan wrote:
> ---
>  drivers/ifxmodem/voicecall.c |   30 ++----------------------------
>  1 files changed, 2 insertions(+), 28 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v3 4/4] ifxmodem: move call creation to xcallstat_notify

[Jeevaka Badrappan]

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

---
 drivers/ifxmodem/voicecall.c |  170 ++++++++++++++++++++++++-----------------
 1 files changed, 99 insertions(+), 71 deletions(-)

diff --git a/drivers/ifxmodem/voicecall.c b/drivers/ifxmodem/voicecall.c
index 648f46b..c94698b 100644
--- a/drivers/ifxmodem/voicecall.c
+++ b/drivers/ifxmodem/voicecall.c
@@ -117,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);
 
@@ -133,53 +134,83 @@ 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;
+
+		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;
+		}
 
-	call->status = status;
+		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 = CALL_STATUS_ACTIVE;
 
-	if (call->type == 0)
-		ofono_voicecall_notify(vc, call);
+		existing_call->status = status;
+		ofono_voicecall_notify(vc, existing_call);
+		break;
+	}
 }
 
 static void xem_notify(GAtResult *result, gpointer user_data)
@@ -250,25 +281,11 @@ 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;
 	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;
-	}
-
-	/* Generate a voice call that was just dialed, we guess the ID */
-	call = create_call(vc, 0, 0, CALL_STATUS_DIALING, NULL, 128, 2);
-	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.
 	 */
@@ -512,25 +529,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);
 
@@ -541,14 +561,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");
@@ -617,13 +636,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);
 
@@ -652,13 +674,19 @@ 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);
+
+	if (validity != 2) {
+		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);
 }
-- 
1.7.0.4

Re: [PATCH v3 4/4] ifxmodem: move call creation to xcallstat_notify

[Denis Kenzior]

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

Hi Jeevaka,

On 02/07/2011 03:07 PM, Jeevaka Badrappan wrote:
> ---
>  drivers/ifxmodem/voicecall.c |  170 ++++++++++++++++++++++++-----------------
>  1 files changed, 99 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/ifxmodem/voicecall.c b/drivers/ifxmodem/voicecall.c
> index 648f46b..c94698b 100644
> --- a/drivers/ifxmodem/voicecall.c
> +++ b/drivers/ifxmodem/voicecall.c
> @@ -117,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);
>  
> @@ -133,53 +134,83 @@ 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;
> +
> +		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;
> +		}
>  
> -	call->status = status;
> +		new_call->id = id;

So if you really insist on creating the call with a particular id, you
might as well make that a parameter of the create_call function.
However, do note that you still have one potential problem.  If the ATD
callback returns before the call is signaled, then oFono synthesizes an
outgoing call and picks an ID for you (according to the rules I
outlined).  If the ID that oFono picked doesn't match the ID you have,
you're in trouble.  So I'm not sure if you still want to go through all
this trouble or not...

> +		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 = CALL_STATUS_ACTIVE;
>  
> -	if (call->type == 0)
> -		ofono_voicecall_notify(vc, call);
> +		existing_call->status = status;
> +		ofono_voicecall_notify(vc, existing_call);
> +		break;
> +	}
>  }
>  
>  static void xem_notify(GAtResult *result, gpointer user_data)
> @@ -250,25 +281,11 @@ 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;
>  	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;
> -	}
> -
> -	/* Generate a voice call that was just dialed, we guess the ID */
> -	call = create_call(vc, 0, 0, CALL_STATUS_DIALING, NULL, 128, 2);
> -	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.
>  	 */
> @@ -512,25 +529,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);
>  
> @@ -541,14 +561,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");
> @@ -617,13 +636,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,

Can CCWA repeat on IFX?

>  				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);
>  
> @@ -652,13 +674,19 @@ 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);
> +
> +	if (validity != 2) {
> +		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);
>  }

Regards,
-Denis

RE: [PATCH v3 4/4] ifxmodem: move call creation to xcallstat_notify

[Jeevaka.Badrappan]

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

Hi Denis,

Denis Kenzior wrote:
> 
> So if you really insist on creating the call with a
> particular id, you might as well make that a parameter of the
> create_call function. However, do note that you still have one
> potential problem. 
> If the ATD callback returns before the call is signaled, then
> oFono synthesizes an outgoing call and picks an ID for you
> (according to the rules I outlined).  If the ID that oFono
> picked doesn't match the ID you have, you're in trouble.  So
> I'm not sure if you still want to go through all this trouble or
> not... 

Yep, agreed. Atleast with ifx I can confirm that ATD callback returns
before the call is signalled. I still want to have the call creation
done inside the xcallstat_notify so that call creation remains in
one place and also done in a place which has call id information.

>>  	/* Assume the CLIP always arrives, and we signal the call there
*/
>> DBG("cring_notify"); @@ -617,13 +636,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,
> 
> Can CCWA repeat on IFX?

CCWA comes only one time in IFX.

Regards,
Jeevaka