Hi Jeevaka,
On 11/29/2010 04:37 AM, Jeevaka Badrappan wrote:
> ---
> src/call-forwarding.c | 266 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 260 insertions(+), 6 deletions(-)
>
> diff --git a/src/call-forwarding.c b/src/call-forwarding.c
> index ce03c40..bb345a6 100644
> --- a/src/call-forwarding.c
> +++ b/src/call-forwarding.c
> @@ -34,6 +34,7 @@
> #include "ofono.h"
>
> #include "common.h"
> +#include "simutil.h"
>
> #define CALL_FORWARDING_FLAG_CACHED 0x1
>
> @@ -58,6 +59,12 @@ struct ofono_call_forwarding {
> int query_next;
> int query_end;
> struct cf_ss_request *ss_req;
> + struct ofono_sim *sim;
> + unsigned char cfis_record_id;
> + unsigned char cfis_indicator;
> + ofono_bool_t cphs_cff_present;
> + ofono_bool_t status_on_sim;
> + ofono_bool_t online;
Why do you need to track this variable? Can't you simply call
ofono_modem_get_online()?
> struct ofono_ussd *ussd;
> unsigned int ussd_watch;
> const struct ofono_call_forwarding_driver *driver;
<snip>
> @@ -372,6 +458,7 @@ static DBusMessage *cf_get_properties_reply(DBusMessage
> *msg,
> DBusMessageIter iter;
> DBusMessageIter dict;
> int i;
> + dbus_bool_t status;
>
> reply = dbus_message_new_method_return(msg);
>
> @@ -384,10 +471,21 @@ 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->online == TRUE) {
So I'm really confused by this one, if we're offline and haven't managed
to query the conditions, just return them. They will be all empty with
the possible exception of VoiceUnconditional
> + for (i = 0; i < 4; i++)
> + property_append_cf_conditions(&dict,
> + cf->cf_conditions[i],
> BEARER_CLASS_VOICE,
> cf_type_lut[i]);
> + } else if (cf->status_on_sim == TRUE)
> + property_append_cf_conditions(&dict,
> + cf->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL],
> + BEARER_CLASS_VOICE,
> + cf_type_lut[CALL_FORWARDING_TYPE_UNCONDITIONAL]);
> +
> + status = cf->status_on_sim;
> + ofono_dbus_dict_append(&dict, "ForwardingFlagOnSim", DBUS_TYPE_BOOLEAN,
> + &status);
>
> dbus_message_iter_close_container(&iter, &dict);
>
<snip>
> @@ -433,7 +539,8 @@ static DBusMessage *cf_get_properties(DBusConnection
> *conn, DBusMessage *msg,
> {
> struct ofono_call_forwarding *cf = data;
>
> - if (cf->flags & CALL_FORWARDING_FLAG_CACHED)
> + if ((cf->flags & CALL_FORWARDING_FLAG_CACHED) ||
> + cf->online == FALSE)
Why do you split this on two lines? Are you sure it won't fit on one?
> return cf_get_properties_reply(msg, cf);
>
> if (!cf->driver->query)
> @@ -536,7 +643,8 @@ static void set_query_cf_callback(const struct
> ofono_error *error, int total,
> if (cf->query_next != cf->query_end) {
> cf->query_next++;
> set_query_next_cf_cond(cf);
> - }
> + } else
> + sim_set_cf_indicator(cf);
Should we do this only if we actually queried the set that includes
unconditional?
> }
>
> static void set_query_next_cf_cond(struct ofono_call_forwarding *cf)
> @@ -597,6 +705,9 @@ static DBusMessage *cf_set_property(DBusConnection *conn,
> DBusMessage *msg,
> int cls;
> int type;
>
> + if (cf->online == FALSE)
> + return __ofono_error_not_available(msg);
> +
> if (__ofono_call_forwarding_is_busy(cf) ||
> __ofono_ussd_is_busy(cf->ussd))
> return __ofono_error_busy(msg);
> @@ -864,7 +975,8 @@ static void ss_set_query_cf_callback(const struct
> ofono_error *error, int total,
> if (cf->query_next != cf->query_end) {
> cf->query_next++;
> ss_set_query_next_cf_cond(cf);
> - }
> + } else
> + sim_set_cf_indicator(cf);
Should we do this only if we actually queried the set that includes
unconditional?
> }
>
> static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf)
<snip>
> @@ -1220,6 +1461,7 @@ void ofono_call_forwarding_register(struct
> ofono_call_forwarding *cf)
> DBusConnection *conn = ofono_dbus_get_connection();
> const char *path = __ofono_atom_get_path(cf->atom);
> struct ofono_modem *modem = __ofono_atom_get_modem(cf->atom);
> + struct ofono_atom *sim_atom;
> struct ofono_atom *ussd_atom;
>
> if (!g_dbus_register_interface(conn, path,
> @@ -1234,6 +1476,18 @@ void ofono_call_forwarding_register(struct
> ofono_call_forwarding *cf)
>
> ofono_modem_add_interface(modem, OFONO_CALL_FORWARDING_INTERFACE);
>
> + sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
> +
> + if (sim_atom) {
> + cf->sim = __ofono_atom_get_data(sim_atom);
> +
> + if (ofono_sim_get_state(cf->sim) == OFONO_SIM_STATE_READY)
This check can be skipped, we're always in post_sim state. The only way
to get there is if we reached OFONO_SIM_STATE_READY
> + sim_read_cf_indicator(cf);
> + }
> +
> + __ofono_modem_add_online_watch(modem, modem_online_status_changed,
> + cf, NULL);
> +
> cf->ussd_watch = __ofono_modem_add_atom_watch(modem,
> OFONO_ATOM_TYPE_USSD,
> ussd_watch, cf, NULL);
Regards,
-Denis
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
