* [PATCHv2] isi: fix handling of incoming/waiting calls
@ 2010-11-08 19:49 Pekka.Pessi
2010-11-08 20:18 ` Denis Kenzior
0 siblings, 1 reply; 2+ messages in thread
From: Pekka.Pessi @ 2010-11-08 19:49 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 7661 bytes --]
From: Pekka Pessi <Pekka.Pessi@nokia.com>
Isimodem driver reports the mobile-terminated call as soon as possible,
however, it is not possible to answer a call before it is in
"MT-ALERTING" or "WAITING" state.
Also, report the state of an early mobile-terminated call to the core as
"waiting" or "incoming", depending on the state of the other calls.
---
drivers/isimodem/voicecall.c | 121 +++++++++++++++++++++++++++++++++++++-----
1 files changed, 108 insertions(+), 13 deletions(-)
diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
index c3365f6..511c38e 100644
--- a/drivers/isimodem/voicecall.c
+++ b/drivers/isimodem/voicecall.c
@@ -65,8 +65,10 @@ 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 ofono_call isi_call_as_ofono_call(struct isi_voicecall const *,
+ struct isi_call const *);
+static int isi_call_status_to_clcc(struct isi_voicecall const *,
+ struct isi_call const *);
static struct isi_call *isi_call_set_idle(struct isi_call *call);
typedef void GIsiIndication(GIsiClient *client,
@@ -97,12 +99,30 @@ 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;
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_context_new(struct ofono_voicecall *ovc,
+ ofono_voicecall_cb_t cb, void *data)
+{
+ struct isi_call_req_context *irc;
+
+ irc = g_try_new0(struct isi_call_req_context, 1);
+
+ if (irc) {
+ irc->ovc = ovc;
+ irc->cb = cb;
+ irc->data = data;
+ }
+
+ return irc;
+}
+
+static struct isi_call_req_context *
isi_call_req(struct ofono_voicecall *ovc,
void const *restrict req,
size_t len,
@@ -135,7 +155,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);
@@ -149,6 +170,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)
@@ -503,18 +525,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);
+
uint8_t const req[] = {
CALL_ANSWER_REQ, call_id, 0
};
size_t rlen = sizeof req;
+ int id = 0;
+
+ for (id = 1; id <= 7; id++) {
+ if ((ivc->calls[id].status == CALL_STATUS_PROCEEDING ||
+ ivc->calls[id].status == CALL_STATUS_COMING) &&
+ (ivc->calls[id].mode_info & CALL_MODE_ORIGINATOR))
+ break;
+ }
+
+ if (id <= 7) {
+ struct isi_call_req_context *irc;
+ irc = isi_call_req_context_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, CALL_ID_ALL, 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,
@@ -830,11 +899,11 @@ static void isi_call_notify(struct ofono_voicecall *ovc,
return;
}
- ocall = isi_call_as_ofono_call(call);
+ ocall = isi_call_as_ofono_call(ivc, call);
- DBG("id=%u,%s,%u,\"%s\",%u,%u",
+ DBG("id=%u,\"%s\",%u,\"%s\",%u,%u",
ocall.id,
- ocall.direction ? "terminated" : "originated",
+ ocall.direction ? "mt" : "mo",
ocall.status,
ocall.phone_number.number,
ocall.phone_number.type,
@@ -875,14 +944,15 @@ static void isi_call_release(struct ofono_voicecall *ovc,
isi_call_set_idle(call);
}
-static struct ofono_call isi_call_as_ofono_call(struct isi_call const *call)
+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(call);
+ 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;
@@ -892,17 +962,41 @@ static struct ofono_call isi_call_as_ofono_call(struct isi_call const *call)
return ocall;
}
+static int isi_call_waiting_or_incoming(struct isi_voicecall const *ivc,
+ struct isi_call const *call)
+{
+ int id = 0;
+
+ for (id = 1; id <= 7; id++) {
+ switch (ivc->calls[id].status) {
+ case CALL_STATUS_ANSWERED:
+ case CALL_STATUS_ACTIVE:
+ case CALL_STATUS_MO_RELEASE:
+ case CALL_STATUS_MT_RELEASE:
+ 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 5; /* waiting */
+ }
+ }
+
+ return 4; /* incoming */
+}
+
/** Get +CLCC status */
-static int isi_call_status_to_clcc(struct isi_call const *call)
+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 2;
case CALL_STATUS_COMING:
- return 4;
+ return isi_call_waiting_or_incoming(ivc, call);
case CALL_STATUS_PROCEEDING:
if ((call->mode_info & CALL_MODE_ORIGINATOR))
- return 4; /* MT */
+ return isi_call_waiting_or_incoming(ivc, call); /* MT */
else
return 2; /* MO */
case CALL_STATUS_MO_ALERTING:
@@ -1072,9 +1166,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);
}
@@ -1149,6 +1243,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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCHv2] isi: fix handling of incoming/waiting calls
2010-11-08 19:49 [PATCHv2] isi: fix handling of incoming/waiting calls Pekka.Pessi
@ 2010-11-08 20:18 ` Denis Kenzior
0 siblings, 0 replies; 2+ messages in thread
From: Denis Kenzior @ 2010-11-08 20:18 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 9157 bytes --]
Hi Pekka,
On 11/08/2010 01:49 PM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
>
> Isimodem driver reports the mobile-terminated call as soon as possible,
> however, it is not possible to answer a call before it is in
> "MT-ALERTING" or "WAITING" state.
>
> Also, report the state of an early mobile-terminated call to the core as
> "waiting" or "incoming", depending on the state of the other calls.
> ---
> drivers/isimodem/voicecall.c | 121 +++++++++++++++++++++++++++++++++++++-----
> 1 files changed, 108 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
> index c3365f6..511c38e 100644
> --- a/drivers/isimodem/voicecall.c
> +++ b/drivers/isimodem/voicecall.c
> @@ -65,8 +65,10 @@ 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 ofono_call isi_call_as_ofono_call(struct isi_voicecall const *,
> + struct isi_call const *);
> +static int isi_call_status_to_clcc(struct isi_voicecall const *,
> + struct isi_call const *);
Is there really a need for all these forward declarations? Try to avoid
these whenever possible. Also, the general convention so far has been
to use const struct foo *, not struct foo const *. They are equivalent,
but people with non-C++ background are typically more used to the first one.
> static struct isi_call *isi_call_set_idle(struct isi_call *call);
>
> typedef void GIsiIndication(GIsiClient *client,
> @@ -97,12 +99,30 @@ 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;
> 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_context_new(struct ofono_voicecall *ovc,
> + ofono_voicecall_cb_t cb, void *data)
> +{
> + struct isi_call_req_context *irc;
> +
> + irc = g_try_new0(struct isi_call_req_context, 1);
> +
> + if (irc) {
> + irc->ovc = ovc;
> + irc->cb = cb;
> + irc->data = data;
> + }
> +
> + return irc;
> +}
> +
> +static struct isi_call_req_context *
> isi_call_req(struct ofono_voicecall *ovc,
> void const *restrict req,
> size_t len,
> @@ -135,7 +155,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);
> @@ -149,6 +170,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)
> @@ -503,18 +525,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);
> +
And another forward declaration of 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);
> +
> uint8_t const req[] = {
> CALL_ANSWER_REQ, call_id, 0
> };
> size_t rlen = sizeof req;
>
> + int id = 0;
why do you bother initializing this variable here? You're
re-initializing it again 2 lines down. Please see doc/coding-style.txt
item M7.
> +
> + for (id = 1; id <= 7; id++) {
> + if ((ivc->calls[id].status == CALL_STATUS_PROCEEDING ||
> + ivc->calls[id].status == CALL_STATUS_COMING) &&
> + (ivc->calls[id].mode_info & CALL_MODE_ORIGINATOR))
> + break;
Please see coding-style.txt item M4.
> + }
> +
> + if (id <= 7) {
> + struct isi_call_req_context *irc;
> + irc = isi_call_req_context_new(ovc, cb, data);
> + if (irc) {
> + isi_ctx_queue(irc, isi_wait_incoming, id);
> + return irc;
> + }
> + }
> +
This code might be easier to follow if you move the if condition into
the for loop above and use a continue statement as needed.
> 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, CALL_ID_ALL, irc->cb, irc->data);
> + isi_ctx_free(irc);
> + return;
> + }
> +
> + ivc = ofono_voicecall_get_data(irc->ovc);
> + switch (ivc->calls[irc->id].status) {
Rule M2 applies here as well.
> + 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,
> @@ -830,11 +899,11 @@ static void isi_call_notify(struct ofono_voicecall *ovc,
> return;
> }
>
> - ocall = isi_call_as_ofono_call(call);
> + ocall = isi_call_as_ofono_call(ivc, call);
>
> - DBG("id=%u,%s,%u,\"%s\",%u,%u",
> + DBG("id=%u,\"%s\",%u,\"%s\",%u,%u",
> ocall.id,
> - ocall.direction ? "terminated" : "originated",
> + ocall.direction ? "mt" : "mo",
> ocall.status,
> ocall.phone_number.number,
> ocall.phone_number.type,
> @@ -875,14 +944,15 @@ static void isi_call_release(struct ofono_voicecall *ovc,
> isi_call_set_idle(call);
> }
>
> -static struct ofono_call isi_call_as_ofono_call(struct isi_call const *call)
> +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(call);
> + 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;
> @@ -892,17 +962,41 @@ static struct ofono_call isi_call_as_ofono_call(struct isi_call const *call)
> return ocall;
> }
>
> +static int isi_call_waiting_or_incoming(struct isi_voicecall const *ivc,
> + struct isi_call const *call)
> +{
> + int id = 0;
And again, rule M7 being broken
> +
> + for (id = 1; id <= 7; id++) {
> + switch (ivc->calls[id].status) {
> + case CALL_STATUS_ANSWERED:
> + case CALL_STATUS_ACTIVE:
> + case CALL_STATUS_MO_RELEASE:
> + case CALL_STATUS_MT_RELEASE:
> + 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 5; /* waiting */
> + }
> + }
> +
You don't use variable call at all here...
> + return 4; /* incoming */
> +}
> +
> /** Get +CLCC status */
> -static int isi_call_status_to_clcc(struct isi_call const *call)
> +static int isi_call_status_to_clcc(struct isi_voicecall const *ivc,
> + struct isi_call const *call)
You add a new parameter to this function to pass into the target
function, but end up not using it in the target. Hmm... ;)
> {
> switch (call->status) {
> case CALL_STATUS_CREATE:
> return 2;
> case CALL_STATUS_COMING:
> - return 4;
> + return isi_call_waiting_or_incoming(ivc, call);
> case CALL_STATUS_PROCEEDING:
> if ((call->mode_info & CALL_MODE_ORIGINATOR))
> - return 4; /* MT */
> + return isi_call_waiting_or_incoming(ivc, call); /* MT */
> else
> return 2; /* MO */
> case CALL_STATUS_MO_ALERTING:
> @@ -1072,9 +1166,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);
> }
> @@ -1149,6 +1243,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;
Regards,
-Denis
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-11-08 20:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-08 19:49 [PATCHv2] isi: fix handling of incoming/waiting calls Pekka.Pessi
2010-11-08 20:18 ` Denis Kenzior
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.