* [PATCH] call-forward: Call forwarding state handling
@ 2011-06-16 14:31 Nicolas Bertrand
2011-06-23 17:45 ` Denis Kenzior
0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Bertrand @ 2011-06-16 14:31 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3242 bytes --]
When CFU is active be cautious with conditional
call-forward activation/deactivation
---
src/call-forwarding.c | 46 ++++++++++++++++++++++++++++++++++++++++------
1 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 73ce433..eff5e9d 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -504,6 +504,7 @@ static DBusMessage *cf_get_properties_reply(DBusMessage *msg,
DBusMessageIter dict;
int i;
dbus_bool_t status;
+ GSList *hidden = NULL;
reply = dbus_message_new_method_return(msg);
if (reply == NULL)
@@ -515,17 +516,33 @@ 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],
- BEARER_CLASS_VOICE,
- cf_type_lut[i]);
-
if ((cf->flags & CALL_FORWARDING_FLAG_CPHS_CFF) ||
cf->cfis_record_id > 0)
status = is_cfu_enabled(cf, NULL);
else
status = FALSE;
+ /*
+ * If unconditional call-forwarding is enabled,
+ * hide conditionnal status
+ */
+ if (status == TRUE) {
+ struct ofono_call_forwarding_condition cd = {0, 0, {"", 0}, 0};
+
+ for (i = 0; i < 4; i++)
+ hidden = g_slist_prepend(hidden, &cd);
+ }
+
+ for (i = 0; i < 4; i++)
+ property_append_cf_conditions(&dict, (status &&
+ i != CALL_FORWARDING_TYPE_UNCONDITIONAL) ?
+ hidden : cf->cf_conditions[i],
+ BEARER_CLASS_VOICE,
+ cf_type_lut[i]);
+
+ if (status == TRUE)
+ g_slist_free(hidden);
+
ofono_dbus_dict_append(&dict, "ForwardingFlagOnSim", DBUS_TYPE_BOOLEAN,
&status);
@@ -552,6 +569,13 @@ static void get_query_cf_callback(const struct ofono_error *error, int total,
cf->flags |= CALL_FORWARDING_FLAG_CACHED;
}
+ if (cf->query_next == CALL_FORWARDING_TYPE_UNCONDITIONAL &&
+ is_cfu_enabled(cf, NULL) == TRUE) {
+ DBusMessage *reply = cf_get_properties_reply(cf->pending, cf);
+ __ofono_dbus_pending_reply(&cf->pending, reply);
+ return;
+ }
+
if (cf->query_next == CALL_FORWARDING_TYPE_NOT_REACHABLE) {
DBusMessage *reply = cf_get_properties_reply(cf->pending, cf);
__ofono_dbus_pending_reply(&cf->pending, reply);
@@ -575,7 +599,8 @@ static DBusMessage *cf_get_properties(DBusConnection *conn, DBusMessage *msg,
struct ofono_modem *modem = __ofono_atom_get_modem(cf->atom);
if ((cf->flags & CALL_FORWARDING_FLAG_CACHED) ||
- ofono_modem_get_online(modem) == FALSE)
+ ofono_modem_get_online(modem) == FALSE ||
+ is_cfu_enabled(cf, NULL) == TRUE)
return cf_get_properties_reply(msg, cf);
if (cf->driver->query == NULL)
@@ -698,6 +723,15 @@ static void set_property_callback(const struct ofono_error *error, void *data)
return;
}
+ if (cf->query_next != CALL_FORWARDING_TYPE_UNCONDITIONAL &&
+ is_cfu_enabled(cf, NULL) == TRUE) {
+ DBusMessage *reply;
+ cf->flags &= ~CALL_FORWARDING_FLAG_CACHED;
+ reply = dbus_message_new_method_return(cf->pending);
+ __ofono_dbus_pending_reply(&cf->pending, reply);
+ return;
+ }
+
/* Successfully set, query the entire set just in case */
set_query_next_cf_cond(cf);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] call-forward: Call forwarding state handling
2011-06-16 14:31 [PATCH] call-forward: Call forwarding state handling Nicolas Bertrand
@ 2011-06-23 17:45 ` Denis Kenzior
0 siblings, 0 replies; 3+ messages in thread
From: Denis Kenzior @ 2011-06-23 17:45 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5656 bytes --]
Hi Nicolas,
On 06/16/2011 09:31 AM, Nicolas Bertrand wrote:
> When CFU is active be cautious with conditional
> call-forward activation/deactivation
> ---
> src/call-forwarding.c | 46 ++++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 40 insertions(+), 6 deletions(-)
>
So I had to go back and re-read the thread that culminated in the need
for these changes. I include it here for reference:
http://lists.ofono.org/pipermail/ofono/2011-January/007234.html
In particular see my previous 'todo' thoughts:
http://lists.ofono.org/pipermail/ofono/2011-January/007721.html
> diff --git a/src/call-forwarding.c b/src/call-forwarding.c
> index 73ce433..eff5e9d 100644
> --- a/src/call-forwarding.c
> +++ b/src/call-forwarding.c
> @@ -504,6 +504,7 @@ static DBusMessage *cf_get_properties_reply(DBusMessage *msg,
> DBusMessageIter dict;
> int i;
> dbus_bool_t status;
> + GSList *hidden = NULL;
>
> reply = dbus_message_new_method_return(msg);
> if (reply == NULL)
> @@ -515,17 +516,33 @@ 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],
> - BEARER_CLASS_VOICE,
> - cf_type_lut[i]);
> -
> if ((cf->flags & CALL_FORWARDING_FLAG_CPHS_CFF) ||
> cf->cfis_record_id > 0)
> status = is_cfu_enabled(cf, NULL);
> else
> status = FALSE;
>
> + /*
> + * If unconditional call-forwarding is enabled,
> + * hide conditionnal status
> + */
> + if (status == TRUE) {
> + struct ofono_call_forwarding_condition cd = {0, 0, {"", 0}, 0};
> +
> + for (i = 0; i < 4; i++)
> + hidden = g_slist_prepend(hidden, &cd);
> + }
Lets not rely on status here, there can be situations where the EFcfis
and EFcff are not present on the SIM, or more likely the device does not
support reading of elementary EFs (e.g. N900). So relying on
is_cfu_enabled is better here.
> +
> + for (i = 0; i < 4; i++)
> + property_append_cf_conditions(&dict, (status &&
> + i != CALL_FORWARDING_TYPE_UNCONDITIONAL) ?
> + hidden : cf->cf_conditions[i],
> + BEARER_CLASS_VOICE,
> + cf_type_lut[i]);
> +
> + if (status == TRUE)
> + g_slist_free(hidden);
> +
This is the right idea, but the code could be a bit more readable,
albeit longer. Can you do something like:
for () {
GSList *l;
if (cfu_enabled && i != CALL_FORWARDING_TYPE_UNCONDITIONAL)
l = hidden;
else
l = cf->cf_conditions[i];
property_append_cf_conditions(&dict, i, l,
BEARER_CLASS_VOICE,
cf_type_lut[i]);
}
> ofono_dbus_dict_append(&dict, "ForwardingFlagOnSim", DBUS_TYPE_BOOLEAN,
> &status);
>
> @@ -552,6 +569,13 @@ static void get_query_cf_callback(const struct ofono_error *error, int total,
> cf->flags |= CALL_FORWARDING_FLAG_CACHED;
> }
>
> + if (cf->query_next == CALL_FORWARDING_TYPE_UNCONDITIONAL &&
> + is_cfu_enabled(cf, NULL) == TRUE) {
> + DBusMessage *reply = cf_get_properties_reply(cf->pending, cf);
> + __ofono_dbus_pending_reply(&cf->pending, reply);
> + return;
> + }
> +
This looks fine to me.
However, there's another path that needs to be taken care of. Namely
that we set the cond lists queried via supplementary service operations
in ss_set_query_cf_callback. You probably need to add extra logic there
to make sure that we don't overwrite / signal settings incorrectly.
In particular things can get funny if one queries conditional / all
settings when cfu is active. Do note that query all/query conditional
MMI is not valid, but oFono implements it anyway. If it makes it
easier, feel free to disable these.
> if (cf->query_next == CALL_FORWARDING_TYPE_NOT_REACHABLE) {
> DBusMessage *reply = cf_get_properties_reply(cf->pending, cf);
> __ofono_dbus_pending_reply(&cf->pending, reply);
> @@ -575,7 +599,8 @@ static DBusMessage *cf_get_properties(DBusConnection *conn, DBusMessage *msg,
> struct ofono_modem *modem = __ofono_atom_get_modem(cf->atom);
>
> if ((cf->flags & CALL_FORWARDING_FLAG_CACHED) ||
> - ofono_modem_get_online(modem) == FALSE)
> + ofono_modem_get_online(modem) == FALSE ||
> + is_cfu_enabled(cf, NULL) == TRUE)
I'm not sure about this change, I understand why you're doing it this
way. There are several places we reset the cached flag due to error
conditions. So to be on the safe side, perhaps we need to use two flags
for this? E.g. something telling us that we need to re-query
conditionals once cfu is disabled.
> return cf_get_properties_reply(msg, cf);
>
> if (cf->driver->query == NULL)
> @@ -698,6 +723,15 @@ static void set_property_callback(const struct ofono_error *error, void *data)
> return;
> }
>
> + if (cf->query_next != CALL_FORWARDING_TYPE_UNCONDITIONAL &&
> + is_cfu_enabled(cf, NULL) == TRUE) {
> + DBusMessage *reply;
> + cf->flags &= ~CALL_FORWARDING_FLAG_CACHED;
> + reply = dbus_message_new_method_return(cf->pending);
> + __ofono_dbus_pending_reply(&cf->pending, reply);
> + return;
> + }
> +
This looks fine to me (but see my comment up above). However, you must
also ensure that PropertyChanged signals are sent accordingly since you
have chosen to make conditional settings empty and vice-versa.
> /* Successfully set, query the entire set just in case */
> set_query_next_cf_cond(cf);
> }
One more request, can you implement this 'quiescent' behavior inside
phonesim? It would make it easier for us to stress-test these changes.
Thanks,
-Denis
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] call-forward: Call forwarding state handling
@ 2011-06-07 16:51 Nicolas Bertrand
0 siblings, 0 replies; 3+ messages in thread
From: Nicolas Bertrand @ 2011-06-07 16:51 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3227 bytes --]
When CFU is active be cautious with conditional
call-forward activation/deactivation
---
src/call-forwarding.c | 44 ++++++++++++++++++++++++++++++++++++++------
1 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 73ce433..42d681e 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -504,6 +504,7 @@ static DBusMessage *cf_get_properties_reply(DBusMessage *msg,
DBusMessageIter dict;
int i;
dbus_bool_t status;
+ GSList *hidden = NULL;
reply = dbus_message_new_method_return(msg);
if (reply == NULL)
@@ -515,17 +516,31 @@ 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],
- BEARER_CLASS_VOICE,
- cf_type_lut[i]);
-
if ((cf->flags & CALL_FORWARDING_FLAG_CPHS_CFF) ||
cf->cfis_record_id > 0)
status = is_cfu_enabled(cf, NULL);
else
status = FALSE;
+ // If unconditional call-forwarding is enabled,
+ // hide conditionnal status
+ if (status == TRUE) {
+ struct ofono_call_forwarding_condition cd = {0, 0, {"", 0}, 0};
+
+ for (i = 0; i < 4; i++)
+ hidden = g_slist_prepend(hidden, &cd);
+ }
+
+ for (i = 0; i < 4; i++)
+ property_append_cf_conditions(&dict, (status &&
+ i != CALL_FORWARDING_TYPE_UNCONDITIONAL) ?
+ hidden : cf->cf_conditions[i],
+ BEARER_CLASS_VOICE,
+ cf_type_lut[i]);
+
+ if (status == TRUE)
+ g_slist_free(hidden);
+
ofono_dbus_dict_append(&dict, "ForwardingFlagOnSim", DBUS_TYPE_BOOLEAN,
&status);
@@ -552,6 +567,13 @@ static void get_query_cf_callback(const struct ofono_error *error, int total,
cf->flags |= CALL_FORWARDING_FLAG_CACHED;
}
+ if (cf->query_next == CALL_FORWARDING_TYPE_UNCONDITIONAL &&
+ is_cfu_enabled(cf, NULL) == TRUE) {
+ DBusMessage *reply = cf_get_properties_reply(cf->pending, cf);
+ __ofono_dbus_pending_reply(&cf->pending, reply);
+ return;
+ }
+
if (cf->query_next == CALL_FORWARDING_TYPE_NOT_REACHABLE) {
DBusMessage *reply = cf_get_properties_reply(cf->pending, cf);
__ofono_dbus_pending_reply(&cf->pending, reply);
@@ -575,7 +597,8 @@ static DBusMessage *cf_get_properties(DBusConnection *conn, DBusMessage *msg,
struct ofono_modem *modem = __ofono_atom_get_modem(cf->atom);
if ((cf->flags & CALL_FORWARDING_FLAG_CACHED) ||
- ofono_modem_get_online(modem) == FALSE)
+ ofono_modem_get_online(modem) == FALSE ||
+ is_cfu_enabled(cf, NULL) == TRUE)
return cf_get_properties_reply(msg, cf);
if (cf->driver->query == NULL)
@@ -698,6 +721,15 @@ static void set_property_callback(const struct ofono_error *error, void *data)
return;
}
+ if (cf->query_next != CALL_FORWARDING_TYPE_UNCONDITIONAL &&
+ is_cfu_enabled(cf, NULL) == TRUE) {
+ DBusMessage *reply;
+ cf->flags &= ~CALL_FORWARDING_FLAG_CACHED;
+ reply = dbus_message_new_method_return(cf->pending);
+ __ofono_dbus_pending_reply(&cf->pending, reply);
+ return;
+ }
+
/* Successfully set, query the entire set just in case */
set_query_next_cf_cond(cf);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-06-23 17:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-16 14:31 [PATCH] call-forward: Call forwarding state handling Nicolas Bertrand
2011-06-23 17:45 ` Denis Kenzior
-- strict thread matches above, loose matches on Subject: below --
2011-06-07 16:51 Nicolas Bertrand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox