Re: [PATCH 1/7] call-forwarding: Read/Write cfis/cphs-cff

2010-12-03 Thread Denis Kenzior
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;



> @@ -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);
>  



> @@ -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)



> @@ -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(st

RE: [PATCH 1/7] call-forwarding: Read/Write cfis/cphs-cff

2010-12-07 Thread Jeevaka.Badrappan
Hi Denis,
 
> > +   ofono_bool_t online;
> 
> Why do you need to track this variable?  Can't you simply 
> call ofono_modem_get_online()?
> 

This way calling of ofono_modem_get_online for each get or set request
can be avoided.

Regards,
Jeevaka
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/7] call-forwarding: Read/Write cfis/cphs-cff

2010-12-07 Thread Denis Kenzior
Hi Jeevaka,

On 12/07/2010 07:59 AM, jeevaka.badrap...@elektrobit.com wrote:
> Hi Denis,
>  
>>> +   ofono_bool_t online;
>>
>> Why do you need to track this variable?  Can't you simply 
>> call ofono_modem_get_online()?
>>
> 
> This way calling of ofono_modem_get_online for each get or set request
> can be avoided.
> 

Sure, but it is doubtful you really need to 'optimize' those cases.  The
code is way cleaner without it in my opinion.

Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/7] call-forwarding: Read/Write cfis/cphs-cff

2010-12-17 Thread Denis Kenzior
Hi Jeevaka,

On 12/17/2010 01:04 AM, Jeevaka Badrappan wrote:
> ---
>  src/call-forwarding.c |  243 
> -
>  1 files changed, 241 insertions(+), 2 deletions(-)
> 

So I applied this patch but refactored it very heavily afterward.  Can
you do review the changes and make sure you're OK with them?  Can you
also submit the needed records for phonesim, so we can get those tested
as well?

I still have one concern with cfis_record_id selection logic.  There are
a couple of possibilities we might not be handling correctly:

- if no EFcfis records contain proper MSP ids
- if EFcfis record has a valid MSP id, but is relevant to teleservices
besides voice.  We still potentially select this record, even though a
voice specific record might or might not exist.

Overall I think we need to test this feature quite a bit more...

Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 1/7] call-forwarding: Read/Write cfis/cphs-cff

2022-06-24 Thread nidhiverma5689
That's very amazing could you tell me the more about this.
https://www.escortdehradun.com/
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org