[PATCH 1/2] isimodem: fix problems in call state reporting

[PATCH 1/2] isimodem: fix problems in call state reporting

[Pekka.Pessi]

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

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

Do not report early incoming calls.

Report "disconnected" state separately.

Call ofono_voicecall_disconnected() only after call id is released.
---
 drivers/isimodem/voicecall.c |   84 ++++++++++++++++++++++++++++++------------
 1 files changed, 60 insertions(+), 24 deletions(-)

diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
index 6a1e582..6e23986 100644
--- a/drivers/isimodem/voicecall.c
+++ b/drivers/isimodem/voicecall.c
@@ -363,10 +363,13 @@ static int isi_call_status_to_clcc(const struct isi_call *call)
 	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_TERMINATED:
+		return 6;
+
+	case CALL_STATUS_ANSWERED:
+	case CALL_STATUS_ACTIVE:
 	case CALL_STATUS_HOLD_INITIATED:
 		return 0;
 
@@ -375,11 +378,10 @@ static int isi_call_status_to_clcc(const struct isi_call *call)
 		return 1;
 
 	case CALL_STATUS_RECONNECT_PENDING:
-	case CALL_STATUS_TERMINATED:
 	case CALL_STATUS_SWAP_INITIATED:
+	default:
 		return 0;
 	}
-	return 0;
 }
 
 static struct ofono_call isi_call_as_ofono_call(const struct isi_call *call)
@@ -433,13 +435,27 @@ static struct isi_call *isi_call_set_idle(struct isi_call *call)
 	return call;
 }
 
-static void isi_call_release(struct ofono_voicecall *ovc, struct isi_call *call)
+static void isi_call_disconnected(struct ofono_voicecall *ovc,
+					struct isi_call *call)
 {
 	struct ofono_error error = {
 		OFONO_ERROR_TYPE_NO_ERROR, 0
 	};
+
+	DBG("disconnected id=%u reason=%u", call->id, call->reason);
+
+	ofono_voicecall_disconnected(ovc, call->id, call->reason, &error);
+
+	isi_call_set_idle(call);
+}
+
+static void isi_call_set_disconnect_reason(struct isi_call *call)
+{
 	enum ofono_disconnect_reason reason;
 
+	if (call->reason != OFONO_DISCONNECT_REASON_UNKNOWN)
+		return;
+
 	switch (call->status) {
 	case CALL_STATUS_IDLE:
 		reason = OFONO_DISCONNECT_REASON_UNKNOWN;
@@ -456,17 +472,9 @@ static void isi_call_release(struct ofono_voicecall *ovc, struct isi_call *call)
 	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);
+	call->reason = reason;
 }
 
 static void isi_call_notify(struct ofono_voicecall *ovc, struct isi_call *call)
@@ -487,11 +495,20 @@ static void isi_call_notify(struct ofono_voicecall *ovc, struct isi_call *call)
 
 	switch (call->status) {
 	case CALL_STATUS_IDLE:
+		isi_call_disconnected(ovc, call);
+		return;
+
+	case CALL_STATUS_COMING:
+	case CALL_STATUS_PROCEEDING:
+		if ((call->mode_info & CALL_MODE_ORIGINATOR))
+			/* Do not notify early MT calls */
+			return;
+		break;
+
 	case CALL_STATUS_MO_RELEASE:
 	case CALL_STATUS_MT_RELEASE:
 	case CALL_STATUS_TERMINATED:
-		isi_call_release(ovc, call);
-		return;
+		isi_call_set_disconnect_reason(call);
 	}
 
 	ocall = isi_call_as_ofono_call(call);
@@ -615,17 +632,33 @@ static void isi_call_status_ind_cb(const GIsiMessage *msg, void *data)
 		}
 	}
 
-	if (old_status != call->status) {
+	if (old_status == call->status)
+		return;
 
-		if (call->status == CALL_STATUS_IDLE) {
-			call->status = CALL_STATUS_TERMINATED;
+	isi_call_notify(ovc, call);
+}
 
-			isi_call_notify(ovc, call);
-			isi_call_set_idle(call);
-			return;
-		}
-	}
+static void isi_call_terminated_ind_cb(const GIsiMessage *msg, void *data)
+{
+	struct ofono_voicecall *ovc = data;
+	struct isi_voicecall *ivc = ofono_voicecall_get_data(ovc);
+	struct isi_call *call;
 
+	uint8_t call_id;
+	uint8_t old_status;
+
+	if (ivc == NULL || g_isi_msg_id(msg) != CALL_TERMINATED_IND ||
+			!g_isi_msg_data_get_byte(msg, 0, &call_id) ||
+			(call_id & 7) == 0)
+		return;
+
+	call = &ivc->calls[call_id & 7];
+	old_status = call->status;
+
+	if (old_status == CALL_STATUS_IDLE)
+		return;
+
+	call->status = CALL_STATUS_TERMINATED;
 	isi_call_notify(ovc, call);
 }
 
@@ -1255,6 +1288,9 @@ static void isi_call_verify_cb(const GIsiMessage *msg, void *data)
 	g_isi_client_ind_subscribe(ivc->client, CALL_STATUS_IND,
 					isi_call_status_ind_cb, ovc);
 
+	g_isi_client_ind_subscribe(ivc->client, CALL_TERMINATED_IND,
+					isi_call_terminated_ind_cb, ovc);
+
 	if (!isi_call_status_req(ovc, CALL_ID_ALL,
 					CALL_STATUS_MODE_ADDR_AND_ORIGIN,
 					NULL, NULL))
-- 
1.7.1

[PATCH 2/2] voicecall: try harder to avoid Dial mismatches

[Pekka.Pessi]

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

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

There were some cases where Dial returned an incorrect call object,
e.g., after oFono crash and recover.
---
 src/voicecall.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 6246787..418e928 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -1160,6 +1160,17 @@ static ofono_bool_t clir_string_to_clir(const char *clirstr,
 	}
 }
 
+static void prepare_to_dial(struct ofono_voicecall *vc)
+{
+	GSList *l;
+	struct voicecall *v;
+
+	for (l = vc->call_list; l; l = l->next) {
+		v = l->data;
+		v->dial_result_handled = TRUE;
+	}
+}
+
 static struct ofono_call *synthesize_outgoing_call(struct ofono_voicecall *vc,
 							const char *number)
 {
@@ -1325,6 +1336,8 @@ static DBusMessage *manager_dial(DBusConnection *conn,
 
 	vc->pending = dbus_message_ref(msg);
 
+	prepare_to_dial(vc);
+
 	string_to_phone_number(number, &ph);
 
 	vc->driver->dial(vc, &ph, clir, manager_dial_callback, vc);
@@ -2461,6 +2474,8 @@ static void dial_request_cb(const struct ofono_error *error, void *data)
 
 static void dial_request(struct ofono_voicecall *vc)
 {
+	prepare_to_dial(vc);
+
 	vc->driver->dial(vc, &vc->dial_req->ph, OFONO_CLIR_OPTION_DEFAULT,
 				dial_request_cb, vc);
 }
-- 
1.7.1

Re: [PATCH 2/2] voicecall: try harder to avoid Dial mismatches

[Denis Kenzior]

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

Hi Pekka,

On 02/01/2011 04:22 PM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> There were some cases where Dial returned an incorrect call object,
> e.g., after oFono crash and recover.
> ---
>  src/voicecall.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 

So I'm guessing that you're restarting oFono and get
ofono_voicecall_notify events without setting dial_result_handled properly.

Do you think it is a better idea to resurrect the old list_calls driver
method and fire this off explicitly when the atom is registered?  The
callback can then take care of setting dial_result_handled appropriately.

Regards,
-Denis

Re: [PATCH 2/2] voicecall: try harder to avoid Dial mismatches

[Pekka Pessi]

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

Hi Denis,

2011/2/2 Denis Kenzior <denkenz@gmail.com>:
> On 02/01/2011 04:22 PM, Pekka.Pessi(a)nokia.com wrote:
>> From: Pekka Pessi <Pekka.Pessi@nokia.com>
>>
>> There were some cases where Dial returned an incorrect call object,
>> e.g., after oFono crash and recover.
>> ---
>>  src/voicecall.c |   15 +++++++++++++++
>>  1 files changed, 15 insertions(+), 0 deletions(-)
>
> So I'm guessing that you're restarting oFono and get
> ofono_voicecall_notify events without setting dial_result_handled properly.
>
> Do you think it is a better idea to resurrect the old list_calls driver
> method and fire this off explicitly when the atom is registered?  The
> callback can then take care of setting dial_result_handled appropriately.

That is one possibility.The belt-and-suspenders approach of
prepare_to_dial() makes oFono more robust in case there is someone
talking directly to modem behind oFono's back, however.

-- 
Pekka.Pessi mail at nokia.com

Re: [PATCH 2/2] voicecall: try harder to avoid Dial mismatches

[Denis Kenzior]

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

Hi Pekka,

On 02/02/2011 08:57 AM, Pekka Pessi wrote:
> Hi Denis,
> 
> 2011/2/2 Denis Kenzior <denkenz@gmail.com>:
>> On 02/01/2011 04:22 PM, Pekka.Pessi(a)nokia.com wrote:
>>> From: Pekka Pessi <Pekka.Pessi@nokia.com>
>>>
>>> There were some cases where Dial returned an incorrect call object,
>>> e.g., after oFono crash and recover.
>>> ---
>>>  src/voicecall.c |   15 +++++++++++++++
>>>  1 files changed, 15 insertions(+), 0 deletions(-)
>>
>> So I'm guessing that you're restarting oFono and get
>> ofono_voicecall_notify events without setting dial_result_handled properly.
>>
>> Do you think it is a better idea to resurrect the old list_calls driver
>> method and fire this off explicitly when the atom is registered?  The
>> callback can then take care of setting dial_result_handled appropriately.
> 
> That is one possibility.The belt-and-suspenders approach of
> prepare_to_dial() makes oFono more robust in case there is someone
> talking directly to modem behind oFono's back, however.
> 

This is not a case we support or will ever be willing to support in the
core anyway.  oFono performs too many optimizations internally and the
assumption is that oFono has exclusive control.

Which brings up a question, do we even want to bother performing crash
recovery or simply stipulate to the modem driver that the state should
be reset completely in case oFono crashes?  Remember that crash recovery
involves sim toolkit, voicecalls, gprs, ussd, etc.  It is pretty much
impossible to handle this properly...

Regards,
-Denis

Re: [PATCH 1/2] isimodem: fix problems in call state reporting

[Aki Niemi]

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

Hi Pekka,

2011/2/2  <Pekka.Pessi@nokia.com>:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
>
> Do not report early incoming calls.
>
> Report "disconnected" state separately.
>
> Call ofono_voicecall_disconnected() only after call id is released.
> ---
>  drivers/isimodem/voicecall.c |   84 ++++++++++++++++++++++++++++++------------
>  1 files changed, 60 insertions(+), 24 deletions(-)

Patch has been pushed. Thanks!

Cheers,
Aki