[PATCH v1 0/3] HFP driver improvements

[PATCH v1 0/3] HFP driver improvements

[Mikel Astiz]

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

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

v1 fixes the comment added in patch v0 2/3, otherwise the rest stays the same.

From original cover-letter:

While testing the HFP modem driver with several phones, I found out an issue with the Nokia 500, as described in patch v0 3/3.

The first two patches address a somehow unrelated issue regarding how call states should be reported.

Mikel Astiz (3):
  hfpmodem: Refactor voicecall notify with foreach
  hfpmodem: Avoid transitional voicecall states
  hfpmodem: Fix release-and-swap without +CIEV

 drivers/hfpmodem/voicecall.c | 72 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 66 insertions(+), 6 deletions(-)

-- 
1.7.11.7

[PATCH v1 1/3] hfpmodem: Refactor voicecall notify with foreach

[Mikel Astiz]

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

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Define a helper function in order to use foreach statements when
ofono_voicecall_notify() needs to be called.
---
 drivers/hfpmodem/voicecall.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/hfpmodem/voicecall.c b/drivers/hfpmodem/voicecall.c
index 505601c..9770059 100644
--- a/drivers/hfpmodem/voicecall.c
+++ b/drivers/hfpmodem/voicecall.c
@@ -91,6 +91,14 @@ static GSList *find_dialing(GSList *calls)
 	return c;
 }
 
+static void voicecall_notify(gpointer value, gpointer user)
+{
+	struct ofono_call *call = value;
+	struct ofono_voicecall *vc = user;
+
+	ofono_voicecall_notify(vc, call);
+}
+
 static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
 					int direction, int status,
 					const char *num, int num_type, int clip)
@@ -1029,15 +1037,13 @@ static void hfp_clcc_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_voicecall *vc = user_data;
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
-	GSList *l;
 
 	if (!ok)
 		return;
 
 	vd->calls = at_util_parse_clcc(result);
 
-	for (l = vd->calls; l; l = l->next)
-		ofono_voicecall_notify(vc, l->data);
+	g_slist_foreach(vd->calls, voicecall_notify, vc);
 }
 
 static void hfp_voicecall_initialized(gboolean ok, GAtResult *result,
-- 
1.7.11.7

[PATCH v1 2/3] hfpmodem: Avoid transitional voicecall states

[Mikel Astiz]

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

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

While processing the result of AT+CLCC, process the differences in a way
that disconnections are reported first, then call state changes and
finally new calls.

The goal is to avoid unnecessary transitional states such as two active
calls existing at the same time.
---
 drivers/hfpmodem/voicecall.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hfpmodem/voicecall.c b/drivers/hfpmodem/voicecall.c
index 9770059..e0da3fc 100644
--- a/drivers/hfpmodem/voicecall.c
+++ b/drivers/hfpmodem/voicecall.c
@@ -205,6 +205,7 @@ static void clcc_poll_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_call *nc, *oc;
 	unsigned int num_active = 0;
 	unsigned int num_held = 0;
+	GSList *notify_calls = NULL;
 
 	if (!ok)
 		return;
@@ -242,7 +243,7 @@ static void clcc_poll_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		} else if (nc && (oc == NULL || (nc->id < oc->id))) {
 			/* new call, signal it */
 			if (nc->type == 0)
-				ofono_voicecall_notify(vc, nc);
+				notify_calls = g_slist_append(notify_calls, nc);
 
 			n = n->next;
 		} else {
@@ -257,13 +258,22 @@ static void clcc_poll_cb(gboolean ok, GAtResult *result, gpointer user_data)
 
 			if (memcmp(nc, oc, sizeof(struct ofono_call)) &&
 					!nc->type)
-				ofono_voicecall_notify(vc, nc);
+				notify_calls = g_slist_prepend(notify_calls,
+									nc);
 
 			n = n->next;
 			o = o->next;
 		}
 	}
 
+	/*
+	 * Disconnections were already reported, so process the rest of the
+	 * notifications. Note that the new calls are placed at the end of the
+	 * list, after other state changes
+	 */
+	g_slist_foreach(notify_calls, voicecall_notify, vc);
+	g_slist_free(notify_calls);
+
 	g_slist_foreach(vd->calls, (GFunc) g_free, NULL);
 	g_slist_free(vd->calls);
 
-- 
1.7.11.7

[PATCH v1 3/3] hfpmodem: Fix release-and-swap without +CIEV

[Mikel Astiz]

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

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Some phones do not send the corresponding call state update (+CIEV)
after a successful release-and-swap operation (AT+CHLD=1).

This has been observed with a Nokia 500, while testing ReleaseAndSwap()
while an active and a held call exist:

ofonod[20414]: > AT+CLCC\r
ofonod[20414]: < \r\n+CLCC: 1,0,1,0,0,"<number1>",145\r\n
ofonod[20414]: < \r\n+CLCC: 2,0,0,0,0,"<number2>",145\r\n
ofonod[20414]: < \r\nOK\r\n
ofonod[20414]: > AT+CHLD=1\r
ofonod[20414]: < \r\nOK\r\n

After this, no +CIEV is received, but the call has been hung up.

The proposed approach to solve this consists of using AT+CLCC, unless
a call release has been received within a specific time period.

The result fixes the problem as can be seen below:

ofonod[20847]: < \r\n+CLCC: 1,0,1,0,0,"<number1>",145\r\n
ofonod[20847]: < \r\n+CLCC: 2,0,0,0,0,"<number2>",145\r\n
ofonod[20847]: < \r\nOK\r\n
ofonod[20847]: > AT+CHLD=1\r
ofonod[20847]: < \r\nOK\r\n
ofonod[20847]: > AT+CLCC\r
ofonod[20847]: < \r\n+CLCC: 1,0,0,0,0,"<number1>",145\r\n
ofonod[20847]: < \r\nOK\r\n
ofonod[20847]: < \r\n+CIEV: 5,2\r\n
ofonod[20847]: < \r\n+CIEV: 5,0\r\n
---
 drivers/hfpmodem/voicecall.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/hfpmodem/voicecall.c b/drivers/hfpmodem/voicecall.c
index e0da3fc..39d81f1 100644
--- a/drivers/hfpmodem/voicecall.c
+++ b/drivers/hfpmodem/voicecall.c
@@ -43,6 +43,7 @@
 
 #define POLL_CLCC_INTERVAL 2000
 #define POLL_CLCC_DELAY 50
+#define EXPECT_RELEASE_DELAY 50
 #define CLIP_TIMEOUT 500
 
 static const char *none_prefix[] = { NULL };
@@ -57,6 +58,7 @@ struct voicecall_data {
 	int cind_val[HFP_INDICATOR_LAST];
 	unsigned int local_release;
 	unsigned int clcc_source;
+	unsigned int expect_release_source;
 	unsigned int clip_source;
 };
 
@@ -194,6 +196,11 @@ static void release_with_status(struct ofono_voicecall *vc, int status)
 		c = c->next;
 		g_slist_free_1(t);
 	}
+
+	if (vd->expect_release_source) {
+		g_source_remove(vd->expect_release_source);
+		vd->expect_release_source = 0;
+	}
 }
 
 static void clcc_poll_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -471,13 +478,47 @@ static void hfp_set_udub(struct ofono_voicecall *vc,
 	CALLBACK_WITH_FAILURE(cb, data);
 }
 
+static gboolean expect_release(gpointer user_data)
+{
+	struct ofono_voicecall *vc = user_data;
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+
+	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
+				clcc_poll_cb, vc, NULL);
+
+	vd->expect_release_source = 0;
+
+	return FALSE;
+}
+
+static void release_all_active_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
+{
+	struct change_state_req *req = user_data;
+	struct voicecall_data *vd = ofono_voicecall_get_data(req->vc);
+
+	if (!ok)
+		goto out;
+
+	if (vd->expect_release_source)
+		g_source_remove(vd->expect_release_source);
+
+	vd->expect_release_source = g_timeout_add(EXPECT_RELEASE_DELAY,
+							expect_release,
+							req->vc);
+
+out:
+	generic_cb(ok, result, user_data);
+}
+
 static void hfp_release_all_active(struct ofono_voicecall *vc,
 					ofono_voicecall_cb_t cb, void *data)
 {
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
 
 	if (vd->ag_mpty_features & AG_CHLD_1) {
-		hfp_template("AT+CHLD=1", vc, generic_cb, 0x1, cb, data);
+		hfp_template("AT+CHLD=1", vc, release_all_active_cb, 0x1, cb,
+									data);
 		return;
 	}
 
@@ -1111,6 +1152,9 @@ static void hfp_voicecall_remove(struct ofono_voicecall *vc)
 	if (vd->clip_source)
 		g_source_remove(vd->clip_source);
 
+	if (vd->expect_release_source)
+		g_source_remove(vd->expect_release_source);
+
 	g_slist_foreach(vd->calls, (GFunc) g_free, NULL);
 	g_slist_free(vd->calls);
 
-- 
1.7.11.7

Re: [PATCH v1 0/3] HFP driver improvements

[Denis Kenzior]

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

Hi Mikel,

On 01/21/2013 09:30 AM, Mikel Astiz wrote:
> From: Mikel Astiz<mikel.astiz@bmw-carit.de>
>
> v1 fixes the comment added in patch v0 2/3, otherwise the rest stays the same.
>
>  From original cover-letter:
>
> While testing the HFP modem driver with several phones, I found out an issue with the Nokia 500, as described in patch v0 3/3.
>
> The first two patches address a somehow unrelated issue regarding how call states should be reported.
>
> Mikel Astiz (3):
>    hfpmodem: Refactor voicecall notify with foreach
>    hfpmodem: Avoid transitional voicecall states
>    hfpmodem: Fix release-and-swap without +CIEV
>
>   drivers/hfpmodem/voicecall.c | 72 ++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 66 insertions(+), 6 deletions(-)
>

All three patches have been applied, thanks.

Regards,
-Denis