9:48 a.m.
Hi Jussi,
On 03/09/2011 04:27 AM, Jussi Kangas wrote:
---
Hi,
Here is a fix for call forwarding issue Jarko found a while back. ( TODO
issue "Call forwarding state handling change" and mailing list
conversation "Ofono CF states not always correct" ).
Br,
Jussi
src/call-forwarding.c | 78 ++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 70 insertions(+), 8 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index d13f990..6f81296 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -514,10 +514,24 @@ static DBusMessage *cf_get_properties_reply(DBusMessage *msg,
OFONO_PROPERTIES_ARRAY_SIGNATURE,
&dict);
- for (i = 0; i < 4; i++)
- property_append_cf_conditions(&dict, cf->cf_conditions[i],
+ if (cf->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL] == 0) {
Please use a comparison to NULL here instead of 0
+ DBG("Append all");
+ for (i = 0; i < 4; i++)
+ property_append_cf_conditions(&dict,
+ cf->cf_conditions[i],
+ BEARER_CLASS_VOICE,
+ cf_type_lut[i]);
Please fix the indentation levels here, the lines following
property_append... should all be the same indentation level.
Also, we might want to skip the unconditional call forwarding settings here.
+ } else {
+ DBG("append only unconditional");
+ property_append_cf_conditions(&dict,
+ cf->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL],
+ BEARER_CLASS_VOICE,
+ cf_type_lut[0]);
+ for (i = 1; i < 4; i++)
+ property_append_cf_conditions(&dict, NULL,
BEARER_CLASS_VOICE,
cf_type_lut[i]);
So I'm confused, the TODO entry says we should not return conditional
entries if the unconditional call forwarding is set as they are now
quiescent.
Are you proposing we handle this a bit differently?
+ }
if ((cf->flags & CALL_FORWARDING_FLAG_CPHS_CFF) ||
cf->cfis_record_id > 0)
@@ -682,8 +696,23 @@ static void set_query_cf_callback(const struct ofono_error *error,
int total,
static void set_query_next_cf_cond(struct ofono_call_forwarding *cf)
{
- cf->driver->query(cf, cf->query_next, BEARER_CLASS_DEFAULT,
- set_query_cf_callback, cf);
+ DBusMessage *reply;
+
+ while (cf->query_next != CALL_FORWARDING_TYPE_NOT_REACHABLE) {
+ if (!cf->cf_conditions[cf->query_next]) {
+ cf->driver->query(cf, cf->query_next,
+ BEARER_CLASS_DEFAULT,
+ set_query_cf_callback, cf);
+ return;
+ } else {
+ cf->query_next++;
+ if (cf->query_next == cf->query_end)
+ break;
+ }
+ }
+
+ reply = dbus_message_new_method_return(cf->pending);
+ __ofono_dbus_pending_reply(&cf->pending, reply);
Really lost here,
}
static void set_property_callback(const struct ofono_error *error, void *data)
@@ -836,6 +865,7 @@ static void disable_conditional_callback(const struct ofono_error
*error,
static void disable_all_callback(const struct ofono_error *error, void *data)
{
struct ofono_call_forwarding *cf = data;
+ int i;
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
DBG("Error occurred during erasure of all");
@@ -848,6 +878,10 @@ static void disable_all_callback(const struct ofono_error *error,
void *data)
/* Query all cf types */
cf->query_next = CALL_FORWARDING_TYPE_UNCONDITIONAL;
cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
+
+ for (i = 0; i < CALL_FORWARDING_TYPE_ALL; i++)
+ cf->cf_conditions[i] = 0;
+
For pointers please use NULL instead of 0.
Why are we setting them to NULL here in the first place? And shouldn't
you free the data?
set_query_next_cf_cond(cf);
}
@@ -1014,8 +1048,25 @@ static void ss_set_query_cf_callback(const struct ofono_error
*error, int total,
static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf)
{
- cf->driver->query(cf, cf->query_next, BEARER_CLASS_DEFAULT,
- ss_set_query_cf_callback, cf);
+ DBusMessage *reply;
+
+ while (cf->query_next != CALL_FORWARDING_TYPE_NOT_REACHABLE) {
+ if (!cf->cf_conditions[cf->query_next]) {
+ cf->driver->query(cf, cf->query_next,
+ BEARER_CLASS_DEFAULT,
+ ss_set_query_cf_callback, cf);
+ return;
+ } else {
+ cf->query_next++;
+ if (cf->query_next == cf->query_end)
+ break;
+ }
+ }
+
+ reply = cf_ss_control_reply(cf, cf->ss_req);
+ __ofono_dbus_pending_reply(&cf->pending, reply);
+ g_free(cf->ss_req);
+ cf->ss_req = NULL;
Again, very lost
}
static void cf_ss_control_callback(const struct ofono_error *error, void *data)
@@ -1032,6 +1083,11 @@ static void cf_ss_control_callback(const struct ofono_error
*error, void *data)
return;
}
+ if (cf->ss_req) {
+ if (cf->ss_req->ss_type == SS_CONTROL_TYPE_ERASURE)
+ cf->cf_conditions[cf->ss_req->cf_type] = 0;
+ }
+
ss_set_query_next_cf_cond(cf);
}
@@ -1180,8 +1236,14 @@ static gboolean cf_ss_control(int type, const char *sc,
cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
break;
case CALL_FORWARDING_TYPE_ALL_CONDITIONAL:
- cf->query_next = CALL_FORWARDING_TYPE_BUSY;
- cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
I don't see how handling both these cases with the same code can work...
+ case CALL_FORWARDING_TYPE_UNCONDITIONAL:
+ if (type == SS_CONTROL_TYPE_REGISTRATION) {
+ cf->query_next = cf->ss_req->cf_type;
+ cf->query_end = cf->ss_req->cf_type;
+ } else {
+ cf->query_next = cf->ss_req->cf_type;
+ cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
+ }
I'm a bit lost here, can you explain some more what you're trying to
accomplish?
break;
default:
cf->query_next = cf->ss_req->cf_type;
Regards,
-Denis